From nobody Sat Jan 15 13:43:41 2022 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 DD4081956A8A; Sat, 15 Jan 2022 13:43:51 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (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 "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JbfX75k0nz3vgF; Sat, 15 Jan 2022 13:43:51 +0000 (UTC) (envelope-from bz@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1642254231; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hNbHbCSsubdR/hkmTBbcdyBw3lgPnjGGVgNiPKquN7w=; b=PKKHWpzqhEI9MbKksZ94PT6+BlpSSc11//ZR9TSzxvysdOrPy172NACAMrhg9umJg9+Td1 25E8pmSX+eN1vK13BAnaKuMwHfxP2Axo9Ubm4zh//b4oB+agzLp5E+sfjQA6cmHzzycDM8 695H277TdJ7xzU5oCmlaMfrqpbiHms7KOpi10BQZZclHSdeyxLMjN2GLrTiOWMZFSLd55c 6vMvPpl5S1rpQN124UskLCBvIDNQwdAL5Oxgv565SwlMPcX1LWOd7JHJ+387zrhNmjFDQR 9sqOto+inUywjo324PgcSymFDaoxBW1mNWjBpHUey/nhOowppN0EsHIdxLXbEw== Received: from mx1.sbone.de (mx1.sbone.de [IPv6:2a01:4f8:13b:39f::9f:25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mx1.sbone.de", Issuer "SBone.DE" (not verified)) (Authenticated sender: bz/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 79E9C242E6; Sat, 15 Jan 2022 13:43:51 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:31::2013:587]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id 0FF838D4A15D; Sat, 15 Jan 2022 13:43:47 +0000 (UTC) Received: from content-filter.sbone.de (content-filter.sbone.de [IPv6:fde9:577b:c1a9:31::2013:2742]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id 8106BE707B2; Sat, 15 Jan 2022 13:43:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:31::2013:587]) by content-filter.sbone.de (content-filter.sbone.de [fde9:577b:c1a9:31::2013:2742]) (amavisd-new, port 10024) with ESMTP id uFchGdAHlfbp; Sat, 15 Jan 2022 13:43:43 +0000 (UTC) Received: from [169.254.226.220] (unknown [IPv6:fde9:577b:c1a9:4902:282d:bcf8:dcef:989]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id A7860E707B0; Sat, 15 Jan 2022 13:43:42 +0000 (UTC) From: "Bjoern A. Zeeb" To: "Mike Karels" Cc: "Rob Wing" , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: eb18708ec8c7 - main - syncache: accept packet with no SA when TCP_MD5SIG is set Date: Sat, 15 Jan 2022 13:43:41 +0000 X-Mailer: MailMate (2.0BETAr6151) Message-ID: <28FCB112-27F4-47C2-A13C-43BCE79DAA09@FreeBSD.org> In-Reply-To: <202201122226.20CMQwMw007750@mail.karels.net> References: <202201122226.20CMQwMw007750@mail.karels.net> 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: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1642254231; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hNbHbCSsubdR/hkmTBbcdyBw3lgPnjGGVgNiPKquN7w=; b=ZERNWFbJOOHnj7Ytqqaq45iwwoN/ZGFoAeQHUlIZ5W93ox4x7FTrgjcIdrxSjVE71QUm6x g+5+Q9DOjyeeyhz5bjw4qfPlGLgJLIzSlSC0jjm34ghJzsgq9ykydnmASeysI2v7nqIiRI IyClnPnI1NvPRW40nWhQOP63MUPPKCd+lTjKml4nQYwJbthUU1HLcf/Tn2XM17sSip9e9S qBSVDnu+Y2YZvK/IoF7BhneLEu62IK48E5F4mLZpAf3bqkF2fBk0T3/wSYrYLi/L57saMk G0njfqoQzNR5Ce2W+9+vhZmbvkjRTA6Dz4AEh5eR4qbHkdV92qsP2UtTexcZCw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1642254231; a=rsa-sha256; cv=none; b=f5aStURFVxCNwb1HZoR0C5DjyDyXZOiarMmjkkhQ+BBRAkL/WKl5CTp5bJqfUMcgx0RSP9 sE2MhrDwFbxRIzHVWYoVxy/8qgxWHmcmHTX9fP7jCySGaBgmlV7bKUu2R9VsWlpMipTqFr ApgKjfor6GWkB34NWhC3OJ75fkYcyCD5kK/vFUzEAS/3YcRezY0OHwzOfzwW5Qma/DTIFU 6KErfww6tvMKPX6F3P+gDKcGYPKsa309HZ+ibt/gXUGey/9pBNbN7eHLhg/jK/gmfOYYIB BFLtoCtEKGZtGCPEFb9vx6fPk23a4MrudkMXxmNbh9N1ROFdy0TKEz6jNX0bJg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N On 12 Jan 2022, at 22:26, Mike Karels wrote: > Rob wrote: >> On Wed, Jan 12, 2022 at 6:58 AM Mike Karels wrote: > >>> Sorry for the delayed response; I've been out. >>> > >> No worries. > >> This change seems wrong to me. The TCP_MD5SIG option clearly >> required MD5 >>> signatures on all connections in the past, for many years, > > >> You're right, since 2017. Before that, TCP_MD5SIG did not require MD5 >> signature when establishing a connection either. > > >>> That meant that connections were limited to peers in the Security >>> Association Database. > >> A program like a routing daemon could then be sure that it was >> talking to a >>> known peer using >>> signed packets > > >> This is still the case, if you've configured a peer to use MD5, it >> will use >> MD5. > > Yes, but if you haven't configured the SA, the result is different, > and > a routing daemon could receive a connection that wasn't from a known > peer. I would have hoped they’d all validate peer address to look up a config snippet and not find that peer then and close the socket.. > >>> Redefining the option at this late date seems unsafe and unwise. > > >> The option hasn't been redefined. > > It has, in the sense that connections from unknown peers without MD5 > signatures are now permitted. A daemon relying on the previous > behavior > wouldn't bother checking whether MD5 was enabled on incoming > connections. True. But for a while a single listen socket could only get either all-encrypted or non-encrypted which in most cases probably never worked. And that never was the original behaviour in FreeBSD. So the actual “change in behaviour” was introduced a while ago and now been restored again. I believe after accept a routing daemon could check the socket option being on the accepted connection to see what actually happened? (I have not checked if that actually works currently). I would agree to say that the flag is a bit convoluted. >>> If there is a use case for a socket that requires MD5SIG for known >>> peers and not for others, it seems to me that it would be better to >>> add a >>> new option with those semantics. >>> > >> The use case is a bgp daemon socket that wants to handle/establish >> connections with MD5 and non-MD5 peers. > > Which is different than the previous use case, where a daemon could be > sure that incoming connections were from a peer in the SADB and used > MD5. Yes, but again matches the previous-previous case which was there from the beginning. >> For peers configured with MD5, the established connection will be >> protected >> by MD5 signatures (i.e., the TCP_MD5SIG option will be set). If a >> peer is >> not configured for MD5, then the established connection will not be >> protected with MD5 signatures (i.e., TCP_MD5SIG will not be set). > >> For what it's worth, this behavior is consistent with OpenBSD. > > That's a useful data point, thanks. > > Mike > >> -Rob > > >>> >>> Mike >>> >>> On 8 Jan 2022, at 19:45, Robert Wing wrote: >>> >>>> The branch main has been updated by rew: >>>> >>>> URL: >>> https://cgit.FreeBSD.org/src/commit/?id=3Deb18708ec8c7e1de6a05aba41971659= >> 549991b10 >>>> >>>> commit eb18708ec8c7e1de6a05aba41971659549991b10 >>>> Author: Robert Wing >>>> AuthorDate: 2022-01-09 01:07:50 +0000 >>>> Commit: Robert Wing >>>> CommitDate: 2022-01-09 01:32:14 +0000 >>>> >>>> syncache: accept packet with no SA when TCP_MD5SIG is set >>>> >>>> When TCP_MD5SIG is set on a socket, all packets are dropped >>>> that >>> don't >>>> contain an MD5 signature. Relax this behavior to accept a >>>> non-signe= >> d >>>> packet when a security association doesn't exist with the peer. >>>> >>>> This is useful when a listen socket set with TCP_MD5SIG wants >>>> to >>> handle >>>> connections protected with and without MD5 signatures. >>>> >>>> Reviewed by: bz (previous version) >>>> Sponsored by: nepustil.net >>>> Sponsored by: Klara Inc. >>>> Differential Revision: https://reviews.freebsd.org/D33227 >>>> --- >>>> share/man/man4/tcp.4 | 6 +++++- >>>> sys/netinet/tcp_syncache.c | 30 ++++++++++++++++++------------ >>>> sys/netipsec/xform_tcp.c | 5 +++++ >>>> 3 files changed, 28 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/share/man/man4/tcp.4 b/share/man/man4/tcp.4 >>>> index 17138fa224ba..d103293132ba 100644 >>>> --- a/share/man/man4/tcp.4 >>>> +++ b/share/man/man4/tcp.4 >>>> @@ -34,7 +34,7 @@ >>>> .\" From: @(#)tcp.4 8.1 (Berkeley) 6/5/93 >>>> .\" $FreeBSD$ >>>> .\" >>>> -.Dd June 27, 2021 >>>> +.Dd January 8, 2022 >>>> .Dt TCP 4 >>>> .Os >>>> .Sh NAME >>>> @@ -339,6 +339,10 @@ This entry can only be specified on a per-host >>> basis at this time. >>>> .Pp >>>> If an SADB entry cannot be found for the destination, >>>> the system does not send any outgoing segments and drops any >>>> inbound >>> segments. >>>> +However, during connection negotiation, a non-signed segment will >>>> be >>> accepted if >>>> +an SADB entry does not exist between hosts. >>>> +When a non-signed segment is accepted, the established connection >>>> is n= >> ot >>>> +protected with MD5 digests. >>>> .It Dv TCP_STATS >>>> Manage collection of connection level statistics using the >>>> .Xr stats 3 >>>> diff --git a/sys/netinet/tcp_syncache.c >>>> b/sys/netinet/tcp_syncache.c >>>> index 7dd8443cad65..32ca3bc2209b 100644 >>>> --- a/sys/netinet/tcp_syncache.c >>>> +++ b/sys/netinet/tcp_syncache.c >>>> @@ -1514,19 +1514,25 @@ syncache_add(struct in_conninfo *inc, >>>> struct >>> tcpopt *to, struct tcphdr *th, >>>> >>>> #if defined(IPSEC_SUPPORT) || defined(TCP_SIGNATURE) >>>> /* >>>> - * If listening socket requested TCP digests, check that >>>> received >>>> - * SYN has signature and it is correct. If signature doesn't >>>> matc= >> h >>>> - * or TCP_SIGNATURE support isn't enabled, drop the packet. >>>> + * When the socket is TCP-MD5 enabled check that, >>>> + * - a signed packet is valid >>>> + * - a non-signed packet does not have a security >>>> association >>>> + * >>>> + * If a signed packet fails validation or a non-signed >>>> packet ha= >> s >>> a >>>> + * security association, the packet will be dropped. >>>> */ >>>> if (ltflags & TF_SIGNATURE) { >>>> - if ((to->to_flags & TOF_SIGNATURE) =3D=3D 0) { >>>> - TCPSTAT_INC(tcps_sig_err_nosigopt); >>>> - goto done; >>>> + if (to->to_flags & TOF_SIGNATURE) { >>>> + if (!TCPMD5_ENABLED() || >>>> + TCPMD5_INPUT(m, th, to->to_signature) >>>> !=3D 0) >>>> + goto done; >>>> + } else { >>>> + if (TCPMD5_ENABLED() && >>>> + TCPMD5_INPUT(m, NULL, NULL) !=3D ENOENT) >>>> + goto done; >>>> } >>>> - if (!TCPMD5_ENABLED() || >>>> - TCPMD5_INPUT(m, th, to->to_signature) !=3D 0) >>>> - goto done; >>>> - } >>>> + } else if (to->to_flags & TOF_SIGNATURE) >>>> + goto done; >>>> #endif /* TCP_SIGNATURE */ >>>> /* >>>> * See if we already have an entry for this connection. >>>> @@ -1724,11 +1730,11 @@ skip_alloc: >>>> } >>>> #if defined(IPSEC_SUPPORT) || defined(TCP_SIGNATURE) >>>> /* >>>> - * If listening socket requested TCP digests, flag this in >>>> the >>>> + * If incoming packet has an MD5 signature, flag this in the >>>> * syncache so that syncache_respond() will do the right >>>> thing >>>> * with the SYN+ACK. >>>> */ >>>> - if (ltflags & TF_SIGNATURE) >>>> + if (to->to_flags & TOF_SIGNATURE) >>>> sc->sc_flags |=3D SCF_SIGNATURE; >>>> #endif /* TCP_SIGNATURE */ >>>> if (to->to_flags & TOF_SACKPERM) >>>> diff --git a/sys/netipsec/xform_tcp.c b/sys/netipsec/xform_tcp.c >>>> index b53544cd00fb..ce2552f0a205 100644 >>>> --- a/sys/netipsec/xform_tcp.c >>>> +++ b/sys/netipsec/xform_tcp.c >>>> @@ -269,6 +269,11 @@ tcp_ipsec_input(struct mbuf *m, struct tcphdr >>>> *th, >>> u_char *buf) >>>> KMOD_TCPSTAT_INC(tcps_sig_err_buildsig); >>>> return (ENOENT); >>>> } >>>> + if (buf =3D=3D NULL) { >>>> + key_freesav(&sav); >>>> + KMOD_TCPSTAT_INC(tcps_sig_err_nosigopt); >>>> + return (EACCES); >>>> + } >>>> /* >>>> * tcp_input() operates with TCP header fields in host >>>> * byte order. We expect them in network byte order. >>>