netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Bug in ipv6 ipsec in handling of packets with extension headers
  2003-06-05 12:25 Bug in ipv6 ipsec in handling of packets with extension headers Henrik Petander
@ 2003-06-05 12:17 ` David S. Miller
  2003-06-05 12:59   ` Henrik Petander
  0 siblings, 1 reply; 6+ messages in thread
From: David S. Miller @ 2003-06-05 12:17 UTC (permalink / raw)
  To: lpetande; +Cc: netdev

   From: Henrik Petander <lpetande@tml.hut.fi>
   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.

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

* Bug in ipv6 ipsec  in handling of packets with extension headers
@ 2003-06-05 12:25 Henrik Petander
  2003-06-05 12:17 ` David S. Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Henrik Petander @ 2003-06-05 12:25 UTC (permalink / raw)
  To: netdev

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

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

* Re: Bug in ipv6 ipsec in handling of packets with extension headers
  2003-06-05 12:17 ` David S. Miller
@ 2003-06-05 12:59   ` Henrik Petander
  2003-06-05 16:54     ` Mitsuru KANDA / 神田 充
  2003-06-06 18:17     ` [PATCH] fix esp6 extension headers handling Mitsuru KANDA / 神田 充
  0 siblings, 2 replies; 6+ messages in thread
From: Henrik Petander @ 2003-06-05 12:59 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

David S. Miller wrote:
>    From: Henrik Petander <lpetande@tml.hut.fi>
>    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.

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

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

* Re: Bug in ipv6 ipsec in handling of packets with extension headers
  2003-06-05 12:59   ` Henrik Petander
@ 2003-06-05 16:54     ` Mitsuru KANDA / 神田 充
  2003-06-06 18:17     ` [PATCH] fix esp6 extension headers handling Mitsuru KANDA / 神田 充
  1 sibling, 0 replies; 6+ messages in thread
From: Mitsuru KANDA / 神田 充 @ 2003-06-05 16:54 UTC (permalink / raw)
  To: Henrik Petander; +Cc: David S. Miller, netdev

Hello,

At Thu, 05 Jun 2003 15:59:32 +0300,
Henrik Petander wrote:
> 
> David S. Miller wrote:
> >    From: Henrik Petander <lpetande@tml.hut.fi>
> >    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.
> 
> 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.
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

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

* [PATCH] fix esp6 extension headers handling
  2003-06-05 12:59   ` Henrik Petander
  2003-06-05 16:54     ` Mitsuru KANDA / 神田 充
@ 2003-06-06 18:17     ` Mitsuru KANDA / 神田 充
  2003-06-07  9:22       ` David S. Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Mitsuru KANDA / 神田 充 @ 2003-06-06 18:17 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, usagi

Hello,

At Thu, 05 Jun 2003 15:59:32 +0300,
Henrik Petander <lpetande@tml.hut.fi> wrote:
> 
> David S. Miller wrote:
> >    From: Henrik Petander <lpetande@tml.hut.fi>
> >    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.
> 
> 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
> 


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
--- linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/include/net/ipv6.h	31 May 2003 07:30:34 -0000	1.1.1.12
+++ linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/include/net/ipv6.h	6 Jun 2003 15:43:46 -0000	1.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
diff -u -r1.1.1.13 -r1.1.1.13.12.1
--- linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/esp6.c	26 May 2003 08:04:11 -0000	1.1.1.13
+++ linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/esp6.c	6 Jun 2003 16:23:01 -0000	1.1.1.13.12.1
@@ -39,57 +39,6 @@
 
 #define MAX_SG_ONSTACK 4
 
