From owner-svn-src-head@freebsd.org Mon Nov 9 21:49:42 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 37C7E46E718; Mon, 9 Nov 2020 21:49:42 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CVPm619QNz3w58; Mon, 9 Nov 2020 21:49:42 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 16B1617F35; Mon, 9 Nov 2020 21:49:42 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 0A9Lnfox069054; Mon, 9 Nov 2020 21:49:41 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 0A9Lnfmh069050; Mon, 9 Nov 2020 21:49:41 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <202011092149.0A9Lnfmh069050@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Mon, 9 Nov 2020 21:49:41 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r367530 - in head/sys/netinet: . tcp_stacks X-SVN-Group: head X-SVN-Commit-Author: tuexen X-SVN-Commit-Paths: in head/sys/netinet: . tcp_stacks X-SVN-Commit-Revision: 367530 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Nov 2020 21:49:42 -0000 Author: tuexen Date: Mon Nov 9 21:49:40 2020 New Revision: 367530 URL: https://svnweb.freebsd.org/changeset/base/367530 Log: 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 Modified: head/sys/netinet/tcp_input.c head/sys/netinet/tcp_stacks/bbr.c head/sys/netinet/tcp_stacks/rack.c head/sys/netinet/tcp_syncache.c head/sys/netinet/tcp_timewait.c Modified: head/sys/netinet/tcp_input.c ============================================================================== --- head/sys/netinet/tcp_input.c Mon Nov 9 21:19:17 2020 (r367529) +++ head/sys/netinet/tcp_input.c Mon Nov 9 21:49:40 2020 (r367530) @@ -977,8 +977,8 @@ findpcb: * 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, 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 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); } } Modified: head/sys/netinet/tcp_stacks/bbr.c ============================================================================== --- head/sys/netinet/tcp_stacks/bbr.c Mon Nov 9 21:19:17 2020 (r367529) +++ head/sys/netinet/tcp_stacks/bbr.c Mon Nov 9 21:49:40 2020 (r367530) @@ -11430,12 +11430,6 @@ bbr_do_segment_nounlock(struct mbuf *m, struct tcphdr #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 */ @@ -11458,6 +11452,23 @@ bbr_do_segment_nounlock(struct mbuf *m, struct tcphdr * Ok just get the current time. */ 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 Modified: head/sys/netinet/tcp_stacks/rack.c ============================================================================== --- head/sys/netinet/tcp_stacks/rack.c Mon Nov 9 21:19:17 2020 (r367529) +++ head/sys/netinet/tcp_stacks/rack.c Mon Nov 9 21:49:40 2020 (r367530) @@ -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,7 +10868,27 @@ rack_do_segment_nounlock(struct mbuf *m, struct tcphdr 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 * ignore broken/spoofed segs. @@ -10920,12 +10940,6 @@ rack_do_segment_nounlock(struct mbuf *m, struct tcphdr 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 Modified: head/sys/netinet/tcp_syncache.c ============================================================================== --- head/sys/netinet/tcp_syncache.c Mon Nov 9 21:19:17 2020 (r367529) +++ head/sys/netinet/tcp_syncache.c Mon Nov 9 21:49:40 2020 (r367530) @@ -1212,6 +1212,40 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt } /* + * 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. * * NOTE: We must decrease TCPS_SYN_RECEIVED count here, not @@ -1254,32 +1288,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: head/sys/netinet/tcp_timewait.c ============================================================================== --- head/sys/netinet/tcp_timewait.c Mon Nov 9 21:19:17 2020 (r367529) +++ head/sys/netinet/tcp_timewait.c Mon Nov 9 21:49:40 2020 (r367530) @@ -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; @@ -410,6 +410,16 @@ tcp_twcheck(struct inpcb *inp, struct tcpopt *to __unu */ 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 */