Message ID | 20130124112410.8535.75598.stgit@localhost6.localdomain6 |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, Jan 24, 2013 at 01:25:46PM +0200, Jussi Kivilinna wrote: > > Maybe it would be cleaner to not mess with pfkeyv2.h at all, but instead mark algorithms that do not support pfkey with flag. See patch below. > Yes, would be an option too. I would be fine with that, but let's here if someone else has an opinion on this. Anyway, we need a solution to integrate Tom's patch soon. > Then I started looking up if sadb_alg_id is being used somewhere outside pfkey. Seems that its value is just being copied around.. but at "http://lxr.linux.no/linux+v3.7/net/xfrm/xfrm_policy.c#L1991" it's used as bit-index. So do larger values than 31 break some stuff? Can multiple algorithms have same sadb_alg_id value? Also in af_key.c, sadb_alg_id being used as bit-index. > Herbert tried to address this already in git commit c5d18e984 ([IPSEC]: Fix catch-22 with algorithm IDs above 31) some years ago. But this looks still messy. If the aalgos, ealgos and calgos mask is ~0, we allow all algorithms. If this is not the case, xfrm and pfkey check the aalgos mask against the algorithm ID, only pfkey checks the ealgo mask and noone checks the calgos mask. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > From: "Steffen Klassert" <steffen.klassert@secunet.com> > To: "Jussi Kivilinna" <jussi.kivilinna@mbnet.fi> > Cc: "Herbert Xu" <herbert@gondor.apana.org.au>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, > linux-crypto@vger.kernel.org, "Tom St Denis" <tstdenis@elliptictech.com>, "David Miller" <davem@davemloft.net> > Sent: Thursday, 24 January, 2013 7:32:10 AM > Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issues > > On Thu, Jan 24, 2013 at 01:25:46PM +0200, Jussi Kivilinna wrote: > > > > Maybe it would be cleaner to not mess with pfkeyv2.h at all, but > > instead mark algorithms that do not support pfkey with flag. See > > patch below. > > > > Yes, would be an option too. I would be fine with that, > but let's here if someone else has an opinion on this. > Anyway, we need a solution to integrate Tom's patch soon. Has anyone addressed the testmgr issues too? I wasn't able to run the self-tests so I don't even know if the vectors were copied correctly. Also a question for the netdev folk, in order to be timely would it be acceptable to patch ah4 and then ah6 with the AEAD changes? Or would the team require both to be patched simultaneously? Tom -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 24, 2013 at 07:37:50AM -0500, Tom St Denis wrote: > > Also a question for the netdev folk, in order to be timely would it be acceptable to patch ah4 and then ah6 with the AEAD changes? Or would the team require both to be patched simultaneously? > We would need patches for both, but the changes to ah4/ah6 should e very similar. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > From: "Steffen Klassert" <steffen.klassert@secunet.com> > To: "Tom St Denis" <tstdenis@elliptictech.com> > Cc: "Herbert Xu" <herbert@gondor.apana.org.au>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, > linux-crypto@vger.kernel.org, "David Miller" <davem@davemloft.net>, "Jussi Kivilinna" <jussi.kivilinna@mbnet.fi> > Sent: Thursday, 24 January, 2013 7:52:34 AM > Subject: Re: [PATCH] CMAC support for CryptoAPI, fixed patch issues, indent, and testmgr build issues > > On Thu, Jan 24, 2013 at 07:37:50AM -0500, Tom St Denis wrote: > > > > Also a question for the netdev folk, in order to be timely would it > > be acceptable to patch ah4 and then ah6 with the AEAD changes? Or > > would the team require both to be patched simultaneously? > > > > We would need patches for both, but the changes to ah4/ah6 should > e very similar. I suspect, I just don't know how my time table will look like so I was trying to not block the submission of one by the other. Fortunately, I don't need hardware to test these changes so I can probably skunkwork them from home. Tom -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 24, 2013 at 01:25:46PM +0200, Jussi Kivilinna wrote: > > Maybe it would be cleaner to not mess with pfkeyv2.h at all, but instead mark algorithms that do not support pfkey with flag. See patch below. > As nobody seems to have another opinion, we could go either with your approach, or we can invert the logic and mark all existing algorithms as pfkey supported. Then we would not need to bother about pfkey again. I'd be fine with both. Do you want to submit a patch? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Steffen Klassert <steffen.klassert@secunet.com>: > On Thu, Jan 24, 2013 at 01:25:46PM +0200, Jussi Kivilinna wrote: >> >> Maybe it would be cleaner to not mess with pfkeyv2.h at all, but >> instead mark algorithms that do not support pfkey with flag. See >> patch below. >> > > As nobody seems to have another opinion, we could go either with your > approach, or we can invert the logic and mark all existing algorithms > as pfkey supported. Then we would not need to bother about pfkey again. > > I'd be fine with both. Do you want to submit a patch? > Ok, I'll invert the logic and send new patch. -Jussi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 421f764..5d5eec2 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -1320,6 +1320,7 @@ struct xfrm_algo_desc { char *name; char *compat; u8 available:1; + u8 sadb_disabled:1; union { struct xfrm_algo_aead_info aead; struct xfrm_algo_auth_info auth; @@ -1561,8 +1562,8 @@ extern void xfrm_input_init(void); extern int xfrm_parse_spi(struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 *seq); extern void xfrm_probe_algs(void); -extern int xfrm_count_auth_supported(void); -extern int xfrm_count_enc_supported(void); +extern int xfrm_count_sadb_auth_supported(void); +extern int xfrm_count_sadb_enc_supported(void); extern struct xfrm_algo_desc *xfrm_aalg_get_byidx(unsigned int idx); extern struct xfrm_algo_desc *xfrm_ealg_get_byidx(unsigned int idx); extern struct xfrm_algo_desc *xfrm_aalg_get_byid(int alg_id); diff --git a/net/key/af_key.c b/net/key/af_key.c index 5b426a6..307cf1d 100644 --- a/net/key/af_key.c +++ b/net/key/af_key.c @@ -816,18 +816,21 @@ static struct sk_buff *__pfkey_xfrm_state2msg(const struct xfrm_state *x, sa->sadb_sa_auth = 0; if (x->aalg) { struct xfrm_algo_desc *a = xfrm_aalg_get_byname(x->aalg->alg_name, 0); - sa->sadb_sa_auth = a ? a->desc.sadb_alg_id : 0; + sa->sadb_sa_auth = (a && !a->sadb_disabled) ? + a->desc.sadb_alg_id : 0; } sa->sadb_sa_encrypt = 0; BUG_ON(x->ealg && x->calg); if (x->ealg) { struct xfrm_algo_desc *a = xfrm_ealg_get_byname(x->ealg->alg_name, 0); - sa->sadb_sa_encrypt = a ? a->desc.sadb_alg_id : 0; + sa->sadb_sa_encrypt = (a && !a->sadb_disabled) ? + a->desc.sadb_alg_id : 0; } /* KAME compatible: sadb_sa_encrypt is overloaded with calg id */ if (x->calg) { struct xfrm_algo_desc *a = xfrm_calg_get_byname(x->calg->alg_name, 0); - sa->sadb_sa_encrypt = a ? a->desc.sadb_alg_id : 0; + sa->sadb_sa_encrypt = (a && !a->sadb_disabled) ? + a->desc.sadb_alg_id : 0; } sa->sadb_sa_flags = 0; @@ -1138,7 +1141,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net, if (sa->sadb_sa_auth) { int keysize = 0; struct xfrm_algo_desc *a = xfrm_aalg_get_byid(sa->sadb_sa_auth); - if (!a) { + if (!a || a->sadb_disabled) { err = -ENOSYS; goto out; } @@ -1160,7 +1163,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net, if (sa->sadb_sa_encrypt) { if (hdr->sadb_msg_satype == SADB_X_SATYPE_IPCOMP) { struct xfrm_algo_desc *a = xfrm_calg_get_byid(sa->sadb_sa_encrypt); - if (!a) { + if (!a || a->sadb_disabled) { err = -ENOSYS; goto out; } @@ -1172,7 +1175,7 @@ static struct xfrm_state * pfkey_msg2xfrm_state(struct net *net, } else { int keysize = 0; struct xfrm_algo_desc *a = xfrm_ealg_get_byid(sa->sadb_sa_encrypt); - if (!a) { + if (!a || a->sadb_disabled) { err = -ENOSYS; goto out; } @@ -1578,13 +1581,13 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig, struct sadb_msg *hdr; int len, auth_len, enc_len, i; - auth_len = xfrm_count_auth_supported(); + auth_len = xfrm_count_sadb_auth_supported(); if (auth_len) { auth_len *= sizeof(struct sadb_alg); auth_len += sizeof(struct sadb_supported); } - enc_len = xfrm_count_enc_supported(); + enc_len = xfrm_count_sadb_enc_supported(); if (enc_len) { enc_len *= sizeof(struct sadb_alg); enc_len += sizeof(struct sadb_supported); @@ -1615,6 +1618,8 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig, struct xfrm_algo_desc *aalg = xfrm_aalg_get_byidx(i); if (!aalg) break; + if (aalg->sadb_disabled) + continue; if (aalg->available) *ap++ = aalg->desc; } @@ -1634,6 +1639,8 @@ static struct sk_buff *compose_sadb_supported(const struct sadb_msg *orig, struct xfrm_algo_desc *ealg = xfrm_ealg_get_byidx(i); if (!ealg) break; + if (ealg->sadb_disabled) + continue; if (ealg->available) *ap++ = ealg->desc; } @@ -2825,6 +2832,8 @@ static int count_ah_combs(const struct xfrm_tmpl *t) const struct xfrm_algo_desc *aalg = xfrm_aalg_get_byidx(i); if (!aalg) break; + if (aalg->sadb_disabled) + continue; if (aalg_tmpl_set(t, aalg) && aalg->available) sz += sizeof(struct sadb_comb); } @@ -2840,6 +2849,9 @@ static int count_esp_combs(const struct xfrm_tmpl *t) if (!ealg) break; + if (ealg->sadb_disabled) + continue; + if (!(ealg_tmpl_set(t, ealg) && ealg->available)) continue; @@ -2848,6 +2860,9 @@ static int count_esp_combs(const struct xfrm_tmpl *t) if (!aalg) break; + if (aalg->sadb_disabled) + continue; + if (aalg_tmpl_set(t, aalg) && aalg->available) sz += sizeof(struct sadb_comb); } @@ -2871,6 +2886,9 @@ static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t) if (!aalg) break; + if (aalg->sadb_disabled) + continue; + if (aalg_tmpl_set(t, aalg) && aalg->available) { struct sadb_comb *c; c = (struct sadb_comb*)skb_put(skb, sizeof(struct sadb_comb)); @@ -2903,6 +2921,9 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t) if (!ealg) break; + if (ealg->sadb_disabled) + continue; + if (!(ealg_tmpl_set(t, ealg) && ealg->available)) continue; @@ -2911,6 +2932,8 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t) const struct xfrm_algo_desc *aalg = xfrm_aalg_get_byidx(k); if (!aalg) break; + if (aalg->sadb_disabled) + continue; if (!(aalg_tmpl_set(t, aalg) && aalg->available)) continue; c = (struct sadb_comb*)skb_put(skb, sizeof(struct sadb_comb)); diff --git a/net/xfrm/xfrm_algo.c b/net/xfrm/xfrm_algo.c index 4694cca..dab881c 100644 --- a/net/xfrm/xfrm_algo.c +++ b/net/xfrm/xfrm_algo.c @@ -713,27 +713,27 @@ void xfrm_probe_algs(void) } EXPORT_SYMBOL_GPL(xfrm_probe_algs); -int xfrm_count_auth_supported(void) +int xfrm_count_sadb_auth_supported(void) { int i, n; for (i = 0, n = 0; i < aalg_entries(); i++) - if (aalg_list[i].available) + if (aalg_list[i].available && !aalg_list[i].sadb_disabled) n++; return n; } -EXPORT_SYMBOL_GPL(xfrm_count_auth_supported); +EXPORT_SYMBOL_GPL(xfrm_count_sadb_auth_supported); -int xfrm_count_enc_supported(void) +int xfrm_count_sadb_enc_supported(void) { int i, n; for (i = 0, n = 0; i < ealg_entries(); i++) - if (ealg_list[i].available) + if (ealg_list[i].available && !ealg_list[i].sadb_disabled) n++; return n; } -EXPORT_SYMBOL_GPL(xfrm_count_enc_supported); +EXPORT_SYMBOL_GPL(xfrm_count_sadb_enc_supported); #if defined(CONFIG_INET_ESP) || defined(CONFIG_INET_ESP_MODULE) || defined(CONFIG_INET6_ESP) || defined(CONFIG_INET6_ESP_MODULE)