Subject: Re: bin/30437: recent NATT changes breaks racoon
To: None <manu@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org>
From: Emmanuel Dreyfus <manu@netbsd.org>
List: netbsd-bugs
Date: 09/26/2005 17:32:02
The following reply was made to PR bin/30437; it has been noted by GNATS.
From: Emmanuel Dreyfus <manu@netbsd.org>
To: jeffi@rcn.com, spz@serpens.de, tron@zhadum.de,
gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/30437: recent NATT changes breaks racoon
Date: Mon, 26 Sep 2005 17:31:50 +0000
--DO5DiztRLs659m5i
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Here is an updated kernel patch that fixes the problem at mine.
I have to check that NAT-T still works with that patch, and even
with multiple endpoints behind the NAT. Help is welcome as I currently
lack the hardware.
--
Emmanuel Dreyfus
manu@netbsd.org
--DO5DiztRLs659m5i
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="no-nat-t.diff"
Index: key.c
===================================================================
RCS file: /cvsroot/src/sys/netkey/key.c,v
retrieving revision 1.135
diff -U2 -r1.135 key.c
--- key.c 3 Jul 2005 22:57:09 -0000 1.135
+++ key.c 26 Sep 2005 17:24:35 -0000
@@ -387,8 +387,10 @@
static struct mbuf *key_setsadbxport __P((u_int16_t, u_int16_t));
static struct mbuf *key_setsadbxtype __P((u_int16_t));
+#endif
static void key_porttosaddr __P((struct sockaddr *, u_int16_t));
#define KEY_PORTTOSADDR(saddr, port) \
- key_porttosaddr((struct sockaddr *)(saddr), (port))
-#endif
+ key_porttosaddr((struct sockaddr *)(saddr), (port))
+static int key_checksalen __P((const struct sockaddr *));
+#define KEY_CHECKSALEN(saddr) key_checksalen((const struct sockaddr *)(saddr))
static struct mbuf *key_setsadblifetime __P((u_int16_t, u_int32_t,
u_int64_t, u_int64_t, u_int64_t));
@@ -804,5 +806,7 @@
bcopy(src, &sin.sin_addr,
sizeof(sin.sin_addr));
+#ifdef IPSEC_NAT_T
sin.sin_port = sport;
+#endif
if (key_sockaddrcmp((struct sockaddr*)&sin,
(struct sockaddr *)&sav->sah->saidx.src,
@@ -817,5 +821,7 @@
bcopy(src, &sin6.sin6_addr,
sizeof(sin6.sin6_addr));
+#ifdef IPSEC_NAT_T
sin6.sin6_port = sport;
+#endif
if (IN6_IS_SCOPE_LINKLOCAL(&sin6.sin6_addr)) {
/* kame fake scopeid */
@@ -845,5 +851,7 @@
bcopy(dst, &sin.sin_addr,
sizeof(sin.sin_addr));
+#ifdef IPSEC_NAT_T
sin.sin_port = dport;
+#endif
if (key_sockaddrcmp((struct sockaddr*)&sin,
(struct sockaddr *)&sav->sah->saidx.dst,
@@ -858,5 +866,7 @@
bcopy(dst, &sin6.sin6_addr,
sizeof(sin6.sin6_addr));
+#ifdef IPSEC_NAT_T
sin6.sin6_port = dport;
+#endif
if (IN6_IS_SCOPE_LINKLOCAL(&sin6.sin6_addr)) {
/* kame fake scopeid */
@@ -4039,5 +4049,5 @@
return port;
}
-
+#endif /* IPSEC_NAT_T */
/*
@@ -4065,5 +4075,6 @@
#endif
default:
- printf("key_porttosaddr: unexpected address family\n");
+ printf("key_porttosaddr: unexpected address family %d\n",
+ saddr->sa_family);
break;
}
@@ -4071,5 +4082,32 @@
return;
}
-#endif /* IPSEC_NAT_T */
+
+/*
+ * Safety check sa_len
+ */
+static int
+key_checksalen(saddr)
+ const struct sockaddr *saddr;
+{
+ switch (saddr->sa_family) {
+ case AF_INET:
+ if (saddr->sa_len != sizeof(struct sockaddr_in))
+ return -1;
+ break;
+#ifdef INET6
+ case AF_INET6:
+ if (saddr->sa_len != sizeof(struct sockaddr_in6))
+ return -1;
+ break;
+#endif
+ default:
+ printf("key_checksalen: unexpected sa_family %d\n",
+ saddr->sa_family);
+ return -1;
+ break;
+ }
+
+ return 0;
+}
/*
@@ -5029,52 +5067,18 @@
}
- /*
- * make sure if port number is zero.
- * If using NAT-T, skip that check.
- */
- switch (((struct sockaddr *)(src0 + 1))->sa_family) {
- case AF_INET:
- if (((struct sockaddr *)(src0 + 1))->sa_len !=
- sizeof(struct sockaddr_in))
- return key_senderror(so, m, EINVAL);
-#ifndef IPSEC_NAT_T
- ((struct sockaddr_in *)(src0 + 1))->sin_port = 0;
-#endif
- break;
- case AF_INET6:
- if (((struct sockaddr *)(src0 + 1))->sa_len !=
- sizeof(struct sockaddr_in6))
- return key_senderror(so, m, EINVAL);
-#ifndef IPSEC_NAT_T
- ((struct sockaddr_in6 *)(src0 + 1))->sin6_port = 0;
-#endif
- break;
- default:
- ; /*???*/
- }
- switch (((struct sockaddr *)(dst0 + 1))->sa_family) {
- case AF_INET:
- if (((struct sockaddr *)(dst0 + 1))->sa_len !=
- sizeof(struct sockaddr_in))
- return key_senderror(so, m, EINVAL);
-#ifndef IPSEC_NAT_T
- ((struct sockaddr_in *)(dst0 + 1))->sin_port = 0;
-#endif
- break;
- case AF_INET6:
- if (((struct sockaddr *)(dst0 + 1))->sa_len !=
- sizeof(struct sockaddr_in6))
- return key_senderror(so, m, EINVAL);
-#ifndef IPSEC_NAT_T
- ((struct sockaddr_in6 *)(dst0 + 1))->sin6_port = 0;
-#endif
- break;
- default:
- ; /*???*/
- }
+ /* sa_len safety check */
+ if (KEY_CHECKSALEN(src0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+ if (KEY_CHECKSALEN(dst0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
- /* XXX boundary check against sa_len */
KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
+ /* If not using NAT-T, make sure port numbers are set to zero. */
+#ifndef IPSEC_NAT_T
+ KEY_PORTTOSADDR(&saidx.src, 0);
+ KEY_PORTTOSADDR(&saidx.dst, 0);
+#endif
+
/* SPI allocation */
spi = key_do_getnewspi((struct sadb_spirange *)mhp->ext[SADB_EXT_SPIRANGE],
@@ -5327,7 +5331,18 @@
dst0 = (struct sadb_address *)(mhp->ext[SADB_EXT_ADDRESS_DST]);
- /* XXX boundary check against sa_len */
+ /* sa_len safety check */
+ if (KEY_CHECKSALEN(src0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+ if (KEY_CHECKSALEN(dst0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+
KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
+ /* If not using NAT-T, make sure if port number is zero. */
+#ifndef IPSEC_NAT_T
+ KEY_PORTTOSADDR(&saidx.src, 0);
+ KEY_PORTTOSADDR(&saidx.dst, 0);
+#endif
+
/* get a SA header */
if ((sah = key_getsah(&saidx)) == NULL) {
@@ -5577,7 +5592,18 @@
dst0 = (struct sadb_address *)mhp->ext[SADB_EXT_ADDRESS_DST];
- /* XXX boundary check against sa_len */
+ /* sa_len safety check */
+ if (KEY_CHECKSALEN(src0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+ if (KEY_CHECKSALEN(dst0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+
KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
+ /* If not using NAT-T, make sure if port number is zero. */
+#ifndef IPSEC_NAT_T
+ KEY_PORTTOSADDR(&saidx.src, 0);
+ KEY_PORTTOSADDR(&saidx.dst, 0);
+#endif
+
/* get a SA header */
if ((newsah = key_getsah(&saidx)) == NULL) {
@@ -5866,7 +5892,18 @@
dst0 = (struct sadb_address *)(mhp->ext[SADB_EXT_ADDRESS_DST]);
- /* XXX boundary check against sa_len */
+ /* sa_len safety check */
+ if (KEY_CHECKSALEN(src0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+ if (KEY_CHECKSALEN(dst0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+
KEY_SETSECASIDX(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
+ /* If not using NAT-T, make sure if port number is zero. */
+#ifndef IPSEC_NAT_T
+ KEY_PORTTOSADDR(&saidx.src, 0);
+ KEY_PORTTOSADDR(&saidx.dst, 0);
+#endif
+
/* get a SA header */
LIST_FOREACH(sah, &sahtree, chain) {
@@ -5933,7 +5970,18 @@
dst0 = (struct sadb_address *)(mhp->ext[SADB_EXT_ADDRESS_DST]);
- /* XXX boundary check against sa_len */
+ /* sa_len safety check */
+ if (KEY_CHECKSALEN(src0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+ if (KEY_CHECKSALEN(dst0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+
KEY_SETSECASIDX(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
+ /* If not using NAT-T, make sure if port number is zero. */
+#ifndef IPSEC_NAT_T
+ KEY_PORTTOSADDR(&saidx.src, 0);
+ KEY_PORTTOSADDR(&saidx.dst, 0);
+#endif
+
LIST_FOREACH(sah, &sahtree, chain) {
if (sah->state == SADB_SASTATE_DEAD)
@@ -6042,7 +6090,18 @@
dst0 = (struct sadb_address *)mhp->ext[SADB_EXT_ADDRESS_DST];
- /* XXX boundary check against sa_len */
+ /* sa_len safety check */
+ if (KEY_CHECKSALEN(src0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+ if (KEY_CHECKSALEN(dst0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+
KEY_SETSECASIDX(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
+ /* If not using NAT-T, make sure if port number is zero. */
+#ifndef IPSEC_NAT_T
+ KEY_PORTTOSADDR(&saidx.src, 0);
+ KEY_PORTTOSADDR(&saidx.dst, 0);
+#endif
+
/* get a SA header */
LIST_FOREACH(sah, &sahtree, chain) {
@@ -6733,7 +6792,18 @@
dst0 = (struct sadb_address *)mhp->ext[SADB_EXT_ADDRESS_DST];
- /* XXX boundary check against sa_len */
+ /* sa_len safety check */
+ if (KEY_CHECKSALEN(src0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+ if (KEY_CHECKSALEN(dst0 + 1) != 0)
+ return key_senderror(so, m, EINVAL);
+
KEY_SETSECASIDX(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
+ /* If not using NAT-T, make sure if port number is zero. */
+#ifndef IPSEC_NAT_T
+ KEY_PORTTOSADDR(&saidx.src, 0);
+ KEY_PORTTOSADDR(&saidx.dst, 0);
+#endif
+
/* get a SA index */
LIST_FOREACH(sah, &sahtree, chain) {
--DO5DiztRLs659m5i--