-/* BUGS:
- * - we assume replay seqno is always present.
- */
-
-/* Move to common area: it is shared with AH. */
-/* Common with AH after some work on arguments. */
-
-static int get_offset(u8 *packet, u32 packet_len, u8 *nexthdr, struct ipv6_opt_hdr **prevhdr)
-{
-	u16 offset = sizeof(struct ipv6hdr);
-	struct ipv6_opt_hdr *exthdr = (struct ipv6_opt_hdr*)(packet + offset);
-	u8 nextnexthdr;
-
-	*nexthdr = ((struct ipv6hdr*)packet)->nexthdr;
-
-	while (offset + 1 < packet_len) {
-
-		switch (*nexthdr) {
-
-		case NEXTHDR_HOP:
-		case NEXTHDR_ROUTING:
-			offset += ipv6_optlen(exthdr);
-			*nexthdr = exthdr->nexthdr;
-			*prevhdr = exthdr;
-			exthdr = (struct ipv6_opt_hdr*)(packet + offset);
-			break;
-
-		case NEXTHDR_DEST:
-			nextnexthdr =
-				((struct ipv6_opt_hdr*)(packet + offset + ipv6_optlen(exthdr)))->nexthdr;
-			/* XXX We know the option is inner dest opt
-			   with next next header check. */
-			if (nextnexthdr != NEXTHDR_HOP &&
-		    	    nextnexthdr != NEXTHDR_ROUTING &&
-			    nextnexthdr != NEXTHDR_DEST) {
-					return offset;
-			}
-			offset += ipv6_optlen(exthdr);
-			*nexthdr = exthdr->nexthdr;
-			*prevhdr = exthdr;
-			exthdr = (struct ipv6_opt_hdr*)(packet + offset);
-			break;
-
-		default :
-			return offset;
-		}
-	}
-
-	return offset;
-}
-
 int esp6_output(struct sk_buff *skb)
 {
 	int err;
@@ -101,12 +50,12 @@
 	struct crypto_tfm *tfm;
 	struct esp_data *esp;
 	struct sk_buff *trailer;
-	struct ipv6_opt_hdr *prevhdr = NULL;
 	int blksize;
 	int clen;
 	int alen;
 	int nfrags;
-	u8 nexthdr;
+	u8 *prevhdr;
+	u8 nexthdr = 0;
 
 	/* First, if the skb is not checksummed, complete checksum. */
 	if (skb->ip_summed == CHECKSUM_HW && skb_checksum_help(skb) == NULL) {
@@ -123,7 +72,9 @@
 	/* Strip IP header in transport mode. Save it. */
 
 	if (!x->props.mode) {
-		hdr_len = get_offset(skb->nh.raw, skb->len, &nexthdr, &prevhdr);
+		hdr_len = ip6_find_1stfragopt(skb, &prevhdr);
+		nexthdr = *prevhdr;
+		*prevhdr = IPPROTO_ESP;
 		iph = kmalloc(hdr_len, GFP_ATOMIC);
 		if (!iph) {
 			err = -ENOMEM;
@@ -178,18 +129,12 @@
 		ipv6_addr_copy(&top_iph->daddr,
 			       (struct in6_addr *)&x->id.daddr);
 	} else { 
-		/* XXX exthdr */
 		esph = (struct ipv6_esp_hdr*)skb_push(skb, x->props.header_len);
 		skb->h.raw = (unsigned char*)esph;
 		top_iph = (struct ipv6hdr*)skb_push(skb, hdr_len);
 		memcpy(top_iph, iph, hdr_len);
 		kfree(iph);
 		top_iph->payload_len = htons(skb->len + alen - sizeof(struct ipv6hdr));
-		if (prevhdr) {
-			prevhdr->nexthdr = IPPROTO_ESP;
-		} else {
-			top_iph->nexthdr = IPPROTO_ESP;
-		}
 		*(u8*)(trailer->tail - 1) = nexthdr;
 	}
 
@@ -302,6 +247,7 @@
 		struct scatterlist sgbuf[nfrags>MAX_SG_ONSTACK ? 0 : nfrags];
 		struct scatterlist *sg = sgbuf;
 		u8 padlen;
+		u8 *prevhdr;
 
 		if (unlikely(nfrags > MAX_SG_ONSTACK)) {
 			sg = kmalloc(sizeof(struct scatterlist)*nfrags, GFP_ATOMIC);
@@ -325,11 +271,13 @@
 		}
 		/* ... check padding bits here. Silly. :-) */ 
 
-		ret_nexthdr = ((struct ipv6hdr*)tmp_hdr)->nexthdr = nexthdr[1];
 		pskb_trim(skb, skb->len - alen - padlen - 2);
 		skb->h.raw = skb_pull(skb, sizeof(struct ipv6_esp_hdr) + esp->conf.ivlen);
 		skb->nh.raw += sizeof(struct ipv6_esp_hdr) + esp->conf.ivlen;
 		memcpy(skb->nh.raw, tmp_hdr, hdr_len);
+		skb->nh.ipv6h->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
+		ip6_find_1stfragopt(skb, &prevhdr);
+		ret_nexthdr = *prevhdr = nexthdr[1];
 	}
 	kfree(tmp_hdr);
 	return ret_nexthdr;
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
--- linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ip6_output.c	26 May 2003 08:04:10 -0000	1.1.1.16
+++ linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ip6_output.c	6 Jun 2003 15:43:34 -0000	1.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
--- linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ipcomp6.c	21 May 2003 13:15:20 -0000	1.1.1.2
+++ linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ipcomp6.c	6 Jun 2003 15:43:34 -0000	1.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
--- linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ipv6_syms.c	26 May 2003 08:04:11 -0000	1.1.1.12
+++ linux25-b2_5_70+CS1_1314_IPSEC6_CLEANUP/net/ipv6/ipv6_syms.c	6 Jun 2003 17:38:20 -0000	1.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);

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

* Re: [PATCH] fix esp6 extension headers handling
  2003-06-06 18:17     ` [PATCH] fix esp6 extension headers handling Mitsuru KANDA / 神田 充
@ 2003-06-07  9:22       ` David S. Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David S. Miller @ 2003-06-07  9:22 UTC (permalink / raw)
  To: mk; +Cc: netdev, usagi

   From: Mitsuru KANDA / 神田 充 <mk@linux-ipv6.org>
   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.

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

end of thread, other threads:[~2003-06-07  9:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-06-05 12:25 Bug in ipv6 ipsec in handling of packets with extension headers Henrik Petander
2003-06-05 12:17 ` David S. Miller
2003-06-05 12:59   ` Henrik Petander
2003-06-05 16:54     ` Mitsuru KANDA / 神田 充
2003-06-06 18:17     ` [PATCH] fix esp6 extension headers handling Mitsuru KANDA / 神田 充
2003-06-07  9:22       ` David S. Miller

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).