From 19bc77c54956d279f738e58badc8d013ece871cc Mon Sep 17 00:00:00 2001 From: Andre Oppermann Date: Sat, 28 Jul 2007 11:51:44 +0000 Subject: [PATCH] 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) --- sys/netinet/tcp_input.c | 20 ++++-------------- sys/netinet/tcp_syncache.c | 42 +++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index 4fd115ff220..81918e0235b 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 c79e68ab898..40084ca33cf 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); }