>Number:         26062
>Category:       kern
>Synopsis:       TCP_SIGNATURE code causes crashes and doesn't work
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jun 25 23:01:01 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator:     Jeff Rizzo
>Release:        NetBSD 2.0F (source date 20040625)
>Organization:
Jeff Rizzo                                         http://www.redcrowgroup.com/
>Environment:
System: NetBSD md5test.boogers.sf.ca.us 2.0F NetBSD 2.0F (MD5TEST) #14: Fri Jun 25 15:28:49 PDT 2004  riz@lychee.tastylime.net:/usr/src-current/sys/arch/i386/compile/MD5TEST i386
Architecture: i386
Machine: i386
>Description:
	There are a few places where, if options TCP_SIGNATURE
	is enabled, key_freesav() will be called with a NULL
	pointer, causing a panic.  In addition, the tcp_signature()
	function doesn't correctly ignore the TCP options when
	making the MD5 hash as required by RFC2385, so without the
	patch I've appended, NetBSD won't interoperate with MD5
	passwords on commercial routers.  
	
	In addition, it appears the line:

	tb.t_flags = tcp_do_rfc1323 ? (TF_REQ_SCALE|TF_REQ_TSTMP) : 0;

	was inadvertantly removed in revision 1.206, which would
	cause problems even if TCP_SIGNATURE was *not* enabled.

	With the appended patch, I was able to get a patched quagga
	BGP daemon (using Bruce M. Simpson's patches for quagga)
	running on NetBSD to successfully peer with a Cisco router
	using RFC2835 passwords.

	Bruce M. Simpson's patches for quagga are available here:
	http://people.freebsd.org/~bms/dump/quagga-tcpmd5/
>How-To-Repeat:
	compile a kernel with options TCP_SIGNATURE and either options
	IPSEC or options FAST_IPSEC.  Watch the machine crash if
	it gets packets with an MD5 signature if no SA is configured.

	Even if the SA is properly configured, and the quagga BGP
	daemon has been patched for RFC2385 support, it will not
	interoperate with (for example) a Cisco.
>Fix:
	Apply the following patch to sys/netinet/tcp_input.c :

Index: tcp_input.c
===================================================================
RCS file: /usr/mirror/main/src/sys/netinet/tcp_input.c,v
retrieving revision 1.207
diff -u -r1.207 tcp_input.c
--- tcp_input.c	23 May 2004 00:37:27 -0000	1.207
+++ tcp_input.c	25 Jun 2004 22:07:22 -0000
@@ -2630,11 +2630,13 @@
 	struct ippseudo ippseudo;
 	struct ip6_hdr_pseudo ip6pseudo;
 	struct tcphdr th0;
-	int l;
+	int l, tcphdrlen;
 
 	if (sav == NULL)
 		return (-1);
 
+	tcphdrlen = th->th_off * 4;
+
 	switch (mtod(m, struct ip *)->ip_v) {
 	case 4:
 		ip = mtod(m, struct ip *);
@@ -2674,10 +2676,10 @@
 	th0.th_sum = 0;
 	MD5Update(&ctx, (char *)&th0, sizeof(th0));
 
-	l = m->m_pkthdr.len - thoff - sizeof(struct tcphdr);
+	l = m->m_pkthdr.len - thoff - tcphdrlen;
 	if (l > 0)
-		m_apply(m, thoff + sizeof(struct tcphdr),
-		    m->m_pkthdr.len - thoff - sizeof(struct tcphdr),
+		m_apply(m, thoff + tcphdrlen,
+		    m->m_pkthdr.len - thoff - tcphdrlen,
 		    tcp_signature_apply, &ctx);
 
 	MD5Update(&ctx, _KEYBUF(sav->key_auth), _KEYLEN(sav->key_auth));
@@ -2834,6 +2836,8 @@
 	}
 
 	if ((sigp ? TF_SIGNATURE : 0) ^ (tp->t_flags & TF_SIGNATURE)) {
+		if (sav == NULL)
+			return (-1);
 #ifdef FAST_IPSEC
 		KEY_FREESAV(&sav);
 #else
@@ -2848,6 +2852,8 @@
 		TCP_FIELDS_TO_NET(th);
 		if (tcp_signature(m, th, toff, sav, sig) < 0) {
 			TCP_FIELDS_TO_HOST(th);
+			if (sav == NULL)
+				return (-1);
 #ifdef FAST_IPSEC
 			KEY_FREESAV(&sav);
 #else
@@ -2859,6 +2865,8 @@
 
 		if (bcmp(sig, sigp, TCP_SIGLEN)) {
 			tcpstat.tcps_badsig++;
+			if (sav == NULL)
+				return (-1);
 #ifdef FAST_IPSEC
 			KEY_FREESAV(&sav);
 #else
@@ -3798,6 +3806,10 @@
 	if (optp)
 #endif
 	{
+		tb.t_flags = tcp_do_rfc1323 ? (TF_REQ_SCALE|TF_REQ_TSTMP) : 0;
+#ifdef TCP_SIGNATURE
+		tb.t_flags |= (tp->t_flags & TF_SIGNATURE);
+#endif
 		if (tcp_dooptions(&tb, optp, optlen, th, m, m->m_pkthdr.len -
 		    sizeof(struct tcphdr) - optlen - hlen, oi) < 0)
 			return (0);
>Release-Note:
>Audit-Trail:
>Unformatted: