Patch to fix the same problem as FreeBSD-SA-05:15.tcp

[Noritoshi Demizu <demizu@xxxxxxxxxxxxxx>]

Attached is a patch to fix the same problem described in FreeBSD-SA-05:15.tcp.
(ftp://ftp.freebsd.org/pub/FreeBSD/CERT/advisories/FreeBSD-SA-05:15.tcp.asc )

Quoted from FreeBSD-SA-05:15.tcp.
 > II.  Problem Description
 >
 > Two problems have been discovered in the FreeBSD TCP stack.
 >
 > First, when a TCP packets containing a timestamp is received, inadequate
 > checking of sequence numbers is performed, allowing an attacker to
 > artificially increase the internal "recent" timestamp for a connection.
 >
 > Second, a TCP packet with the SYN flag set is accepted for established
 > connections, allowing an attacker to overwrite certain TCP options.

The patch below is basically ported from FreeBSD.
The first chunk of the patch solves the second problem.
The rest of the patch solves the first problem.

I have tested the patch.  First, I observed that DragonFlyBSD-current
has the two problems above.  Then, I applied the patch below and
saw that the problems were fixed by the patch.

Regards,
Noritoshi Demizu


Index: netinet/tcp_input.c
===================================================================
RCS file: /home/cvsup/DragonFlyBSD/dcvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.60
diff -u -r1.60 tcp_input.c
--- netinet/tcp_input.c	10 May 2005 15:48:10 -0000	1.60
+++ netinet/tcp_input.c	1 Jul 2005 10:33:50 -0000
@@ -1078,7 +1078,7 @@
 	 * XXX this is tradtitional behavior, may need to be cleaned up.
 	 */
 	tcp_dooptions(&to, optp, optlen, (thflags & TH_SYN) != 0);
-	if (thflags & TH_SYN) {
+	if (tp->t_state == TCPS_SYN_SENT && (thflags & TH_SYN)) {
 		if (to.to_flags & TOF_SCALE) {
 			tp->t_flags |= TF_RCVD_SCALE;
 			tp->requested_s_scale = to.to_requested_s_scale;
@@ -1795,10 +1795,25 @@
 	/*
 	 * If last ACK falls within this segment's sequence numbers,
 	 * record its timestamp.
-	 * NOTE that the test is modified according to the latest
-	 * proposal of the tcplw@xxxxxxxx list (Braden 1993/04/26).
-	 */
-	if ((to.to_flags & TOF_TS) && SEQ_LEQ(th->th_seq, tp->last_ack_sent)) {
+	 * NOTE:
+	 * 1) That the test incorporates suggestions from the latest
+	 *    proposal of the tcplw@xxxxxxxx list (Braden 1993/04/26).
+	 * 2) That updating only on newer timestamps interferes with
+	 *    our earlier PAWS tests, so this check should be solely
+	 *    predicated on the sequence space of this segment.
+	 * 3) That we modify the segment boundary check to be
+	 *        Last.ACK.Sent <= SEG.SEQ + SEG.LEN
+	 *    instead of RFC1323's
+	 *        Last.ACK.Sent < SEG.SEQ + SEG.LEN,
+	 *    This modified check allows us to overcome RFC1323's
+	 *    limitations as described in Stevens TCP/IP Illustrated
+	 *    Vol. 2 p.869. In such cases, we can still calculate the
+	 *    RTT correctly when RCV.NXT == Last.ACK.Sent.
+	 */
+	if ((to.to_flags & TOF_TS) && SEQ_LEQ(th->th_seq, tp->last_ack_sent) &&
+	    SEQ_LEQ(tp->last_ack_sent, (th->th_seq + tlen
+					+ ((thflags & TH_SYN) != 0)
+					+ ((thflags & TH_FIN) != 0)))) {
 		tp->ts_recent_age = ticks;
 		tp->ts_recent = to.to_tsval;
 	}
@@ -2678,6 +2693,12 @@
 			to->to_tsval = ntohl(to->to_tsval);
 			bcopy(cp + 6, &to->to_tsecr, sizeof to->to_tsecr);
 			to->to_tsecr = ntohl(to->to_tsecr);
+			/*
+			 * If echoed timestamp is later than the current time,
+			 * fall back to non RFC1323 RTT calculation.
+			 */
+			if (to->to_tsecr != 0 && TSTMP_GT(to->to_tsecr, ticks))
+				to->to_tsecr = 0;
 			break;
 		case TCPOPT_CC:
 			if (optlen != TCPOLEN_CC)
Index: netinet/tcp_seq.h
===================================================================
RCS file: /home/cvsup/DragonFlyBSD/dcvs/src/sys/netinet/tcp_seq.h,v
retrieving revision 1.7
diff -u -r1.7 tcp_seq.h
--- netinet/tcp_seq.h	21 Dec 2004 02:54:15 -0000	1.7
+++ netinet/tcp_seq.h	1 Jul 2005 10:33:50 -0000
@@ -111,6 +111,7 @@
 
 /* for modulo comparisons of timestamps */
 #define TSTMP_LT(a,b)	((int)((a)-(b)) < 0)
+#define TSTMP_GT(a,b)	((int)((a)-(b)) > 0)
 #define TSTMP_GEQ(a,b)	((int)((a)-(b)) >= 0)
 
 /*