From nobody Sat Aug 03 23:06:08 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4WbyxD5Gh3z5RWTQ; Sat, 03 Aug 2024 23:06:08 +0000 (UTC) (envelope-from git@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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WbyxD4pXfz4NmM; Sat, 3 Aug 2024 23:06:08 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1722726368; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=bCC3w/JUKVgQvWO9cBMHp/dKQMCCbxld5r0SHPmpL2c=; b=Qt0yXOMS4rM+ihhPRr8WVe3VdPXO/xBDcoT2xdv6DHfSpkpKa/1U5f+M6i398qaiHMh1IH K/yMtHSsBc2iFO4xi+Wa6RUAxFKVwWi42J4bTkqA3eOmJWTAVZCNaGSxEYs6pzjbWp1lv8 ElnBrHbJPWBvC1NnzJIZiKip8HA3LSGyG3pPNapwo6NrKHk1CjMIRaMKSWrl8crQrmvHxV 8MuukK7onN6LRfMnt7VAS/9IXRNlHVLePA7qDO6oKplFUUar/fIxjVCgl2Vd8VHnDwJQmP z8sl91PMKNTZv4IsuCBNHLybp8F2A7pdK7CmUdZSq9v+8yJxhb4jQVSz2McDwA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1722726368; a=rsa-sha256; cv=none; b=YUJBpIUfMGnLPbB3YB1BzoMrfQTd5GD6Yz+jSfdHVpdQRT6t/qeKopjMJrZOIX1gvHRydW yM/rQCo4iwHI8tOaLBAAr+yPOSjmpT3wqMHYO4iDvxVynuwguo5liM9NmmCGR1GdtLca76 Ft2hGTl2W2KSNoquE6MZYeRFb0pu/oPQk22e7Emp+OauNcyBGLHo/jWejUM/oOcIR9eH/2 r7FNTCnHnyB1Y3I3ntPLkjRCD0nA0KNIMLAQlSJSkukaqniDsGB4zMipNNkNFExQ82vGWb ePOzqtwEcKtwzqUGa0YF3RjSXKuJ5lHJ89IyL38ye2KRirIEMZDIk+RXMPaDXg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1722726368; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=bCC3w/JUKVgQvWO9cBMHp/dKQMCCbxld5r0SHPmpL2c=; b=gvK983VqhFj5L/xzapfLJ99lXwWZKY9Xnx6tJ3mMru9pBQtnBCOUvjg0f+6lMoGfpvam1u vtD4zu2AX6PEKD2BQya0paiFZmJrDNuhSVageKgqIncA2lTkcZ910wxRnt8p3wzY3qNsQQ 8NhaS3aVn+k1NjgxyfIAXaByp0YqSFLjrBkOaMP3O46eVx9yaA/ZPZ/bwYoqRBoHasXxjm i9tqqubiFV8R2fnr8t/MdsiRBAyen+jomJDhPuaosRfXmEw45DLlUM5mfYScbVCa9uRc27 b4AkN6BhmHIBfocbpXoZ6sst/OiQotCqXpFLzoyC3LY9q0tWPTPjM91H6i1e9w== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4WbyxD4J0pzXK5; Sat, 3 Aug 2024 23:06:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 473N680L096229; Sat, 3 Aug 2024 23:06:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 473N687E096226; Sat, 3 Aug 2024 23:06:08 GMT (envelope-from git) Date: Sat, 3 Aug 2024 23:06:08 GMT Message-Id: <202408032306.473N687E096226@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Michael Tuexen Subject: git: 1ce8cf6f7bdf - stable/14 - tcp: improve SEG.ACK validation List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: tuexen X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 1ce8cf6f7bdf5e9f8e497be5e3c54767fa0a7cf8 Auto-Submitted: auto-generated The branch stable/14 has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=1ce8cf6f7bdf5e9f8e497be5e3c54767fa0a7cf8 commit 1ce8cf6f7bdf5e9f8e497be5e3c54767fa0a7cf8 Author: Michael Tuexen AuthorDate: 2024-07-21 09:37:35 +0000 Commit: Michael Tuexen CommitDate: 2024-08-03 23:05:13 +0000 tcp: improve SEG.ACK validation Implement the improved SEG.ACK validation described in RFC 5961. In addition to that, also detect ghost ACKs, which are ACKs for data that has never been sent. The additional checks are enabled by default, but can be disabled by setting the sysctl-variable net.inet.tcp.insecure_ack to a non-zero value. PR: 250357 Reviewed by: Peter Lei, rscheff (older version) Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D45894 (cherry picked from commit 646c28ea80cb0f9258386626297495b5a0e56db5) --- share/man/man4/tcp.4 | 5 ++++- sys/netinet/tcp_input.c | 44 +++++++++++++++++++++++++++++++++++++++++++ sys/netinet/tcp_stacks/bbr.c | 37 ++++++++++++++++++++++++++++++++++++ sys/netinet/tcp_stacks/rack.c | 39 ++++++++++++++++++++++++++++++++++++++ sys/netinet/tcp_var.h | 8 +++++++- usr.bin/netstat/inet.c | 8 ++++++-- 6 files changed, 137 insertions(+), 4 deletions(-) diff --git a/share/man/man4/tcp.4 b/share/man/man4/tcp.4 index 16c9e0ce84df..1f5cc7734bbf 100644 --- a/share/man/man4/tcp.4 +++ b/share/man/man4/tcp.4 @@ -33,7 +33,7 @@ .\" .\" From: @(#)tcp.4 8.1 (Berkeley) 6/5/93 .\" -.Dd November 17, 2023 +.Dd July 21, 2024 .Dt TCP 4 .Os .Sh NAME @@ -708,6 +708,9 @@ Default is false. .It Va insecure_syn Use criteria defined in RFC793 instead of RFC5961 for accepting SYN segments. Default is false. +.It Va insecure_ack +Use criteria defined in RFC793 for validating SEG.ACK. +Default is false. .It Va isn_reseed_interval The interval (in seconds) specifying how often the secret data used in RFC 1948 initial sequence number calculations should be reseeded. diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index 133818b73a1d..21f4d046d6c9 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -209,6 +209,11 @@ SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(tcp_insecure_rst), 0, "Follow RFC793 instead of RFC5961 criteria for accepting RST packets"); +VNET_DEFINE(int, tcp_insecure_ack) = 0; +SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_ack, CTLFLAG_VNET | CTLFLAG_RW, + &VNET_NAME(tcp_insecure_ack), 0, + "Follow RFC793 criteria for validating SEG.ACK"); + VNET_DEFINE(int, tcp_recvspace) = 1024*64; #define V_tcp_recvspace VNET(tcp_recvspace) SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW, @@ -2417,6 +2422,45 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th, /* * Ack processing. */ + if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) { + /* Checking SEG.ACK against ISS is definitely redundant. */ + tp->t_flags2 |= TF2_NO_ISS_CHECK; + } + if (!V_tcp_insecure_ack) { + tcp_seq seq_min; + bool ghost_ack_check; + + if (tp->t_flags2 & TF2_NO_ISS_CHECK) { + /* Check for too old ACKs (RFC 5961, Section 5.2). */ + seq_min = tp->snd_una - tp->max_sndwnd; + ghost_ack_check = false; + } else { + if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) { + /* Checking for ghost ACKs is stricter. */ + seq_min = tp->iss + 1; + ghost_ack_check = true; + } else { + /* + * Checking for too old ACKs (RFC 5961, + * Section 5.2) is stricter. + */ + seq_min = tp->snd_una - tp->max_sndwnd; + ghost_ack_check = false; + } + } + if (SEQ_LT(th->th_ack, seq_min)) { + if (ghost_ack_check) + TCPSTAT_INC(tcps_rcvghostack); + else + TCPSTAT_INC(tcps_rcvacktooold); + /* Send a challenge ACK. */ + tcp_respond(tp, mtod(m, void *), th, m, + tp->rcv_nxt, tp->snd_nxt, TH_ACK); + tp->last_ack_sent = tp->rcv_nxt; + m = NULL; + goto drop; + } + } switch (tp->t_state) { /* * In SYN_RECEIVED state, the ack ACKs our SYN, so enter diff --git a/sys/netinet/tcp_stacks/bbr.c b/sys/netinet/tcp_stacks/bbr.c index 7803865af818..2bdab744e0d9 100644 --- a/sys/netinet/tcp_stacks/bbr.c +++ b/sys/netinet/tcp_stacks/bbr.c @@ -7700,6 +7700,43 @@ bbr_process_ack(struct mbuf *m, struct tcphdr *th, struct socket *so, bbr = (struct tcp_bbr *)tp->t_fb_ptr; lost = bbr->r_ctl.rc_lost; nsegs = max(1, m->m_pkthdr.lro_nsegs); + if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) { + /* Checking SEG.ACK against ISS is definitely redundant. */ + tp->t_flags2 |= TF2_NO_ISS_CHECK; + } + if (!V_tcp_insecure_ack) { + tcp_seq seq_min; + bool ghost_ack_check; + + if (tp->t_flags2 & TF2_NO_ISS_CHECK) { + /* Check for too old ACKs (RFC 5961, Section 5.2). */ + seq_min = tp->snd_una - tp->max_sndwnd; + ghost_ack_check = false; + } else { + if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) { + /* Checking for ghost ACKs is stricter. */ + seq_min = tp->iss + 1; + ghost_ack_check = true; + } else { + /* + * Checking for too old ACKs (RFC 5961, + * Section 5.2) is stricter. + */ + seq_min = tp->snd_una - tp->max_sndwnd; + ghost_ack_check = false; + } + } + if (SEQ_LT(th->th_ack, seq_min)) { + if (ghost_ack_check) + TCPSTAT_INC(tcps_rcvghostack); + else + TCPSTAT_INC(tcps_rcvacktooold); + /* Send challenge ACK. */ + ctf_do_dropafterack(m, tp, th, thflags, tlen, ret_val); + bbr->r_wanted_output = 1; + return (1); + } + } if (SEQ_GT(th->th_ack, tp->snd_max)) { ctf_do_dropafterack(m, tp, th, thflags, tlen, ret_val); bbr->r_wanted_output = 1; diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c index d918d9385446..c27f745ade62 100644 --- a/sys/netinet/tcp_stacks/rack.c +++ b/sys/netinet/tcp_stacks/rack.c @@ -12149,6 +12149,45 @@ rack_process_ack(struct mbuf *m, struct tcphdr *th, struct socket *so, INP_WLOCK_ASSERT(tptoinpcb(tp)); rack = (struct tcp_rack *)tp->t_fb_ptr; + if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) { + /* Checking SEG.ACK against ISS is definitely redundant. */ + tp->t_flags2 |= TF2_NO_ISS_CHECK; + } + if (!V_tcp_insecure_ack) { + tcp_seq seq_min; + bool ghost_ack_check; + + if (tp->t_flags2 & TF2_NO_ISS_CHECK) { + /* Check for too old ACKs (RFC 5961, Section 5.2). */ + seq_min = tp->snd_una - tp->max_sndwnd; + ghost_ack_check = false; + } else { + if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) { + /* Checking for ghost ACKs is stricter. */ + seq_min = tp->iss + 1; + ghost_ack_check = true; + } else { + /* + * Checking for too old ACKs (RFC 5961, + * Section 5.2) is stricter. + */ + seq_min = tp->snd_una - tp->max_sndwnd; + ghost_ack_check = false; + } + } + if (SEQ_LT(th->th_ack, seq_min)) { + if (ghost_ack_check) + TCPSTAT_INC(tcps_rcvghostack); + else + TCPSTAT_INC(tcps_rcvacktooold); + /* Send challenge ACK. */ + __ctf_do_dropafterack(m, tp, th, thflags, tlen, ret_val, + &rack->r_ctl.challenge_ack_ts, + &rack->r_ctl.challenge_ack_cnt); + rack->r_wanted_output = 1; + return (1); + } + } if (SEQ_GT(th->th_ack, tp->snd_max)) { __ctf_do_dropafterack(m, tp, th, thflags, tlen, ret_val, &rack->r_ctl.challenge_ack_ts, diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h index a8621563e190..010ad748260a 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -858,6 +858,7 @@ tcp_packets_this_ack(struct tcpcb *tp, tcp_seq ack) #define TF2_MBUF_QUEUE_READY 0x00020000 /* Inputs can be queued */ #define TF2_DONT_SACK_QUEUE 0x00040000 /* Don't wake on sack */ #define TF2_CANNOT_DO_ECN 0x00080000 /* The stack does not do ECN */ +#define TF2_NO_ISS_CHECK 0x00200000 /* Don't check SEG.ACK against ISS */ /* * Structure to hold TCP options that are only used during segment @@ -1109,8 +1110,11 @@ struct tcpstat { uint64_t tcps_tlpresends; /* number of tlp resends */ uint64_t tcps_tlpresend_bytes; /* number of bytes resent by tlp */ + /* SEG.ACK validation failures */ + uint64_t tcps_rcvghostack; /* received ACK for data never sent */ + uint64_t tcps_rcvacktooold; /* received ACK for data too long ago */ - uint64_t _pad[4]; /* 4 TBD placeholder for STABLE */ + uint64_t _pad[2]; /* 2 TBD placeholder for STABLE */ }; #define tcps_rcvmemdrop tcps_rcvreassfull /* compat */ @@ -1291,6 +1295,7 @@ VNET_DECLARE(int, tcp_ecn_maxretries); VNET_DECLARE(int, tcp_initcwnd_segments); VNET_DECLARE(int, tcp_insecure_rst); VNET_DECLARE(int, tcp_insecure_syn); +VNET_DECLARE(int, tcp_insecure_ack); VNET_DECLARE(uint32_t, tcp_map_entries_limit); VNET_DECLARE(uint32_t, tcp_map_split_limit); VNET_DECLARE(int, tcp_minmss); @@ -1337,6 +1342,7 @@ VNET_DECLARE(struct inpcbinfo, tcbinfo); #define V_tcp_initcwnd_segments VNET(tcp_initcwnd_segments) #define V_tcp_insecure_rst VNET(tcp_insecure_rst) #define V_tcp_insecure_syn VNET(tcp_insecure_syn) +#define V_tcp_insecure_ack VNET(tcp_insecure_ack) #define V_tcp_map_entries_limit VNET(tcp_map_entries_limit) #define V_tcp_map_split_limit VNET(tcp_map_split_limit) #define V_tcp_minmss VNET(tcp_minmss) diff --git a/usr.bin/netstat/inet.c b/usr.bin/netstat/inet.c index 19f7ba1490c4..3442666ce0bd 100644 --- a/usr.bin/netstat/inet.c +++ b/usr.bin/netstat/inet.c @@ -649,8 +649,12 @@ tcp_stats(u_long off, const char *name, int af1 __unused, int proto __unused) "{N:/UDP tunneled pkt%s}\n"); p(tcps_tunneled_errs, "\t\t{:received-bad-udp-tunneled-pkts/%ju} " "{N:/UDP tunneled pkt cnt with error%s}\n"); - p(tcps_rcvacktoomuch, "\t\t{:received-acks-for-unsent-data/%ju} " - "{N:/ack%s for unsent data}\n"); + p(tcps_rcvacktoomuch, "\t\t{:received-acks-for-data-not-yet-sent/%ju} " + "{N:/ack%s for data not yet sent}\n"); + p(tcps_rcvghostack, "\t\t{:received-acks-for-data-never-been-sent/%ju} " + "{N:/ack%s for data never been sent (ghost acks)}\n"); + p(tcps_rcvacktooold, "\t\t{:received-acks-for-data-being-too-old/%ju} " + "{N:/ack%s for data being too old}\n"); p2(tcps_rcvpack, tcps_rcvbyte, "\t\t" "{:received-in-sequence-packets/%ju} {N:/packet%s} " "({:received-in-sequence-bytes/%ju} {N:/byte%s}) "