o Move all detailed checks for RST in LISTEN state from tcp_input() to

  syncache_rst().
o Fix tests for flag combinations of RST and SYN, ACK, FIN.  Before
  a RST for a connection in syncache did not properly free the entry.
o Add more detailed logging.

Approved by:	re (rwatson)

2 files changed, 45 insertions, 17 deletions

diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 4fd115f..81918e0 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -648,24 +648,12 @@ findpcb:
 		 * Our (SYN|ACK) response was rejected.
 		 * Check with syncache and remove entry to prevent
 		 * retransmits.
-		 */
-		if ((thflags & (TH_ACK|TH_RST)) == (TH_ACK|TH_RST)) {
-			if ((s = tcp_log_addrs(&inc, th, NULL, NULL)))
-				log(LOG_DEBUG, "%s; %s: Listen socket: "
-				    "Our SYN|ACK was rejected, connection "
-				    "attempt aborted by remote endpoint\n",
-				    s, __func__);
-			syncache_chkrst(&inc, th);
-			goto dropunlock;
-		}
-		/*
-		 * Spurious RST.  Ignore.
+		 *
+		 * NB: syncache_chkrst does its own logging of failure
+		 * causes.
 		 */
 		if (thflags & TH_RST) {
-			if ((s = tcp_log_addrs(&inc, th, NULL, NULL)))
-				log(LOG_DEBUG, "%s; %s: Listen socket: "
-				    "Spurious RST, segment rejected\n",
-				    s, __func__);
+			syncache_chkrst(&inc, th);
 			goto dropunlock;
 		}
 		/*
diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c
index c79e68a..40084ca 100644
--- a/sys/netinet/tcp_syncache.c
+++ b/sys/netinet/tcp_syncache.c
@@ -473,11 +473,39 @@ syncache_chkrst(struct in_conninfo *inc, struct tcphdr *th)
 {
 	struct syncache *sc;
 	struct syncache_head *sch;
+	char *s = NULL;
 
 	sc = syncache_lookup(inc, &sch);	/* returns locked sch */
 	SCH_LOCK_ASSERT(sch);
-	if (sc == NULL)
+
+	/*
+	 * Any RST to our SYN|ACK must not carry ACK, SYN or FIN flags.
+	 * See RFC 793 page 65, section SEGMENT ARRIVES.
+	 */
+	if (th->th_flags & (TH_ACK|TH_SYN|TH_FIN)) {
+		if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
+			log(LOG_DEBUG, "%s; %s: Spurious RST with ACK, SYN or "
+			    "FIN flag set, segment ignored\n", s, __func__);
+		tcpstat.tcps_badrst++;
+		goto done;
+	}
+
+	/*
+	 * No corresponding connection was found in syncache.
+	 * If syncookies are enabled and possibly exclusively
+	 * used, or we are under memory pressure, a valid RST
+	 * may not find a syncache entry.  In that case we're
+	 * done and no SYN|ACK retransmissions will happen.
+	 * Otherwise the the RST was misdirected or spoofed.
+	 */
+	if (sc == NULL) {
+		if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
+			log(LOG_DEBUG, "%s; %s: Spurious RST without matching "
+			    "syncache entry (possibly syncookie only), "
+			    "segment ignored\n", s, __func__);
+		tcpstat.tcps_badrst++;
 		goto done;
+	}
 
 	/*
 	 * If the RST bit is set, check the sequence number to see
@@ -495,9 +523,21 @@ syncache_chkrst(struct in_conninfo *inc, struct tcphdr *th)
 	if (SEQ_GEQ(th->th_seq, sc->sc_irs) &&
 	    SEQ_LEQ(th->th_seq, sc->sc_irs + sc->sc_wnd)) {
 		syncache_drop(sc, sch);
+		if ((s = tcp_log_addrs(inc, th, NULL, NULL)))
+			log(LOG_DEBUG, "%s; %s: Our SYN|ACK was rejected, "
+			    "connection attempt aborted by remote endpoint\n",
+			    s, __func__);
 		tcpstat.tcps_sc_reset++;
+	} else if ((s = tcp_log_addrs(inc, th, NULL, NULL))) {
+		log(LOG_DEBUG, "%s; %s: RST with invalid SEQ %u != IRS %u "
+		    "(+WND %u), segment ignored\n",
+		    s, __func__, th->th_seq, sc->sc_irs, sc->sc_wnd);
+		tcpstat.tcps_badrst++;
 	}
+
 done:
+	if (s != NULL)
+		free(s, M_TCPLOG);
 	SCH_UNLOCK(sch);
 }