[UDP] Check encap_type at config time

[UDP] Check encap_type at config time

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

Hi Dave:

My foray into the TTL stuff is turning into an all-out assault on NAT-T :)

The following patch moves the udp->encap_type check from the per-packet
hot-path into udp_setsockopt().

As a consequence, this allows user space to detect whether the kernel
actually supports the encap type that they're requesting.  Pity no one
did this before the NON-IKE patch was applied.  As it is there is no
easy way to detect whether NON-IKE support is present.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

PS I will be doing a similar patch for the encap_type in xfrm.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1272 bytes --]

===== net/ipv4/udp.c 1.61 vs edited =====
--- 1.61/net/ipv4/udp.c	2004-06-05 06:59:36 +10:00
+++ edited/net/ipv4/udp.c	2004-06-25 21:56:34 +10:00
@@ -964,6 +964,7 @@
 	len = skb->tail - udpdata;
 
 	switch (encap_type) {
+	default:
 	case UDP_ENCAP_ESPINUDP:
 		/* Check if this is a keepalive packet.  If so, eat it. */
 		if (len == 1 && udpdata[0] == 0xff) {
@@ -1016,12 +1017,6 @@
 		} else
 			/* Must be an IKE packet.. pass it through */
 			return 1;
-
-	default:
-		if (net_ratelimit())
-			printk(KERN_INFO "udp_encap_rcv(): Unhandled UDP encap type: %u\n",
-			       encap_type);
-		return 1;
 	}
 #endif
 }
@@ -1297,7 +1292,16 @@
 		break;
 		
 	case UDP_ENCAP:
-		up->encap_type = val;
+		switch (val) {
+		case 0:
+		case UDP_ENCAP_ESPINUDP:
+		case UDP_ENCAP_ESPINUDP_NON_IKE:
+			up->encap_type = val;
+			break;
+		default:
+			err = -ENOPROTOOPT;
+			break;
+		}
 		break;
 
 	default:
===== net/ipv6/udp.c 1.67 vs edited =====
--- 1.67/net/ipv6/udp.c	2004-06-21 09:37:54 +10:00
+++ edited/net/ipv6/udp.c	2004-06-25 20:13:50 +10:00
@@ -1044,7 +1044,14 @@
 		break;
 		
 	case UDP_ENCAP:
-		up->encap_type = val;
+		switch (val) {
+		case 0:
+			up->encap_type = val;
+			break;
+		default:
+			err = -ENOPROTOOPT;
+			break;
+		}
 		break;
 
 	default:

[IPSEC] Check encap_type at config time

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

On Fri, Jun 25, 2004 at 10:11:47PM +1000, herbert wrote:
> 
> PS I will be doing a similar patch for the encap_type in xfrm.

Here is the patch to check encap_type at the earliest possible
opportunity in xfrm_user/af_key.

This will allow us to assume in esp4 that the encap_type from x->encap
is always valid.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1007 bytes --]

===== net/key/af_key.c 1.61 vs edited =====
--- 1.61/net/key/af_key.c	2004-06-06 18:27:42 +10:00
+++ edited/net/key/af_key.c	2004-06-25 19:33:51 +10:00
@@ -1075,6 +1075,15 @@
 		n_type = ext_hdrs[SADB_X_EXT_NAT_T_TYPE-1];
 		natt->encap_type = n_type->sadb_x_nat_t_type_type;
 
+		switch (natt->encap_type) {
+		case UDP_ENCAP_ESPINUDP:
+		case UDP_ENCAP_ESPINUDP_NON_IKE:
+			break;
+		default:
+			err = -ENOPROTOOPT;
+			goto out;
+		}
+
 		if (ext_hdrs[SADB_X_EXT_NAT_T_SPORT-1]) {
 			struct sadb_x_nat_t_port* n_port =
 				ext_hdrs[SADB_X_EXT_NAT_T_SPORT-1];
===== net/xfrm/xfrm_user.c 1.42 vs edited =====
--- 1.42/net/xfrm/xfrm_user.c	2004-03-25 09:18:34 +11:00
+++ edited/net/xfrm/xfrm_user.c	2004-06-25 19:33:51 +10:00
@@ -78,6 +78,15 @@
 	if ((rt->rta_len - sizeof(*rt)) < sizeof(*encap))
 		return -EINVAL;
 
+	encap = RTA_DATA(rt);
+	switch (encap->encap_type) {
+	case UDP_ENCAP_ESPINUDP:
+	case UDP_ENCAP_ESPINUDP_NON_IKE:
+		break;
+	default:
+		return -ENOPROTOOPT;
+	}
+
 	return 0;
 }
 

