Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 30 Nov 2020 09:45:44 +0000 (UTC)
From:      Michael Tuexen <tuexen@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r368181 - in stable/12/sys/netinet: . tcp_stacks
Message-ID:  <202011300945.0AU9jilR008960@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: tuexen
Date: Mon Nov 30 09:45:44 2020
New Revision: 368181
URL: https://svnweb.freebsd.org/changeset/base/368181

Log:
  MFC r367530:
  RFC 7323 specifies that:
  * TCP segments without timestamps should be dropped when support for
    the timestamp option has been negotiated.
  * TCP segments with timestamps should be processed normally if support
    for the timestamp option has not been negotiated.
  This patch enforces the above.
  Manually resolved merge conflicts.
  
  MFC 367891:
  Fix an issue I introuced in r367530: tcp_twcheck() can be called
  with to == NULL for SYN segments. So don't assume tp != NULL.
  Thanks to jhb@ for reporting and suggesting a fix.
  
  MFC r367946:
  Fix two occurences of a typo in a comment introduced in r367530.
  Thanks to lstewart@ for reporting them.
  
  PR:			250499
  Reviewed by:		gnn, rrs
  Sponsored by:		Netflix, Inc.
  Differential Revision:	https://reviews.freebsd.org/D27148

Modified:
  stable/12/sys/netinet/tcp_input.c
  stable/12/sys/netinet/tcp_stacks/rack.c
  stable/12/sys/netinet/tcp_syncache.c
  stable/12/sys/netinet/tcp_timewait.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/netinet/tcp_input.c
==============================================================================
--- stable/12/sys/netinet/tcp_input.c	Mon Nov 30 09:22:33 2020	(r368180)
+++ stable/12/sys/netinet/tcp_input.c	Mon Nov 30 09:45:44 2020	(r368181)
@@ -975,8 +975,8 @@ findpcb:
 		}
 		INP_INFO_RLOCK_ASSERT(&V_tcbinfo);
 
-		if (thflags & TH_SYN)
-			tcp_dooptions(&to, optp, optlen, TO_SYN);
+		tcp_dooptions(&to, optp, optlen,
+		    (thflags & TH_SYN) ? TO_SYN : 0);
 		/*
 		 * NB: tcp_twcheck unlocks the INP and frees the mbuf.
 		 */
@@ -1706,20 +1706,29 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, stru
 	}
 
 	/*
-	 * If timestamps were negotiated during SYN/ACK they should
-	 * appear on every segment during this session and vice versa.
+	 * If timestamps were negotiated during SYN/ACK and a
+	 * segment without a timestamp is received, silently drop
+	 * the segment.
+	 * See section 3.2 of RFC 7323.
 	 */
 	if ((tp->t_flags & TF_RCVD_TSTMP) && !(to.to_flags & TOF_TS)) {
 		if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
 			log(LOG_DEBUG, "%s; %s: Timestamp missing, "
-			    "no action\n", s, __func__);
+			    "segment silently dropped\n", s, __func__);
 			free(s, M_TCPLOG);
 		}
+		goto drop;
 	}
+	/*
+	 * If timestamps were not negotiated during SYN/ACK and a
+	 * segment with a timestamp is received, ignore the
+	 * timestamp and process the packet normally.
+	 * See section 3.2 of RFC 7323.
+	 */
 	if (!(tp->t_flags & TF_RCVD_TSTMP) && (to.to_flags & TOF_TS)) {
 		if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
 			log(LOG_DEBUG, "%s; %s: Timestamp not expected, "
-			    "no action\n", s, __func__);
+			    "segment processed normally\n", s, __func__);
 			free(s, M_TCPLOG);
 		}
 	}

Modified: stable/12/sys/netinet/tcp_stacks/rack.c
==============================================================================
--- stable/12/sys/netinet/tcp_stacks/rack.c	Mon Nov 30 09:22:33 2020	(r368180)
+++ stable/12/sys/netinet/tcp_stacks/rack.c	Mon Nov 30 09:45:44 2020	(r368181)
@@ -6708,7 +6708,27 @@ rack_hpts_do_segment(struct mbuf *m, struct tcphdr *th
 		TCP_LOG_EVENT(tp, th, &so->so_rcv, &so->so_snd, TCP_LOG_IN, 0,
 		    tlen, &log, true);
 	}
