| Message ID | 4D2F3453.9020203@6wind.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Thu, 13 Jan 2011 18:20:19 +0100 > here is a patch to fix alignment of IPv4 AH. Note that this break > compatiblity for some algorithms (like SHA256) with old kernels > ... but upstream cannot use SHA256 on IPv4, for example, with a target > that is RFC compliant. > > I don't know what is the best way to fix this. We cannot just start rejecting the old 8-byte alignment on input if Linux has been using an 8-byte alignment since day one. If you want this change to be considered seriously, you need to relax the AH4 input check. -- 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
David Miller <davem@davemloft.net> wrote: > > We cannot just start rejecting the old 8-byte alignment on input if > Linux has been using an 8-byte alignment since day one. > > If you want this change to be considered seriously, you need to relax > the AH4 input check. I second your sentiment. However, in this particular case it would appear that our old implementation was also overly strict in rejecting 32-bit alignment so even if we relax it now it still wouldn't work with an old implementation once we reduce the padding on output (unless you traffic was one-way only). So perhaps an SA configuration flag is needed? Cheers,
On 28/01/2011 05:51, Herbert Xu wrote: > David Miller<davem@davemloft.net> wrote: >> >> We cannot just start rejecting the old 8-byte alignment on input if >> Linux has been using an 8-byte alignment since day one. >> >> If you want this change to be considered seriously, you need to relax >> the AH4 input check. > > I second your sentiment. However, in this particular case it > would appear that our old implementation was also overly strict > in rejecting 32-bit alignment so even if we relax it now it still > wouldn't work with an old implementation once we reduce the padding > on output (unless you traffic was one-way only). Yes, this was my initial problem. > > So perhaps an SA configuration flag is needed? I agree. If David is ok, I will update the patch. Regards, Nicolas -- 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
From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Fri, 28 Jan 2011 09:51:40 +0100 > On 28/01/2011 05:51, Herbert Xu wrote: >> So perhaps an SA configuration flag is needed? > I agree. If David is ok, I will update the patch. Sounds good to me. -- 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
From 14bbe173eed25cf59e3e54222eb7de1a5578e54e Mon Sep 17 00:00:00 2001 From: Dang Hongwu <hongwu.dang@6wind.com> Date: Wed, 22 Dec 2010 11:38:47 -0500 Subject: [PATCH] ipsec: fix IPv4 AH alignment on 32 bits The Linux IPv4 AH stack aligns the AH header on a 64 bit boundary (like in IPv6). This is not RFC compliant (see RFC4302, Section 3.3.3.2.1), it should be aligned on 32 bits. For most of the authentication algorithms, the ICV size is 96 bits. The AH header alignment on 32 or 64 bits gives the same results. However for SHA-256-128 for instance, the wrong 64 bit alignment results in adding useless padding in IPv4 AH, which is forbidden by the RFC. Signed-off-by: Dang Hongwu <hongwu.dang@6wind.com> Signed-off-by: Christophe Gouault <christophe.gouault@6wind.com> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- include/net/xfrm.h | 1 + net/ipv4/ah4.c | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/net/xfrm.h b/include/net/xfrm.h index bcfb6b2..525d882 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -36,6 +36,7 @@ #define XFRM_PROTO_ROUTING IPPROTO_ROUTING #define XFRM_PROTO_DSTOPTS IPPROTO_DSTOPTS +#define XFRM_ALIGN4(len) (((len) + 3) & ~3) #define XFRM_ALIGN8(len) (((len) + 7) & ~7) #define MODULE_ALIAS_XFRM_MODE(family, encap) \ MODULE_ALIAS("xfrm-mode-" __stringify(family) "-" __stringify(encap)) diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c index 86961be..95561d6 100644 --- a/net/ipv4/ah4.c +++ b/net/ipv4/ah4.c @@ -201,7 +201,7 @@ static int ah_output(struct xfrm_state *x, struct sk_buff *skb) top_iph->ttl = 0; top_iph->check = 0; - ah->hdrlen = (XFRM_ALIGN8(sizeof(*ah) + ahp->icv_trunc_len) >> 2) - 2; + ah->hdrlen = (XFRM_ALIGN4(sizeof(*ah) + ahp->icv_trunc_len) >> 2) - 2; ah->reserved = 0; ah->spi = x->id.spi; @@ -299,8 +299,8 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb) nexthdr = ah->nexthdr; ah_hlen = (ah->hdrlen + 2) << 2; - if (ah_hlen != XFRM_ALIGN8(sizeof(*ah) + ahp->icv_full_len) && - ah_hlen != XFRM_ALIGN8(sizeof(*ah) + ahp->icv_trunc_len)) + if (ah_hlen != XFRM_ALIGN4(sizeof(*ah) + ahp->icv_full_len) && + ah_hlen != XFRM_ALIGN4(sizeof(*ah) + ahp->icv_trunc_len)) goto out; if (!pskb_may_pull(skb, ah_hlen)) @@ -450,7 +450,7 @@ static int ah_init_state(struct xfrm_state *x) BUG_ON(ahp->icv_trunc_len > MAX_AH_AUTH_LEN); - x->props.header_len = XFRM_ALIGN8(sizeof(struct ip_auth_hdr) + + x->props.header_len = XFRM_ALIGN4(sizeof(struct ip_auth_hdr) + ahp->icv_trunc_len); if (x->props.mode == XFRM_MODE_TUNNEL) x->props.header_len += sizeof(struct iphdr); -- 1.5.6.5