Re: [IPSEC] Check encap_type at config time

[David S. Miller]

On Fri, 25 Jun 2004 22:35:27 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Here is the patch to check encap_type at the earliest possible
> opportunity in xfrm_user/af_key.
> 
> This will allow us to assume in esp4 that the encap_type from x->encap
> is always valid.

Applied.

[IPSEC] Remove run-time encap_type checks in esp4

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Fri, Jun 25, 2004 at 10:39:28AM -0700, David S. Miller wrote:
> On Fri, 25 Jun 2004 22:35:27 +1000
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > Here is the patch to check encap_type at the earliest possible
> > opportunity in xfrm_user/af_key.
> 
> Applied.

Thanks.

This allows us to remove all the per-packet checks on x->encap->encap_type.
I've left the check in esp_input just in case someone adds a non-ESP encap
type in future.

However, printing a warning and then continuing is definitely wrong.
So expect a follow-up patch to drop the packet when encap_type is
unknown in esp_input.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 2875 bytes --]

===== net/ipv4/esp4.c 1.44 vs edited =====
--- 1.44/net/ipv4/esp4.c	2004-06-25 18:38:22 +10:00
+++ edited/net/ipv4/esp4.c	2004-06-26 11:47:15 +10:00
@@ -94,8 +94,9 @@
 	if (x->props.mode) {
 		top_iph = (struct iphdr*)skb_push(*pskb, x->props.header_len);
 		esph = (struct ip_esp_hdr*)(top_iph+1);
-		if (encap && encap->encap_type) {
+		if (encap) {
 			switch (encap->encap_type) {
+			default:
 			case UDP_ENCAP_ESPINUDP:
 				uh = (struct udphdr*) esph;
 				esph = (struct ip_esp_hdr*)(uh+1);
@@ -108,12 +109,6 @@
 				esph = (struct ip_esp_hdr*)(udpdata32+2);
 				top_iph->protocol = IPPROTO_UDP;
 				break;
-			default:
-				printk(KERN_INFO
-				       "esp_output(): Unhandled encap: %u\n",
-				       encap->encap_type);
-				top_iph->protocol = IPPROTO_ESP;
-				break;
 			}
 		} else
 			top_iph->protocol = IPPROTO_ESP;
@@ -136,8 +131,9 @@
 		esph = (struct ip_esp_hdr*)skb_push(*pskb, x->props.header_len);
 		top_iph = (struct iphdr*)skb_push(*pskb, iph->ihl*4);
 		memcpy(top_iph, &tmp_iph, iph->ihl*4);
-		if (encap && encap->encap_type) {
+		if (encap) {
 			switch (encap->encap_type) {
+			default:
 			case UDP_ENCAP_ESPINUDP:
 				uh = (struct udphdr*) esph;
 				esph = (struct ip_esp_hdr*)(uh+1);
@@ -150,12 +146,6 @@
 				esph = (struct ip_esp_hdr*)(udpdata32+2);
 				top_iph->protocol = IPPROTO_UDP;
 				break;
-			default:
-				printk(KERN_INFO
-				       "esp_output(): Unhandled encap: %u\n",
-				       encap->encap_type);
-				top_iph->protocol = IPPROTO_ESP;
-				break;
 			}
 		} else
 			top_iph->protocol = IPPROTO_ESP;
@@ -365,11 +355,8 @@
 		if (encap->encap_type != decap->decap_type)
 			return -EINVAL;
 
-		/* Next, if we don't have an encap type, then ignore it */
-		if (!encap->encap_type)
-			return 0;
-
 		switch (encap->encap_type) {
+		default:
 		case UDP_ENCAP_ESPINUDP:
 		case UDP_ENCAP_ESPINUDP_NON_IKE:
 			/*
@@ -406,11 +393,6 @@
 				skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 			break;
-		default:
-			printk(KERN_INFO
-			       "esp4_post_input(): Unhandled encap type: %u\n",
-			       encap->encap_type);
-			break;
 		}
 	}
 	return 0;
@@ -547,20 +529,14 @@
 	if (x->encap) {
 		struct xfrm_encap_tmpl *encap = x->encap;
 
-		if (encap->encap_type) {
-			switch (encap->encap_type) {
-			case UDP_ENCAP_ESPINUDP:
-				x->props.header_len += sizeof(struct udphdr);
-				break;
-			case UDP_ENCAP_ESPINUDP_NON_IKE:
-				x->props.header_len += sizeof(struct udphdr) + 2 * sizeof(u32);
-				break;
-			default:
-				printk (KERN_INFO
-				"esp_init_state(): Unhandled encap type: %u\n",
-					encap->encap_type);
-				break;
-			}
+		switch (encap->encap_type) {
+		default:
+		case UDP_ENCAP_ESPINUDP:
+			x->props.header_len += sizeof(struct udphdr);
+			break;
+		case UDP_ENCAP_ESPINUDP_NON_IKE:
+			x->props.header_len += sizeof(struct udphdr) + 2 * sizeof(u32);
+			break;
 		}
 	}
 	x->data = esp;

[IPSEC] Drop bogus NAT-T printks in esp_input

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

On Sat, Jun 26, 2004 at 11:57:15AM +1000, herbert wrote:
> 
> However, printing a warning and then continuing is definitely wrong.
> So expect a follow-up patch to drop the packet when encap_type is
> unknown in esp_input.

Here is the patch to drop the packet if encap_type is unknown.
I've also removed the other two bogus printk's as they cannot
occur (printing a message is the last thing you want to do even
if they did occur :).

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 886 bytes --]

===== net/ipv4/esp4.c 1.45 vs edited =====
--- 1.45/net/ipv4/esp4.c	2004-06-26 15:42:21 +10:00
+++ edited/net/ipv4/esp4.c	2004-06-26 19:57:22 +10:00
@@ -301,28 +301,14 @@
 			switch (decap->decap_type) {
 			case UDP_ENCAP_ESPINUDP:
 			case UDP_ENCAP_ESPINUDP_NON_IKE:
-
-				if ((void*)uh == (void*)esph) {
-					printk(KERN_DEBUG
-					       "esp_input(): Got ESP; expecting ESPinUDP\n");
-					break;
-				}
-
 				encap_data->proto = AF_INET;
 				encap_data->saddr.a4 = iph->saddr;
 				encap_data->sport = uh->source;
 				encap_len = (void*)esph - (void*)uh;
-				if (encap_len != sizeof(*uh))
-				  printk(KERN_DEBUG
-					 "esp_input(): UDP -> ESP: too much room: %d\n",
-					 encap_len);
 				break;
 
 			default:
-				printk(KERN_INFO
-			       "esp_input(): processing unknown encap type: %u\n",
-				       decap->decap_type);
-				break;
+				goto out;
 			}
 		}
 

Re: [IPSEC] Drop bogus NAT-T printks in esp_input

[David S. Miller]

On Sat, 26 Jun 2004 20:07:18 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Sat, Jun 26, 2004 at 11:57:15AM +1000, herbert wrote:
> > 
> > However, printing a warning and then continuing is definitely wrong.
> > So expect a follow-up patch to drop the packet when encap_type is
> > unknown in esp_input.
> 
> Here is the patch to drop the packet if encap_type is unknown.
> I've also removed the other two bogus printk's as they cannot
> occur (printing a message is the last thing you want to do even
> if they did occur :).

Also applied, thanks.

Re: [IPSEC] Remove run-time encap_type checks in esp4

[David S. Miller]

On Sat, 26 Jun 2004 11:57:15 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> This allows us to remove all the per-packet checks on x->encap->encap_type.
> I've left the check in esp_input just in case someone adds a non-ESP encap
> type in future.

Applied.

Re: [UDP] Check encap_type at config time

[David S. Miller]

On Fri, 25 Jun 2004 22:11:47 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> The following patch moves the udp->encap_type check from the per-packet
> hot-path into udp_setsockopt().
> 
> As a consequence, this allows user space to detect whether the kernel
> actually supports the encap type that they're requesting.  Pity no one
> did this before the NON-IKE patch was applied.  As it is there is no
> easy way to detect whether NON-IKE support is present.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, looks great.