On 12 Jan 2022, at 22:26, Mike Karels wrote:
Rob wrote:
On Wed, Jan 12, 2022 at 6:58 AM Mike Karels <m...@karels.net> 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 <r...@freebsd.org>
AuthorDate: 2022-01-09 01:07:50 +0000
Commit: Robert Wing <r...@freebsd.org>
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.