+
 	/*
+	 * Parse options on any incoming segment.
+	 */
+	tcp_dooptions(&to, (u_char *)(th + 1),
+	    (th->th_off << 2) - sizeof(struct tcphdr),
+	    (thflags & TH_SYN) ? TO_SYN : 0);
+
+	/*
+	 * If timestamps were negotiated during SYN/ACK and a
+	 * segment without a timestamp is received, silently drop
+	 * the segment.
+	 * See section 3.2 of RFC 7323.
+	 */
+	if ((tp->t_flags & TF_RCVD_TSTMP) && !(to.to_flags & TOF_TS)) {
+		way_out = 5;
+		retval = 0;
+		goto done_with_input;
+	}
+
+	/*
 	 * Segment received on connection. Reset idle time and keep-alive
 	 * timer. XXX: This should be done after segment validation to
 	 * ignore broken/spoofed segs.
@@ -6761,12 +6781,6 @@ rack_hpts_do_segment(struct mbuf *m, struct tcphdr *th
 			rack_cong_signal(tp, th, CC_ECN);
 		}
 	}
-	/*
-	 * Parse options on any incoming segment.
-	 */
-	tcp_dooptions(&to, (u_char *)(th + 1),
-	    (th->th_off << 2) - sizeof(struct tcphdr),
-	    (thflags & TH_SYN) ? TO_SYN : 0);
 
 	/*
 	 * If echoed timestamp is later than the current time, fall back to
@@ -6898,6 +6912,7 @@ rack_hpts_do_segment(struct mbuf *m, struct tcphdr *th
 			rack_timer_audit(tp, rack, &so->so_snd);
 			way_out = 2;
 		}
+	done_with_input:
 		rack_log_doseg_done(rack, cts, nxt_pkt, did_out, way_out);
 		if (did_out)
 			rack->r_wanted_output = 0;

Modified: stable/12/sys/netinet/tcp_syncache.c
==============================================================================
--- stable/12/sys/netinet/tcp_syncache.c	Mon Nov 30 09:22:33 2020	(r368180)
+++ stable/12/sys/netinet/tcp_syncache.c	Mon Nov 30 09:45:44 2020	(r368181)
@@ -1142,6 +1142,40 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt
 		}
 
 		/*
+		 * If timestamps were not negotiated during SYN/ACK and a
+		 * segment with a timestamp is received, ignore the
+		 * timestamp and process the packet normally.
+		 * See section 3.2 of RFC 7323.
+		 */
+		if (!(sc->sc_flags & SCF_TIMESTAMP) &&
+		    (to->to_flags & TOF_TS)) {
+			if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
+				log(LOG_DEBUG, "%s; %s: Timestamp not "
+				    "expected, segment processed normally\n",
+				    s, __func__);
+				free(s, M_TCPLOG);
+				s = NULL;
+			}
+		}
+
+		/*
+		 * If timestamps were negotiated during SYN/ACK and a
+		 * segment without a timestamp is received, silently drop
+		 * the segment.
+		 * See section 3.2 of RFC 7323.
+		 */
+		if ((sc->sc_flags & SCF_TIMESTAMP) &&
+		    !(to->to_flags & TOF_TS)) {
+			SCH_UNLOCK(sch);
+			if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
+				log(LOG_DEBUG, "%s; %s: Timestamp missing, "
+				    "segment silently dropped\n", s, __func__);
+				free(s, M_TCPLOG);
+			}
+			return (-1);  /* Do not send RST */
+		}
+
+		/*
 		 * Pull out the entry to unlock the bucket row.
 		 * 
 		 * NOTE: We must decrease TCPS_SYN_RECEIVED count here, not
@@ -1184,32 +1218,6 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt
 			log(LOG_DEBUG, "%s; %s: SEQ %u != IRS+1 %u, segment "
 			    "rejected\n", s, __func__, th->th_seq, sc->sc_irs);
 		goto failed;
-	}
-
-	/*
-	 * If timestamps were not negotiated during SYN/ACK they
-	 * must not appear on any segment during this session.
-	 */
-	if (!(sc->sc_flags & SCF_TIMESTAMP) && (to->to_flags & TOF_TS)) {
-		if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
-			log(LOG_DEBUG, "%s; %s: Timestamp not expected, "
-			    "segment rejected\n", s, __func__);
-		goto failed;
-	}
-
-	/*
-	 * If timestamps were negotiated during SYN/ACK they should
-	 * appear on every segment during this session.
-	 * XXXAO: This is only informal as there have been unverified
-	 * reports of non-compliants stacks.
-	 */
-	if ((sc->sc_flags & SCF_TIMESTAMP) && !(to->to_flags & TOF_TS)) {
-		if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
-			log(LOG_DEBUG, "%s; %s: Timestamp missing, "
-			    "no action\n", s, __func__);
-			free(s, M_TCPLOG);
-			s = NULL;
-		}
 	}
 
 	*lsop = syncache_socket(sc, *lsop, m);

Modified: stable/12/sys/netinet/tcp_timewait.c
==============================================================================
--- stable/12/sys/netinet/tcp_timewait.c	Mon Nov 30 09:22:33 2020	(r368180)
+++ stable/12/sys/netinet/tcp_timewait.c	Mon Nov 30 09:45:44 2020	(r368181)
@@ -373,9 +373,10 @@ tcp_twstart(struct tcpcb *tp)
 /*
  * Returns 1 if the TIME_WAIT state was killed and we should start over,
  * looking for a pcb in the listen state.  Returns 0 otherwise.
+ * It be called with to == NULL only for pure SYN-segments.
  */
 int
-tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unused, struct tcphdr *th,
+tcp_twcheck(struct inpcb *inp, struct tcpopt *to, struct tcphdr *th,
     struct mbuf *m, int tlen)
 {
 	struct tcptw *tw;
@@ -396,6 +397,8 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unu
 		goto drop;
 
 	thflags = th->th_flags;
+	KASSERT(to != NULL || (thflags & (TH_SYN | TH_ACK)) == TH_SYN,
+	        ("tcp_twcheck: called without options on a non-SYN segment"));
 
 	/*
 	 * NOTE: for FIN_WAIT_2 (to be added later),
@@ -443,6 +446,16 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unu
 	 */
 	if ((thflags & TH_ACK) == 0)
 		goto drop;
+
+	/*
+	 * If timestamps were negotiated during SYN/ACK and a
+	 * segment without a timestamp is received, silently drop
+	 * the segment.
+	 * See section 3.2 of RFC 7323.
+	 */
+	if (((to->to_flags & TOF_TS) == 0) && (tw->t_recent != 0)) {
+		goto drop;
+	}
 
 	/*
 	 * Reset the 2MSL timer if this is a duplicate FIN.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202011300945.0AU9jilR008960>