Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path

netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path
@ 2017-04-19  2:15 Dave Johnson
  2017-04-20 12:44 ` [PATCH] netfilter: " Dave Johnson
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Johnson @ 2017-04-19  2:15 UTC (permalink / raw)
  To: netfilter-devel

I Have the following setup.

3 linux systems A, B, and C.

host A is an end host with IPv6 address Z2.

host B is doing v6/v6 forwarding (no NAT) between an ethernet
interface and a tun (openvpn) with addresses Z1 and Y2.

host C is doing v6/v6 SNAT between a tun (openvpn) and an
ethernet interface with addresses Y1 and X2.  X2 is a static address
so SNAT instead of MASQUERADE is used.

Network X is a public IPv6 subnet.

Networks Y and Z are private networks and need NAT66 SNAT on their way
to the internet done in host C.

  +--------+       +---------------+       +---------------+       +------...
  | host-A |       |     host-B    |       |     host-C    |       | internet
  |  IP-Z2 <- eth -> IP-Z1   IP-Y2 <- tun -> IP-Y1   IP-X2 <- eth -> IP-X1
  +--------+       +---------------+       +---------------+       +------...
                       (IPv6 Fwd)             (IPv6 SNAT)

See problem when doing a traceroute with icmp6 from host A to
internet.

All ICMPV6_TIME_EXCEED responses from X1 and beyond are getting
corrupted by host C on the reverse path back to host A.

Final hop return ICMPV6_ECHO_REPLY which is fine.

  host-A$ traceroute -6 --icmp -n www.google.com
  traceroute to www.google.com (2607:f8b0:4004:800::2004), 30 hops max, 80 byte packets
   1  [host-B-Z1]  0.313 ms  0.326 ms  0.371 ms
   2  [host-C-Y1]  19.732 ms  20.040 ms  20.051 ms
   3  * * *
   4  * * *
   5  * * *
   6  * * *
   7  * * *
   8  * * *
   9  * * *
  10  * * *
  11  2607:f8b0:4004:800::2004  26.670 ms  27.193 ms  27.202 ms

All those ICMPV6_TIME_EXCEED packets are corrupted by host C and are
discarded by host A:

host A is logging:

  Apr 18 17:10:56 host-A kernel: [9843084.191460] ICMPv6 checksum failed [some-internet-router-3 > host-A-Z2]
  Apr 18 17:10:56 host-A kernel: [9843084.191504] ICMPv6 checksum failed [some-internet-router-3 > host-A-Z2]
  Apr 18 17:10:56 host-A kernel: [9843084.193587] ICMPv6 checksum failed [some-internet-router-3 > host-A-Z2]
  Apr 18 17:10:56 host-A kernel: [9843084.196254] ICMPv6 checksum failed [some-internet-router-4 > host-A-Z2]
  Apr 18 17:10:56 host-A kernel: [9843084.196398] ICMPv6 checksum failed [some-internet-router-4 > host-A-Z2]
  Apr 18 17:10:56 host-A kernel: [9843084.196526] ICMPv6 checksum failed [some-internet-router-4 > host-A-Z2]

A packet capture on host-C's X2 interface show correct (pre-nat)
packets on the reverse path, and on host-C's Y1 tun interface show
incorrect (post-nat) packets on the reverse path.

Looking at the packet capture, 4 updates are needed by host C on it's
way from internet back to host A:

1) outer (ICMPV6_TIME_EXCEED) packet dest ip from X2 to Z2
2) outer (ICMPV6_TIME_EXCEED) packet icmp6 checksum recalc
3) inner (ICMPV6_ECHO_REQUEST) packet src ip from X2 to Z2
4) inner (ICMPV6_ECHO_REQUEST) packet icmp6 checksum recalc

1,3,4 are done correctly by host C.
2 is changed by host C but to an incorrect value.

Originally seen in kernel 3.16, but brought host C up to kernel 4.9.23
to make sure it's not already fixed in recent builds, still happens in
that version.

seems like possible issue in nf_nat_icmpv6_reply_translation() or
icmpv6_manip_pkt() area.

Thoughts before I start digging deaper?

-- 
Dave

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] netfilter: Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path
  2017-04-19  2:15 Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path Dave Johnson
@ 2017-04-20 12:44 ` Dave Johnson
  2017-04-24  8:43   ` Pablo Neira Ayuso
  2017-04-25  9:15   ` Pablo Neira Ayuso
  0 siblings, 2 replies; 6+ messages in thread
