From 283c76c7c3f2f634f19f303a771a3f81fe890cab Mon Sep 17 00:00:00 2001 From: Michael Tuexen Date: Mon, 9 Nov 2020 21:49:40 +0000 Subject: [PATCH] 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. PR: 250499 Reviewed by: gnn, rrs MFC after: 1 week Sponsored by: Netflix, Inc Differential Revision: https://reviews.freebsd.org/D27148 --- sys/netinet/tcp_input.c | 21 ++++++++---- sys/netinet/tcp_stacks/bbr.c | 23 ++++++++++---- sys/netinet/tcp_stacks/rack.c | 28 ++++++++++++---- sys/netinet/tcp_syncache.c | 60 ++++++++++++++++++++--------------- sys/netinet/tcp_timewait.c | 12 ++++++- 5 files changed, 98 insertions(+), 46 deletions(-) diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index cb32e2f5d27c..a1ed0f7ea43c 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -977,8 +977,8 @@ tcp_input(struct mbuf **mp, int *offp, int proto) * XXXRW: It may be time to rethink timewait locking. */ if (inp->inp_flags & INP_TIMEWAIT) { - 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. */ @@ -1680,20 +1680,29 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so, } /* - * 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 without 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); } } diff --git a/sys/netinet/tcp_stacks/bbr.c b/sys/netinet/tcp_stacks/bbr.c index 5114ef4d2ba3..ccf138f70719 100644 --- a/sys/netinet/tcp_stacks/bbr.c +++ b/sys/netinet/tcp_stacks/bbr.c @@ -11430,12 +11430,6 @@ bbr_do_segment_nounlock(struct mbuf *m, struct tcphdr *th, struct socket *so, #ifdef STATS stats_voi_update_abs_ulong(tp->t_stats, VOI_TCP_FRWIN, tiwin); #endif - /* - * 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 (m->m_flags & M_TSTMP) { /* Prefer the hardware timestamp if present */ @@ -11459,6 +11453,23 @@ bbr_do_segment_nounlock(struct mbuf *m, struct tcphdr *th, struct socket *so, */ bbr->r_ctl.rc_rcvtime = lcts = cts = tcp_get_usecs(&bbr->rc_tv); } + /* + * 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)) { + retval = 0; + goto done_with_input; + } /* * If echoed timestamp is later than the current time, fall back to * non RFC1323 RTT calculation. Normalize timestamp if syncookies diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c index 7523a98db4c5..c225377807ba 100644 --- a/sys/netinet/tcp_stacks/rack.c +++ b/sys/netinet/tcp_stacks/rack.c @@ -10525,7 +10525,7 @@ rack_handoff_ok(struct tcpcb *tp) if ((tp->t_state == TCPS_SYN_SENT) || (tp->t_state == TCPS_SYN_RECEIVED)) { /* - * We really don't know if you support sack, + * We really don't know if you support sack, * you have to get to ESTAB or beyond to tell. */ return (EAGAIN); @@ -10868,6 +10868,26 @@ rack_do_segment_nounlock(struct mbuf *m, struct tcphdr *th, struct socket *so, ctf_do_dropwithreset(m, tp, th, BANDLIM_RST_OPENPORT, tlen); return(1); } + + /* + * 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 @@ -10920,12 +10940,6 @@ rack_do_segment_nounlock(struct mbuf *m, struct tcphdr *th, struct socket *so, 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 diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c index 9ea63dc723d2..6f4bdbc2f32d 100644 --- a/sys/netinet/tcp_syncache.c +++ b/sys/netinet/tcp_syncache.c @@ -1211,6 +1211,40 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th, return (-1); /* Do not send RST */ } + /* + * If timestamps were not negotiated during SYN/ACK and a + * segment without 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. * @@ -1256,32 +1290,6 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th, 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); if (*lsop == NULL) diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c index 54966811a1b2..c52eab956303 100644 --- a/sys/netinet/tcp_timewait.c +++ b/sys/netinet/tcp_timewait.c @@ -376,7 +376,7 @@ tcp_twstart(struct tcpcb *tp) * looking for a pcb in the listen state. Returns 0 otherwise. */ 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; @@ -411,6 +411,16 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unused, struct tcphdr *th, if (thflags & TH_RST) 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; + } + #if 0 /* PAWS not needed at the moment */ /*