tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: UDP_ENCAP_ESPINUDP_NON_IKE



I have written a patch to remove UDP_ENCAP_ESPINUDP_NON_IKE from NetBSD, which is attached to this message. The patch is for the netbsd-8 branch.

I have tested the kernel produced by this patch with my NetBSD 7.1 system and it seems to work as designed.

Chuck


On 05/17/2018 02:16 AM, Maxime Villard wrote:
I think it would be good to retire UDP_ENCAP_ESPINUDP_NON_IKE. It is not
part of any RFC, it was just introduced in a draft, and then subsequently
removed from that draft.

RFC3948 makes authority, and it documents only the Non-ESP marker - that
is to say, UDP_ENCAP_ESPINUDP.

I think we are not supposed to support undocumented options.

FreeBSD did the same, see [1]. OpenBSD has never had that, it seems.

The reason I'm bringing this, is because there appears to be a bug with
UDP_ENCAP_ESPINUDP_NON_IKE, in udp_usrreq.c:

1317         skip = sizeof(struct udphdr) + 2 * sizeof(uint32_t);
1318     }
1319
1320     /*
1321      * Get the UDP ports. They are handled in network order
1322      * everywhere in the IPSEC_NAT_T code.
1323      */
1324     udphdr = (struct udphdr *)((char *)data - skip);

Here we have:

    data = mtod(m) + IP_header_len + sizeof(struct udphdr)

So it is wrong to substract 'skip', because then 'udphdr' will point to
some data in the IP header. I'm not sure if it means the code has never
worked, or if I'm just completely misreading it.

What do you think? I asked Ryota and Kengo, they don't know. One concern is raccoon, because it supports each draft that led to the RFC [2]. To me this is wrong too, there shouldn't be support for temporary specs, that have no
meaning once the RFC is out.

Don't know.

Maxime

[1] http://freshbsd.org/commit/freebsd/r309808
[2] https://nxr.netbsd.org/xref/src/crypto/dist/ipsec-tools/src/racoon/vendorid.c#67


--- src/lib/libipsec/config.h	2012-01-04 11:09:42.000000000 -0500
+++ src/lib/libipsec/config.h	2018-05-26 13:27:41.782512252 -0400
@@ -17,7 +17,7 @@
 #define ENABLE_NATT 
 
 /* Enable NAT-Traversal draft 00 */
-#define ENABLE_NATT_00 
+/* #define ENABLE_NATT_00 */ 
 
 /* Enable NAT-Traversal draft 01 */
 /* #undef ENABLE_NATT_01 */
--- src/share/man/man4/udp.4	2012-11-12 00:13:28.000000000 -0500
+++ src/share/man/man4/udp.4	2018-05-26 11:26:46.346008025 -0400
@@ -111,13 +111,13 @@
 .Tn ESP
 packets in
 .Tn UDP .
-There are two valid encapsulation options:
-.Tn UDP_ENCAP_ESPINUDP_NON_IKE
-from draft-ietf-ipsec-nat-t-ike-00/01 and
+There is one valid encapsulation option:
 .Tn UDP_ENCAP_ESPINUDP
 from draft-ietf-ipsec-udp-encaps-06
 defined in
 .In netinet/udp.h .
+.Tn UDP_ENCAP_ESPINUDP_NON_IKE
+from draft-ietf-ipsec-nat-t-ike-00/01 is no longer supported.
 .Pp
 Options at the
 .Tn IP
--- src/sys/netipsec/ipsecif.c	2018-05-17 10:07:03.000000000 -0400
+++ src/sys/netipsec/ipsecif.c	2018-05-26 11:14:58.635469529 -0400
@@ -173,8 +173,7 @@
 	if (sav == NULL)
 		return 0;
 
