Bug in ipv6 ipsec in handling of packets with extension headers

Hi,

There's a bug in get_offset function of ah6 and esp6. The function returns also a pointer, prev_hdr, pointing to the last extension header before the IPSec headers. This pointer points to the skb. The ipsec headers go between the payload and the extension header, making the pointer invalid. However, after this the pointer is used for setting the next header field of the extension header to IPPROTO_ESP or IPPROTO_AH. This corrupts the packet, if any extension headers are present.

An easy way to test this is to send a data packet with routing header protected by IPSec.

A possible fix is to change the pointer into an offset from the start of the packet and use the offset later to set the nexthdr value in the extension header.

Thanks,

Henrik

From: Henrik Petander <[email protected]>
Date: Thu, 05 Jun 2003 15:25:14 +0300

A possible fix is to change the pointer into an offset from the start of the packet and use the offset later to set the nexthdr value in the extension header.

Please indicate the version of the sources you are looking at when making reports.

Thank you.

Sure, esp6.c bitkeeper version was 1.16. Also a fix to the bug report: the problem is with esp6 and not with ah6, which does not use the get_offset function.

Henrik

Hello,

At Thu, 05 Jun 2003 15:59:32 +0300,

I have fixed this in our tree (replaced by ip6_found_nexthdr()).

I will send a patch related to these ipsec6 fix collection by this weekend ASAP.

Regards,
-mk

Hello,

At Thu, 05 Jun 2003 15:59:32 +0300,

The attached diff fixes esp6 extension headers handling bug which reported by Henrik.

I introduced ip6_find_1stfragopt() instead of get_offset().

# ip6_found_nexthdr() is just renamed ip6_find_1stfragopt()

# in order to represent collect functionality.

Regards,
-mk

Index: linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/include/net/ipv6.h
===================================================================
RCS file: /cvsroot/usagi/usagi-backport/linux25/include/net/ipv6.h,v retrieving revision 1.1.1.12
retrieving revision 1.1.1.12.8.1

diff -u -r1.1.1.12 -r1.1.1.12.8.1
@@ -315,7 +315,7 @@
 					       unsigned length,
 					       struct ipv6_txoptions *opt,
 					       int hlimit, int flags);
-extern int			ip6_found_nexthdr(struct sk_buff *skb, u8 **nexthdr);
+extern int			ip6_find_1stfragopt(struct sk_buff *skb, u8 **nexthdr);
 
 extern int			ip6_append_data(struct sock *sk,
 						int getfrag(void *from, char *to, int offset, int len, int odd, struct sk_buff *skb),

Index: linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/esp6.c
===================================================================
RCS file: /cvsroot/usagi/usagi-backport/linux25/net/ipv6/esp6.c,v retrieving revision 1.1.1.13
retrieving revision 1.1.1.13.12.1

Index: linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ip6_output.c
===================================================================
RCS file: /cvsroot/usagi/usagi-backport/linux25/net/ipv6/ip6_output.c,v retrieving revision 1.1.1.16
retrieving revision 1.1.1.16.16.1

diff -u -r1.1.1.16 -r1.1.1.16.16.1
@@ -887,7 +887,7 @@
 #endif
 }
 
-int ip6_found_nexthdr(struct sk_buff *skb, u8 **nexthdr)
+int ip6_find_1stfragopt(struct sk_buff *skb, u8 **nexthdr)
 {
 	u16 offset = sizeof(struct ipv6hdr);
 	struct ipv6_opt_hdr *exthdr = (struct ipv6_opt_hdr*)(skb->nh.ipv6h + 1);

@@ -929,7 +929,7 @@
 	u8 *prevhdr, nexthdr = 0;
 
 	dev = rt->u.dst.dev;
-	hlen = ip6_found_nexthdr(skb, &prevhdr);
+	hlen = ip6_find_1stfragopt(skb, &prevhdr);
 	nexthdr = *prevhdr;
 
 	mtu = dst_pmtu(&rt->u.dst) - hlen - sizeof(struct frag_hdr);

Index: linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ipcomp6.c
===================================================================
RCS file: /cvsroot/usagi/usagi-backport/linux25/net/ipv6/ipcomp6.c,v retrieving revision 1.1.1.2
retrieving revision 1.1.1.2.14.1

diff -u -r1.1.1.2 -r1.1.1.2.14.1
@@ -105,7 +105,7 @@
 	iph = skb->nh.ipv6h;
 	iph->payload_len = htons(skb->len);
 	
-	ip6_found_nexthdr(skb, &prevhdr);
+	ip6_find_1stfragopt(skb, &prevhdr);
 	*prevhdr = nexthdr;
 out:
 	if (tmp_hdr)

@@ -160,7 +160,7 @@
 		skb->nh.raw = skb->data; /* == top_iph */
 		skb->h.raw = skb->nh.raw + hdr_len;
 	} else {
-		hdr_len = ip6_found_nexthdr(skb, &prevhdr);
+		hdr_len = ip6_find_1stfragopt(skb, &prevhdr);
 		nexthdr = *prevhdr;
 	}
 

@@ -203,7 +203,7 @@
 
 	top_iph->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
 	skb->nh.raw = skb->data; /* top_iph */
-	ip6_found_nexthdr(skb, &prevhdr); 
+	ip6_find_1stfragopt(skb, &prevhdr); 
 	*prevhdr = IPPROTO_COMP;
 
 	ipch = (struct ipv6_comp_hdr *)((unsigned char *)top_iph + hdr_len);

Index: linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ipv6_syms.c
===================================================================
RCS file: /cvsroot/usagi/usagi-backport/linux25/net/ipv6/ipv6_syms.c,v retrieving revision 1.1.1.12
retrieving revision 1.1.1.12.16.4

diff -u -r1.1.1.12 -r1.1.1.12.16.4
@@ -35,6 +35,6 @@
 EXPORT_SYMBOL(in6addr_any);
 EXPORT_SYMBOL(in6addr_loopback);
 EXPORT_SYMBOL(in6_dev_finish_destroy);
-EXPORT_SYMBOL(ip6_found_nexthdr);
+EXPORT_SYMBOL(ip6_find_1stfragopt);
 EXPORT_SYMBOL(xfrm6_rcv);
 EXPORT_SYMBOL(xfrm6_clear_mutable_options);

From: Mitsuru KANDA / 神田 充 <[email protected]>
Date: Sat, 07 Jun 2003 03:17:10 +0900

The attached diff fixes esp6 extension headers handling bug which reported by Henrik.

I introduced ip6_find_1stfragopt() instead of get_offset().

# ip6_found_nexthdr() is just renamed ip6_find_1stfragopt()

   # in order to represent collect functionality.

Patch applied, thank you.