The following reply was made to PR bin/30437; it has been noted by GNATS.

From: "S.P.Zeidler" <spz@serpens.de>
To: Emmanuel Dreyfus <manu@netbsd.org>
Cc: gnats-bugs@netbsd.org
Subject: Re: bin/30437: recent NATT changes breaks racoon
Date: Sat, 1 Oct 2005 15:26:18 +0200

 Hi,
 
 -current as of 27th September, the patch seems to make stuff work
 (at least my laptop does have a connection :)
 
 regards,
 	spz
 
 Thus wrote Emmanuel Dreyfus (manu@netbsd.org):
 
 > 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
 
 > 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) {
 
 
 -- 
 spz@serpens.de (S.P.Zeidler)