From: Dave Johnson @ 2017-04-20 12:44 UTC (permalink / raw)
  To: netfilter-devel, coreteam


When recalculating the outer ICMPv6 checksum for a reverse path NATv6
such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was
accessing data beyond the headlen of the skb for non-linear skb.  This
resulted in incorrect ICMPv6 checksum as garbage data was used.

Signed-off-by: Dave Johnson <dave-kernel@centerclick.org>
---
diff -rup linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
--- linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-18 01:12:30.000000000 -0400
+++ linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-20 08:13:41.070493666 -0400
@@ -229,7 +229,12 @@ int nf_nat_icmpv6_reply_translation(stru
 		return 0;
 
 	if (skb->ip_summed != CHECKSUM_PARTIAL) {
-		struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+		struct ipv6hdr *ipv6h;
+
+		if (!skb_make_writable(skb, skb->len))
+			return 0;
+
+		ipv6h = ipv6_hdr(skb);
 		inside = (void *)skb->data + hdrlen;
 		inside->icmp6.icmp6_cksum = 0;
 		inside->icmp6.icmp6_cksum =

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] netfilter: Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path
  2017-04-20 12:44 ` [PATCH] netfilter: " Dave Johnson
@ 2017-04-24  8:43   ` Pablo Neira Ayuso
       [not found]     ` <22781.62083.680363.165680@gargle.gargle.HOWL>
  2017-04-25  9:15   ` Pablo Neira Ayuso
  1 sibling, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2017-04-24  8:43 UTC (permalink / raw)
  To: Dave Johnson; +Cc: netfilter-devel, coreteam

On Thu, Apr 20, 2017 at 08:44:21AM -0400, Dave Johnson wrote:
> 
> When recalculating the outer ICMPv6 checksum for a reverse path NATv6
> such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was
> accessing data beyond the headlen of the skb for non-linear skb.  This
> resulted in incorrect ICMPv6 checksum as garbage data was used.
> 
> Signed-off-by: Dave Johnson <dave-kernel@centerclick.org>
> ---
> diff -rup linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
> --- linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-18 01:12:30.000000000 -0400
> +++ linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-20 08:13:41.070493666 -0400
> @@ -229,7 +229,12 @@ int nf_nat_icmpv6_reply_translation(stru
>  		return 0;
>  
>  	if (skb->ip_summed != CHECKSUM_PARTIAL) {
> -		struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> +		struct ipv6hdr *ipv6h;
> +
> +		if (!skb_make_writable(skb, skb->len))

can we just make sure what we need is linear? I mean, just the ipv6
header that is what we need, instead of the entire skbuff.

^ permalink raw reply	[flat|nested] 6+ messages in thread

[parent not found: <22781.62083.680363.165680@gargle.gargle.HOWL>]

* Re: [PATCH] netfilter: Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path
       [not found]     ` <22781.62083.680363.165680@gargle.gargle.HOWL>
@ 2017-04-24 13:10       ` Dave Johnson
       [not found]       ` <22781.63534.706649.79254@gargle.gargle.HOWL>
  1 sibling, 0 replies; 6+ messages in thread
From: Dave Johnson @ 2017-04-24 13:10 UTC (permalink / raw)
  To: Pablo Neira Ayuso, netfilter-devel, coreteam

Pablo Neira Ayuso writes:
> On Thu, Apr 20, 2017 at 08:44:21AM -0400, Dave Johnson wrote:
> > 
> > When recalculating the outer ICMPv6 checksum for a reverse path NATv6
> > such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was
> > accessing data beyond the headlen of the skb for non-linear skb.  This
> > resulted in incorrect ICMPv6 checksum as garbage data was used.
> > 
> > Signed-off-by: Dave Johnson <dave-kernel@centerclick.org>
> > ---
> > diff -rup linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
> > --- linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-18 01:12:30.000000000 -0400
> > +++ linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-20 08:13:41.070493666 -0400
> > @@ -229,7 +229,12 @@ int nf_nat_icmpv6_reply_translation(stru
> >  		return 0;
> >  
> >  	if (skb->ip_summed != CHECKSUM_PARTIAL) {
> > -		struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> > +		struct ipv6hdr *ipv6h;
> > +
> > +		if (!skb_make_writable(skb, skb->len))
> 
> can we just make sure what we need is linear? I mean, just the ipv6
> header that is what we need, instead of the entire skbuff.

the checksum below those lines is across the entire skb as unknown
updates were done in the l4 manip call just prior to this.

nf_nat_icmp_reply_translation() for ipv4 uses skb_checksum() to walk
non-linear skbs.  gave that a try and it works, will send an updated
patch in a bit.

^ permalink raw reply	[flat|nested] 6+ messages in thread

[parent not found: <22781.63534.706649.79254@gargle.gargle.HOWL>]

* Re: [PATCH] netfilter: Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path
       [not found]       ` <22781.63534.706649.79254@gargle.gargle.HOWL>
@ 2017-04-24 13:11         ` Dave Johnson
  0 siblings, 0 replies; 6+ messages in thread
From: Dave Johnson @ 2017-04-24 13:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso, netfilter-devel, coreteam


When recalculating the outer ICMPv6 checksum for a reverse path NATv6
such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was
accessing data beyond the headlen of the skb for non-linear skb.  This
resulted in incorrect ICMPv6 checksum as garbage data was used.

Patch replaces csum_partial() with skb_checksum() which supports
non-linear skbs similar to nf_nat_icmp_reply_translation() from ipv4.

Signed-off-by: Dave Johnson <dave-kernel@centerclick.org>
---
diff -rup linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
--- linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-18 01:12:30.000000000 -0400
+++ linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c	2017-04-24 09:00:31.476785772 -0400
@@ -235,7 +235,7 @@ int nf_nat_icmpv6_reply_translation(stru
 		inside->icmp6.icmp6_cksum =
 			csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
 					skb->len - hdrlen, IPPROTO_ICMPV6,
-					csum_partial(&inside->icmp6,
+					skb_checksum(skb, hdrlen,
 						     skb->len - hdrlen, 0));
 	}
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] netfilter: Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path
  2017-04-20 12:44 ` [PATCH] netfilter: " Dave Johnson
  2017-04-24  8:43   ` Pablo Neira Ayuso
@ 2017-04-25  9:15   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2017-04-25  9:15 UTC (permalink / raw)
  To: Dave Johnson; +Cc: netfilter-devel, coreteam

On Thu, Apr 20, 2017 at 08:44:21AM -0400, Dave Johnson wrote:
> 
> When recalculating the outer ICMPv6 checksum for a reverse path NATv6
> such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was
> accessing data beyond the headlen of the skb for non-linear skb.  This
> resulted in incorrect ICMPv6 checksum as garbage data was used.

Applied to nf, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-04-25  9:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-19  2:15 Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path Dave Johnson
2017-04-20 12:44 ` [PATCH] netfilter: " Dave Johnson
2017-04-24  8:43   ` Pablo Neira Ayuso
     [not found]     ` <22781.62083.680363.165680@gargle.gargle.HOWL>
2017-04-24 13:10       ` Dave Johnson
     [not found]       ` <22781.63534.706649.79254@gargle.gargle.HOWL>
2017-04-24 13:11         ` Dave Johnson
2017-04-25  9:15   ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).