-	if (!(sav->natt_type & UDP_ENCAP_ESPINUDP) &&
-	    !(sav->natt_type & UDP_ENCAP_ESPINUDP_NON_IKE)) {
+	if (!(sav->natt_type & UDP_ENCAP_ESPINUDP)) {
 		mtu = 0;
 		goto out;
 	}
--- src/sys/netipsec/ipsec_output.c	2018-05-22 15:29:57.347933000 -0400
+++ src/sys/netipsec/ipsec_output.c	2018-05-26 11:57:28.689152770 -0400
@@ -156,7 +156,6 @@
 #endif /* INET6 */
 	struct mbuf * mo;
 	struct udphdr *udp = NULL;
-	uint64_t * data = NULL;
 	int hlen, roff;
 
 	IPSEC_SPLASSERT_SOFTNET("ipsec_process_done");
@@ -172,7 +171,8 @@
 
 		hlen = sizeof(struct udphdr);
 		if (sav->natt_type == UDP_ENCAP_ESPINUDP_NON_IKE) 
-			hlen += sizeof(uint64_t);
+		printf("ipsec_process_done: unexpected protocol option: "
+			"UDP_ENCAP_ESPINUDP_NON_IKE.\n");
 
 		mo = m_makespace(m, sizeof(struct ip), hlen, &roff);
 		if (mo == NULL) {
@@ -186,15 +186,8 @@
 		}
 		
 		udp = (struct udphdr*) (mtod(mo, char*) + roff);
-		data = (uint64_t*) (udp + 1);
 
-		if (sav->natt_type == UDP_ENCAP_ESPINUDP_NON_IKE)
-			*data = 0; /* NON-IKE Marker */
-
-		if (sav->natt_type == UDP_ENCAP_ESPINUDP_NON_IKE)
-			udp->uh_sport = htons(UDP_ENCAP_ESPINUDP_PORT);
-		else
-			udp->uh_sport = key_portfromsaddr(&saidx->src);
+		udp->uh_sport = key_portfromsaddr(&saidx->src);
 		
 		udp->uh_dport = key_portfromsaddr(&saidx->dst);
 		udp->uh_sum = 0;
@@ -498,8 +491,7 @@
 	if (isr == isr->sp->req) { /* Check only if called from ipsec4_output */
 		KASSERT(mtu != NULL);
 		ip = mtod(m, struct ip *);
-		if (!(sav->natt_type &
-		    (UDP_ENCAP_ESPINUDP|UDP_ENCAP_ESPINUDP_NON_IKE))) {
+		if (!(sav->natt_type & UDP_ENCAP_ESPINUDP)) {
 			goto noneed;
 		}
 		if (ntohs(ip->ip_len) <= sav->esp_frag)
--- src/sys/netinet/udp_usrreq.c	2018-05-22 15:29:56.078274000 -0400
+++ src/sys/netinet/udp_usrreq.c	2018-05-26 11:58:48.772971707 -0400
@@ -606,7 +606,7 @@
 
 #ifdef IPSEC
 		/* Handle ESP over UDP */
-		if (inp->inp_flags & INP_ESPINUDP_ALL) {
+		if (inp->inp_flags & INP_ESPINUDP) {
 			struct sockaddr *sa = (struct sockaddr *)src;
 
 			switch(udp4_espinudp(mp, off, sa, inp->inp_socket)) {
@@ -749,7 +749,8 @@
 
 			case UDP_ENCAP_ESPINUDP_NON_IKE:
 				inp->inp_flags &= ~INP_ESPINUDP_ALL;
-				inp->inp_flags |= INP_ESPINUDP_NON_IKE;
+				/* Don't set unsupported option, report error */
+				error = ENOPROTOOPT;
 				break;
 			default:
 				error = EINVAL;
@@ -1307,13 +1308,10 @@
 	}
 
 	if (inp->inp_flags & INP_ESPINUDP_NON_IKE) {
-		u_int32_t *st = (u_int32_t *)data;
-
-		if ((len <= sizeof(u_int64_t) + sizeof(struct esp))
-		    || ((st[0] | st[1]) != 0))
-			return 0; /* Normal UDP processing */
-
-		skip = sizeof(struct udphdr) + sizeof(u_int64_t);
+		printf("udp4_espinudp: unexpected protocol option: "
+			"UDP_ENCAP_ESPINUDP_NON_IKE.\n");
+		m_freem(m);
+		return -1; /* Normal UDP processing */
 	}
 
 	/*


Home | Main Index | Thread Index | Old Index