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>