The branch main has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=7b57f2513361fb98fd5e2262f130989fe65946c6
commit 7b57f2513361fb98fd5e2262f130989fe65946c6 Author: Michael Tuexen <[email protected]> AuthorDate: 2025-08-30 14:47:10 +0000 Commit: Michael Tuexen <[email protected]> CommitDate: 2025-08-30 14:47:10 +0000 tcp: improve sending of SYN-cookies Ensure that when the sysctl-variable net.inet.tcp.syncookies_only is non zero, SYN-cookies are sent and no SYN-cache entry is added to the SYN-cache. In particular, this behavior should not depend on the value of the sysctl-variable net.inet.tcp.syncookies, which controls whether SYN cookies are used in combination with the SYN-cache to deal with bucket overflows. Also ensure that tcps_sc_completed does not include TCP connections established via a SYN-cookie. While there, make V_tcp_syncookies and V_tcp_syncookiesonly bool instead of int, since they are used as boolean variables. Reviewed by: rscheff, cc, Peter Lei, Nick Banks MFC after: 1 week Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D52225 --- sys/netinet/tcp_syncache.c | 89 +++++++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c index d617d0ed4aac..bec1a0bd14c4 100644 --- a/sys/netinet/tcp_syncache.c +++ b/sys/netinet/tcp_syncache.c @@ -102,15 +102,15 @@ #include <security/mac/mac_framework.h> -VNET_DEFINE_STATIC(int, tcp_syncookies) = 1; +VNET_DEFINE_STATIC(bool, tcp_syncookies) = true; #define V_tcp_syncookies VNET(tcp_syncookies) -SYSCTL_INT(_net_inet_tcp, OID_AUTO, syncookies, CTLFLAG_VNET | CTLFLAG_RW, +SYSCTL_BOOL(_net_inet_tcp, OID_AUTO, syncookies, CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(tcp_syncookies), 0, "Use TCP SYN cookies if the syncache overflows"); -VNET_DEFINE_STATIC(int, tcp_syncookiesonly) = 0; +VNET_DEFINE_STATIC(bool, tcp_syncookiesonly) = false; #define V_tcp_syncookiesonly VNET(tcp_syncookiesonly) -SYSCTL_INT(_net_inet_tcp, OID_AUTO, syncookies_only, CTLFLAG_VNET | CTLFLAG_RW, +SYSCTL_BOOL(_net_inet_tcp, OID_AUTO, syncookies_only, CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(tcp_syncookiesonly), 0, "Use only TCP SYN cookies"); @@ -553,9 +553,8 @@ syncache_timer(void *xsch) static inline bool syncache_cookiesonly(void) { - - return (V_tcp_syncookies && (V_tcp_syncache.paused || - V_tcp_syncookiesonly)); + return ((V_tcp_syncookies && V_tcp_syncache.paused) || + V_tcp_syncookiesonly); } /* @@ -1083,40 +1082,48 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th, #endif if (sc == NULL) { - /* - * There is no syncache entry, so see if this ACK is - * a returning syncookie. To do this, first: - * A. Check if syncookies are used in case of syncache - * overflows - * B. See if this socket has had a syncache entry dropped in - * the recent past. We don't want to accept a bogus - * syncookie if we've never received a SYN or accept it - * twice. - * C. check that the syncookie is valid. If it is, then - * cobble up a fake syncache entry, and return. - */ - if (locked && !V_tcp_syncookies) { - SCH_UNLOCK(sch); - TCPSTAT_INC(tcps_sc_spurcookie); - if ((s = tcp_log_addrs(inc, th, NULL, NULL))) - log(LOG_DEBUG, "%s; %s: Spurious ACK, " - "segment rejected (syncookies disabled)\n", - s, __func__); - goto failed; - } - if (locked && !V_tcp_syncookiesonly && - sch->sch_last_overflow < time_uptime - SYNCOOKIE_LIFETIME) { + if (locked) { + /* + * The syncache is currently in use (neither disabled, + * nor paused), but no entry was found. + */ + if (!V_tcp_syncookies) { + /* + * Since no syncookies are used in case of + * a bucket overflow, don't even check for + * a valid syncookie. + */ + SCH_UNLOCK(sch); + TCPSTAT_INC(tcps_sc_spurcookie); + if ((s = tcp_log_addrs(inc, th, NULL, NULL))) + log(LOG_DEBUG, "%s; %s: Spurious ACK, " + "segment rejected " + "(syncookies disabled)\n", + s, __func__); + goto failed; + } + if (sch->sch_last_overflow < + time_uptime - SYNCOOKIE_LIFETIME) { + /* + * Since the bucket did not overflow recently, + * don't even check for a valid syncookie. + */ + SCH_UNLOCK(sch); + TCPSTAT_INC(tcps_sc_spurcookie); + if ((s = tcp_log_addrs(inc, th, NULL, NULL))) + log(LOG_DEBUG, "%s; %s: Spurious ACK, " + "segment rejected " + "(no syncache entry)\n", + s, __func__); + goto failed; + } SCH_UNLOCK(sch); - TCPSTAT_INC(tcps_sc_spurcookie); - if ((s = tcp_log_addrs(inc, th, NULL, NULL))) - log(LOG_DEBUG, "%s; %s: Spurious ACK, " - "segment rejected (no syncache entry)\n", - s, __func__); - goto failed; } - if (locked) - SCH_UNLOCK(sch); bzero(&scs, sizeof(scs)); + /* + * Now check, if the syncookie is valid. If it is, create an on + * stack syncache entry. + */ if (syncookie_expand(inc, sch, &scs, th, to, *lsop, port)) { sc = &scs; TCPSTAT_INC(tcps_sc_recvcookie); @@ -1291,7 +1298,7 @@ syncache_expand(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th, if (__predict_false(*lsop == NULL)) { TCPSTAT_INC(tcps_sc_aborted); TCPSTATES_DEC(TCPS_SYN_RECEIVED); - } else + } else if (sc != &scs) TCPSTAT_INC(tcps_sc_completed); if (sc != &scs) @@ -1718,13 +1725,13 @@ syncache_add(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th, if (V_tcp_do_ecn && (tp->t_flags2 & TF2_CANNOT_DO_ECN) == 0) sc->sc_flags |= tcp_ecn_syncache_add(tcp_get_flags(th), iptos); - if (V_tcp_syncookies) + if (V_tcp_syncookies || V_tcp_syncookiesonly) sc->sc_iss = syncookie_generate(sch, sc); else sc->sc_iss = arc4random(); #ifdef INET6 if (autoflowlabel) { - if (V_tcp_syncookies) + if (V_tcp_syncookies || V_tcp_syncookiesonly) sc->sc_flowlabel = sc->sc_iss; else sc->sc_flowlabel = ip6_randomflowlabel();
