From 48930a5ded1a35ccebe11a4c904f298be708ad07 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 7 Feb 2024 10:11:44 +0100 Subject: [PATCH 1/3] pid1: make MaxConnectionsPerSource= also work for AF_UNIX sockets The setting currently puts limits on connections per IP address and AF_UNIX CID. Let's extend it to cover AF_UNIX too, where it puts a limit on connections per UID. This is particularly useful for the various Accept=yes Varlink services we now have, as it means, the number of per-user instance services cannot grow without bounds. --- man/systemd.socket.xml | 7 ++++--- src/core/socket.c | 44 ++++++++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml index 0c74d5b2a9..1ac97ae137 100644 --- a/man/systemd.socket.xml +++ b/man/systemd.socket.xml @@ -470,9 +470,10 @@ MaxConnectionsPerSource= - The maximum number of connections for a service per source IP address. - This is very similar to the MaxConnections= directive - above. Disabled by default. + The maximum number of connections for a service per source IP address (in case of + IPv4/IPv6), per source CID (in case of AF_VSOCK), or source UID (in case of + AF_UNIX). This is very similar to the MaxConnections= + directive above. Disabled by default. diff --git a/src/core/socket.c b/src/core/socket.c index 2a5a8386d6..0ccbfa0090 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -53,6 +53,7 @@ struct SocketPeer { Socket *socket; union sockaddr_union peer; socklen_t peer_salen; + struct ucred peer_cred; }; static const UnitActiveState state_translation_table[_SOCKET_STATE_MAX] = { @@ -420,6 +421,8 @@ static void peer_address_hash_func(const SocketPeer *s, struct siphash *state) { siphash24_compress_typesafe(s->peer.in6.sin6_addr, state); else if (s->peer.sa.sa_family == AF_VSOCK) siphash24_compress_typesafe(s->peer.vm.svm_cid, state); + else if (s->peer.sa.sa_family == AF_UNIX) + siphash24_compress_typesafe(s->peer_cred.uid, state); else assert_not_reached(); } @@ -438,6 +441,8 @@ static int peer_address_compare_func(const SocketPeer *x, const SocketPeer *y) { return memcmp(&x->peer.in6.sin6_addr, &y->peer.in6.sin6_addr, sizeof(x->peer.in6.sin6_addr)); case AF_VSOCK: return CMP(x->peer.vm.svm_cid, y->peer.vm.svm_cid); + case AF_UNIX: + return CMP(x->peer_cred.uid, y->peer_cred.uid); } assert_not_reached(); } @@ -466,16 +471,22 @@ static int socket_load(Unit *u) { return socket_verify(s); } -static SocketPeer *socket_peer_new(void) { +static SocketPeer *socket_peer_dup(const SocketPeer *q) { SocketPeer *p; + assert(q); + p = new(SocketPeer, 1); if (!p) return NULL; *p = (SocketPeer) { .n_ref = 1, + .peer = q->peer, + .peer_salen = q->peer_salen, + .peer_cred = q->peer_cred, }; + return p; } @@ -492,8 +503,9 @@ DEFINE_TRIVIAL_REF_UNREF_FUNC(SocketPeer, socket_peer, socket_peer_free); int socket_acquire_peer(Socket *s, int fd, SocketPeer **ret) { _cleanup_(socket_peer_unrefp) SocketPeer *remote = NULL; - SocketPeer sa = { + SocketPeer key = { .peer_salen = sizeof(union sockaddr_union), + .peer_cred = UCRED_INVALID, }, *i; int r; @@ -501,27 +513,36 @@ int socket_acquire_peer(Socket *s, int fd, SocketPeer **ret) { assert(s); assert(ret); - if (getpeername(fd, &sa.peer.sa, &sa.peer_salen) < 0) + if (getpeername(fd, &key.peer.sa, &key.peer_salen) < 0) return log_unit_error_errno(UNIT(s), errno, "getpeername() failed: %m"); - if (!IN_SET(sa.peer.sa.sa_family, AF_INET, AF_INET6, AF_VSOCK)) { + switch (key.peer.sa.sa_family) { + case AF_INET: + case AF_INET6: + case AF_VSOCK: + break; + + case AF_UNIX: + r = getpeercred(fd, &key.peer_cred); + if (r < 0) + return log_unit_error_errno(UNIT(s), r, "Failed to get peer credentials of socket: %m"); + break; + + default: *ret = NULL; return 0; } - i = set_get(s->peers_by_address, &sa); + i = set_get(s->peers_by_address, &key); if (i) { *ret = socket_peer_ref(i); return 1; } - remote = socket_peer_new(); + remote = socket_peer_dup(&key); if (!remote) return log_oom(); - remote->peer = sa.peer; - remote->peer_salen = sa.peer_salen; - r = set_ensure_put(&s->peers_by_address, &peer_address_hash_ops, remote); if (r < 0) return log_unit_error_errno(UNIT(s), r, "Failed to insert peer info into hash table: %m"); @@ -2300,7 +2321,10 @@ static void socket_enter_running(Socket *s, int cfd_in) { if (r > 0 && p->n_ref > s->max_connections_per_source) { _cleanup_free_ char *t = NULL; - (void) sockaddr_pretty(&p->peer.sa, p->peer_salen, true, false, &t); + if (p->peer.sa.sa_family == AF_UNIX) + (void) asprintf(&t, "UID " UID_FMT, p->peer_cred.uid); + else + (void) sockaddr_pretty(&p->peer.sa, p->peer_salen, /* translate_ipv6= */ true, /* include_port= */ false, &t); log_unit_warning(UNIT(s), "Too many incoming connections (%u) from source %s, dropping connection.", From 5d1e8cd3e057261c6db3fb3d7de39b7ca48efd3b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 7 Feb 2024 13:19:54 +0100 Subject: [PATCH 2/3] units: enable MaxConnectionsPerSocket= for all our Accept=yes units Let's make sure that user's cannot DoS services for other users so easily, and enable MaxConnectionsPerSocket= by default for all of them. Note that this is mostly paranoia for systemd-pcrextend.socket and systemd-sysext.socket: the socket is only accessible to root anyway, hence the accounting shouldn#t change anything. But this is just a safety net, in preparation that we open up some functionality of these services sooner or later. --- units/systemd-coredump.socket | 1 + units/systemd-creds.socket | 1 + units/systemd-pcrextend.socket | 1 + units/systemd-sysext.socket | 1 + 4 files changed, 4 insertions(+) diff --git a/units/systemd-coredump.socket b/units/systemd-coredump.socket index a2d457fc02..c78eacd823 100644 --- a/units/systemd-coredump.socket +++ b/units/systemd-coredump.socket @@ -19,3 +19,4 @@ ListenSequentialPacket=/run/systemd/coredump SocketMode=0600 Accept=yes MaxConnections=16 +MaxConnectionsPerSource=8 diff --git a/units/systemd-creds.socket b/units/systemd-creds.socket index 65b76bd027..0f89b39c3e 100644 --- a/units/systemd-creds.socket +++ b/units/systemd-creds.socket @@ -18,3 +18,4 @@ ListenStream=/run/systemd/io.systemd.Credentials FileDescriptorName=varlink SocketMode=0666 Accept=yes +MaxConnectionsPerSource=16 diff --git a/units/systemd-pcrextend.socket b/units/systemd-pcrextend.socket index 7d156c1448..41db50acd8 100644 --- a/units/systemd-pcrextend.socket +++ b/units/systemd-pcrextend.socket @@ -20,6 +20,7 @@ ListenStream=/run/systemd/io.systemd.PCRExtend FileDescriptorName=varlink SocketMode=0600 Accept=yes +MaxConnectionsPerSource=16 [Install] WantedBy=sockets.target diff --git a/units/systemd-sysext.socket b/units/systemd-sysext.socket index ad870c5bfb..1a616ca69c 100644 --- a/units/systemd-sysext.socket +++ b/units/systemd-sysext.socket @@ -20,6 +20,7 @@ ListenStream=/run/systemd/io.systemd.sysext FileDescriptorName=varlink SocketMode=0600 Accept=yes +MaxConnectionsPerSource=16 [Install] WantedBy=sockets.target From 3456c89ac26872f4befa2bdf7c260529932e3909 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 7 Feb 2024 15:08:22 +0100 Subject: [PATCH 3/3] test: add a simple test for MaxConnectionsPerSocket= --- test/units/testsuite-74.socket.sh | 75 +++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100755 test/units/testsuite-74.socket.sh diff --git a/test/units/testsuite-74.socket.sh b/test/units/testsuite-74.socket.sh new file mode 100755 index 0000000000..b9f0e82899 --- /dev/null +++ b/test/units/testsuite-74.socket.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +# shellcheck disable=SC2016 +set -eux +set -o pipefail + +# shellcheck source=test/units/util.sh +. "$(dirname "$0")"/util.sh + +at_exit() { + systemctl stop per-source-limit.socket + rm -f /run/systemd/system/per-source-limit.socket /run/systemd/system/per-source-limit@.service + rm -f /tmp/foo.conn1 /tmp/foo.conn2 /tmp/foo.conn3 /tmp/foo.conn4 + systemctl daemon-reload +} + +trap at_exit EXIT + +cat > /run/systemd/system/per-source-limit.socket < /run/systemd/system/per-source-limit@.service < /tmp/foo.conn1 & +J1="$!" +socat - UNIX-CONNECT:/run/per-source-limit.sk > /tmp/foo.conn2 & +J2="$!" + +waitfor() { + while ! grep -q "waldo" "$1" ; do + sleep .2 + done +} + +# Wait until the word "waldo" shows in the output files +waitfor /tmp/foo.conn1 +waitfor /tmp/foo.conn2 + +# The next connection should fail, because the limit is hit +socat - UNIX-CONNECT:/run/per-source-limit.sk > /tmp/foo.conn3 & +J3="$!" + +# But this one should work, because done under a differen UID +setpriv --reuid=1 socat - UNIX-CONNECT:/run/per-source-limit.sk > /tmp/foo.conn4 & +J4="$!" + +waitfor /tmp/foo.conn4 + +# The third job should fail quickly, wait for it +wait "$J3" + +# The other jobs will hang forever, since we run "sleep infinity" on the server side. Let's kill the jobs now. +kill "$J1" +kill "$J2" +kill "$J4" + +# The 3rd connection should not have seen "waldo", since it should have been refused too early +(! grep -q "waldo" /tmp/foo.conn3 )