From 89be87b6a9218f4f28a85e221e2a2b40dd7f95a7 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Thu, 7 Nov 2024 14:33:58 +0100 Subject: [PATCH 1/5] session: Support Unix socket mode for client certificates When cockpit-session's stdin is a Unix socket, it is being spawned by cockpit-ws through cockpit-session@.service. In that case it doesn't make sense to look at its own cgroup, but we need to check the cgroup of the socket peer (i.e. cockpit-ws). We must guard against PID recycling attacks: 1. Mallory logs into cockpit, gets ws pid M, and hacks ws: connect to session, forks, keeps the session fd in a different process, and kills pid M. 3. Mallory waits until Alice logs in again and happens to get ws pid M (which can happen with a sufficient number of forks, social engineering, and some luck). cockpit-session checks that pid M is in cgroup /cockpit/alice, and starts an alice session for Mallory's ws. (Note: SO_PEERCRED gives you pid/uid/gid at the time connect() was made.) Thus require that the peer (ws) must have started earlier than cockpit-session. This is the same approach that polkit uses as a fallback if pidfds are not available: https://github.com/polkit-org/polkit/blob/main/src/polkit/polkitunixprocess.c Note that pidfds don't help us: There is no API to directly get from a pidfd to a cgroup, startup time, or /proc/ dirfd, this has to happen via `pidfd_getpid()` and opening /proc/pid. But that's exactly what we want to avoid, and thus is pointless (they are also only available since kernel 6.5). --- src/session/client-certificate.c | 131 ++++++++++++++++++++++++++----- 1 file changed, 113 insertions(+), 18 deletions(-) diff --git a/src/session/client-certificate.c b/src/session/client-certificate.c index d233894ee1a..51b37f74df1 100644 --- a/src/session/client-certificate.c +++ b/src/session/client-certificate.c @@ -31,7 +31,9 @@ #include #include #include +#include #include +#include #include #include @@ -41,6 +43,39 @@ #define CLIENT_CERTIFICATE_DIRECTORY "/run/cockpit/tls/clients" +static int +open_proc_pid (pid_t pid) +{ + char path[100]; + int r = snprintf (path, sizeof path, "/proc/%lu", (unsigned long) pid); + if (r < 0 || r >= sizeof path) + errx (EX, "memory error"); + int fd = open (path, O_DIRECTORY | O_NOFOLLOW | O_PATH | O_CLOEXEC); + if (fd < 0) + err (EX, "failed to open %s", path); + return fd; +} + +static size_t +read_proc_file (int dirfd, const char *name, char *buffer, size_t bufsize) +{ + int fd = openat (dirfd, name, O_RDONLY | O_CLOEXEC); + if (fd < 0) + err (EX, "Failed to open %s proc file", name); + + /* we don't accept/expect EINTR or short reads here: this is /proc, and we don't have + * signal handlers which survive the login */ + ssize_t len = read (fd, buffer, bufsize); + if (len < 0) + err (EX, "Failed to read /proc file %s", name); + + close (fd); + if (len >= bufsize) + errx (EX, "proc file %s exceeds buffer size %zu", name, bufsize); + buffer[len] = '\0'; + return len; +} + /* This is a bit lame, but having a hard limit on peer certificates is * desirable: Let's not get DoSed by huge certs */ #define MAX_PEER_CERT_SIZE 100000 @@ -49,31 +84,49 @@ * including "0::" prefix and newline. * NB: the kernel doesn't allow newlines in cgroup names. */ static char * -read_proc_self_cgroup (size_t *out_length) +read_proc_pid_cgroup (int dirfd, size_t *out_length) { - FILE *fp = fopen ("/proc/self/cgroup", "r"); + char buffer[1024]; + size_t len = read_proc_file (dirfd, "cgroup", buffer, sizeof buffer); - if (fp == NULL) + if (strncmp (buffer, "0::/", 4) == 0 && /* must be a cgroupsv2 */ + buffer[len - 1] == '\n') /* must end with a newline */ { - warn ("Failed to open /proc/self/cgroup"); - exit_init_problem ("internal-error", "Failed to open /proc/self/cgroup"); + *out_length = len; + return strdupx (buffer); } - char buffer[1024]; - *out_length = fread (buffer, 1, sizeof (buffer) - 1, fp); - buffer[*out_length] = '\0'; - fclose (fp); - - if (*out_length > 5 && /* at least "0::/\n" */ - *out_length <= sizeof (buffer) - 1 && /* not too big */ - strncmp (buffer, "0::", 3) == 0 && /* must be a cgroupsv2 */ - buffer[*out_length - 1] == '\n') /* must end with a newline */ - return strdupx (buffer); - warnx ("unexpected cgroups content, certificate matching only supports cgroup v2: '%s'", buffer); exit_init_problem ("authentication-unavailable", "certificate matching only supports cgroup v2"); } +static +unsigned long long get_proc_pid_start_time (int dirfd) +{ + char buffer[4096]; + read_proc_file (dirfd, "stat", buffer, sizeof buffer); + + /* start time is the token at index 19 after the '(process name)' entry - since only this + * field can contain the ')' character, search backwards for this to avoid malicious + * processes trying to fool us; See proc_pid_stat(5) */ + const char *p = strrchr (buffer, ')'); + if (p == NULL) + errx (EX, "Failed to find process name in /proc/pid/stat: %s", buffer); + for (int i = 0; i <= 19; i++) /* NB: ')' is the first token */ + { + p = strchr (p, ' '); + if (p == NULL) + errx (EX, "Failed to find start time in /proc/pid/stat"); + ++p; /* skip over the space */ + } + + char *endptr; + unsigned long long start_time = strtoull (p, &endptr, 10); + if (*endptr != ' ') + errx (EX, "Failed to parse start time in /proc/pid/stat from %s", p); + return start_time; +} + /* valid_256_bit_hex_string: * @str: a string * @@ -301,11 +354,53 @@ cockpit_session_client_certificate_map_user (const char *client_certificate_file exit_init_problem ("authentication-unavailable", "No https instance certificate present"); } + /* check if stdin is a Unix socket; then we are being called via cockpit-session@.service and need to + * check the cgroup of our peer */ + struct ucred ucred; + socklen_t ucred_len = sizeof ucred; + int ws_pid_dirfd; + if (getsockopt (STDIN_FILENO, SOL_SOCKET, SO_PEERCRED, &ucred, &ucred_len) == 0 && + /* this is an inout parameter, be extra suspicious */ + ucred_len >= sizeof ucred) + { + debug ("unix socket mode, reading cgroup from peer pid %d", ucred.pid); + ws_pid_dirfd = open_proc_pid (ucred.pid); + unsigned long long ws_start_time = get_proc_pid_start_time (ws_pid_dirfd); + + int my_pid_dirfd = open_proc_pid (getpid ()); + unsigned long long my_start_time = get_proc_pid_start_time (my_pid_dirfd); + close (my_pid_dirfd); + + debug ("peer start time: %llu, my start time: %llu", ws_start_time, my_start_time); + + /* Guard against pid recycling: If a malicious user captures ws, keeps the socket in a forked child and exits + * the original pid, they can trap a different user to login, get the old pid (pointing ot their cgroup), and + * capture their session. To prevent that, require that ws must have started earlier than ourselves. */ + if (my_start_time < ws_start_time) + { + warnx ("start time of this process (%llu) is older than cockpit-ws (%llu), pid recycling attack?", + my_start_time, ws_start_time); + close (ws_pid_dirfd); + exit_init_problem ("access-denied", "implausible cockpit-ws start time"); + } + } + else { + debug ("failed to read stdin peer credentials: %m; not in socket mode, reading cgroup from my own pid %d", getpid ()); + ws_pid_dirfd = open_proc_pid (getpid ()); + } + size_t ws_cgroup_length; - char *ws_cgroup = read_proc_self_cgroup (&ws_cgroup_length); + char *ws_cgroup = read_proc_pid_cgroup (ws_pid_dirfd, &ws_cgroup_length); assert (ws_cgroup); + close (ws_pid_dirfd); + + /* read_proc_pid_cgroup() already ensures that, but just in case we refactor this: this is *essential* for the + * subsequent comparison */ + if (ws_cgroup[ws_cgroup_length - 1] != '\n') + errx (EX, "cgroup does not end in newline"); + /* A simple prefix comparison is appropriate here because ws_cgroup - * will contain exactly one newline (at the end), and the expected + * contains exactly one newline (at the end), and the expected * value of ws_cgroup is on the first line in cert_pem. */ if (strncmp (cert_pem, ws_cgroup, ws_cgroup_length) != 0) From 4e326486782dc0fdcd030ed8063e12009c78dc8d Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Wed, 4 Jan 2023 19:07:05 +0100 Subject: [PATCH 2/5] tools: Move cockpit-session.socket to cockpit-ws package --- tools/cockpit.spec | 4 ++-- tools/debian/cockpit-tests.install | 2 -- tools/debian/cockpit-ws.install | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/cockpit.spec b/tools/cockpit.spec index a38ce6d154e..a47fe05ef70 100644 --- a/tools/cockpit.spec +++ b/tools/cockpit.spec @@ -378,6 +378,8 @@ authentication via sssd/FreeIPA. %{_unitdir}/cockpit-motd.service %{_unitdir}/cockpit.socket %{_unitdir}/cockpit-ws-user.service +%{_unitdir}/cockpit-session.socket +%{_unitdir}/cockpit-session@.service %{_unitdir}/cockpit-wsinstance-http.socket %{_unitdir}/cockpit-wsinstance-http.service %{_unitdir}/cockpit-wsinstance-https-factory.socket @@ -560,8 +562,6 @@ These files are not required for running Cockpit. %files -n cockpit-tests -f tests.list %{pamdir}/mock-pam-conv-mod.so -%{_unitdir}/cockpit-session.socket -%{_unitdir}/cockpit-session@.service %package -n cockpit-packagekit Summary: Cockpit user interface for packages diff --git a/tools/debian/cockpit-tests.install b/tools/debian/cockpit-tests.install index ef197f163f6..42166587f7a 100644 --- a/tools/debian/cockpit-tests.install +++ b/tools/debian/cockpit-tests.install @@ -1,3 +1 @@ usr/share/cockpit/playground -${env:deb_systemdsystemunitdir}/cockpit-session.socket -${env:deb_systemdsystemunitdir}/cockpit-session@.service diff --git a/tools/debian/cockpit-ws.install b/tools/debian/cockpit-ws.install index a19fb2111e5..8e470709f81 100644 --- a/tools/debian/cockpit-ws.install +++ b/tools/debian/cockpit-ws.install @@ -3,6 +3,8 @@ etc/pam.d/cockpit ${env:deb_systemdsystemunitdir}/cockpit.service ${env:deb_systemdsystemunitdir}/cockpit-motd.service ${env:deb_systemdsystemunitdir}/cockpit.socket +${env:deb_systemdsystemunitdir}/cockpit-session.socket +${env:deb_systemdsystemunitdir}/cockpit-session@.service ${env:deb_systemdsystemunitdir}/cockpit-ws-user.service ${env:deb_systemdsystemunitdir}/cockpit-wsinstance-http.service ${env:deb_systemdsystemunitdir}/cockpit-wsinstance-http.socket From cb05ee1a06d06895acf23353d616dff0f44ef5df Mon Sep 17 00:00:00 2001 From: Allison Karlitskaya Date: Mon, 10 Jan 2022 18:15:17 +0100 Subject: [PATCH 3/5] ws: connect to cockpit-session via socket Unless it's otherwise specified in the configuration file, we now spawn cockpit-session by connecting to /run/cockpit/session if that exists. We leave the cockpit_ws_session_program variable in place to allow the tests to override things. Update the unit files for cockpit-ws to ensure that the socket is available when cockpit-ws is running. Adjust TestConnection.testBasic accordingly: When running cockpit-session via unix socket activation, its group permissions are irrelevant. Break the socket instead. Also adjust the reverse proxy tests which start the `cockpit-ws` binary directly to ensure that the session socket is aware. A custom production setup which doesn't use cockpit.socket will have to `Requires/After=cockpit-session.socket` as well to continue to function (as running cockpit-ws as root is undesirable). Drop `testAuthUnixPath`, as that's now the default. Instead, add a new `testAuthDirectSession` which tests a custom cockpit-ws setup that directly runs cockpit-session. Co-Authored-By: Martin Pitt --- .../cockpit-wsinstance-http.service.in | 2 + .../cockpit-wsinstance-https@.service.in | 2 + src/ws/cockpitauth.c | 11 +++- test/verify/check-connection | 52 ++++++++++++------- 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/systemd/cockpit-wsinstance-http.service.in b/src/systemd/cockpit-wsinstance-http.service.in index 97af8abccb7..a6860f26ba5 100644 --- a/src/systemd/cockpit-wsinstance-http.service.in +++ b/src/systemd/cockpit-wsinstance-http.service.in @@ -2,6 +2,8 @@ Description=Cockpit Web Service http instance Documentation=man:cockpit-ws(8) BindsTo=cockpit.service +Requires=cockpit-session.socket +After=cockpit-session.socket [Service] ExecStart=@libexecdir@/cockpit-ws --no-tls --port=0 diff --git a/src/systemd/cockpit-wsinstance-https@.service.in b/src/systemd/cockpit-wsinstance-https@.service.in index f4a3517b643..b7a3977e6d4 100644 --- a/src/systemd/cockpit-wsinstance-https@.service.in +++ b/src/systemd/cockpit-wsinstance-https@.service.in @@ -2,6 +2,8 @@ Description=Cockpit Web Service https instance %I Documentation=man:cockpit-ws(8) BindsTo=cockpit.service +Requires=cockpit-session.socket +After=cockpit-session.socket [Service] Slice=system-cockpithttps.slice diff --git a/src/ws/cockpitauth.c b/src/ws/cockpitauth.c index f6d56d10cdd..35741897599 100644 --- a/src/ws/cockpitauth.c +++ b/src/ws/cockpitauth.c @@ -54,7 +54,9 @@ const gchar *cockpit_ws_ssh_program = "/usr/bin/env python3 -m cockpit.beiboot --remote-bridge=supported"; /* Some tunables that can be set from tests */ -const gchar *cockpit_ws_session_program = LIBEXECDIR "/cockpit-session"; +const gchar *cockpit_ws_session_program = NULL; + +#define WS_SESSION_SOCKET "/run/cockpit/session" /* Timeout of authenticated session when no connections */ guint cockpit_ws_service_idle = 15; @@ -1116,7 +1118,12 @@ cockpit_session_launch (CockpitAuth *self, g_str_equal (type, "tls-cert")) { if (command == NULL && unix_path == NULL) - command = cockpit_ws_session_program; + { + if (cockpit_ws_session_program) + command = cockpit_ws_session_program; + else + unix_path = WS_SESSION_SOCKET; + } } g_autoptr(CockpitPipe) pipe = NULL; diff --git a/test/verify/check-connection b/test/verify/check-connection index f4140adbfa6..acaa53cf43d 100755 --- a/test/verify/check-connection +++ b/test/verify/check-connection @@ -161,15 +161,18 @@ class TestConnection(testlib.MachineCase): self.assertNoAdminProcessLeaks() if not m.ws_container: # no cockpit-session - # damage cockpit-session permissions → "Authentication not available" + # damage cockpit-session socket → "Authentication not available" m.execute(f"chmod g-x {self.libexecdir}/cockpit-session") - b.open("/system") - b.try_login() - # .. but the login page does not expose the error very specifically - b.wait_in_text('#login-error-title', "Authentication failed") - m.execute(f"chmod g+x {self.libexecdir}/cockpit-session") + try: + m.execute("mv /run/cockpit/session /run/cockpit/session.disabled") + b.open("/system") + b.try_login() + # .. but the login page does not expose the error very specifically + b.wait_in_text('#login-error-title', "Authentication failed") + finally: + m.execute("mv /run/cockpit/session.disabled /run/cockpit/session") - self.allow_journal_messages(".*cockpit-session: bridge program failed.*") + self.allow_journal_messages("/run/cockpit/session: couldn't connect.*") # pretend cockpit-bridge is not installed, expect specific error message m.execute("while B=$(command -v cockpit-bridge); do mv $B ${B}.disabled; done") @@ -1001,6 +1004,7 @@ until pgrep -f '^(/usr/[^ ]+/[^ /]*python[^ /]* )?/usr/bin/cockpit-bridge'; do s m.spawn("socat TCP-LISTEN:9091,reuseaddr,fork TCP:localhost:9099", "socat.log") # ws with plain --no-tls should fail after login with mismatching Origin (expected http, got https) + m.execute("systemctl start cockpit-session.socket") m.spawn(f"runuser -u cockpit-wsinstance -- {self.ws_executable} --no-tls -p 9099", "ws-notls.log") m.wait_for_cockpit_running(tls=True) @@ -1135,28 +1139,36 @@ until pgrep -f '^(/usr/[^ ]+/[^ /]*python[^ /]* )?/usr/bin/cockpit-bridge'; do s b.wait_visible("#login") b.assert_pixels("body", "login-screen") - @testlib.skipWsContainer("cockpit/ws doesn't use systemd units") + @testlib.skipOstree("tests cockpit-ws package") + @testlib.skipWsContainer("tests cockpit-ws package") @testlib.nondestructive - def testAuthUnixPath(self): - """test UnixPath for auth method in cockpit.conf""" + def testAuthDirectSession(self): m = self.machine + b = self.browser - m.execute(['systemctl', 'start', 'cockpit-session.socket']) - self.addCleanup(m.execute, 'systemctl stop cockpit-session.socket') - m.write('/etc/cockpit/cockpit.conf', """ + m.stop_cockpit() + # make sure that this doesn't use the session socket + m.execute('systemctl stop cockpit-session.socket; systemctl mask cockpit-session.socket') + self.addCleanup(m.execute, 'systemctl unmask cockpit-session.socket') + m.write('/etc/cockpit/cockpit.conf', f""" [Negotiate] Action=none [Basic] -UnixPath=/run/cockpit/session +Command={self.libexecdir}/cockpit-session """) - # make sure this isn't being run via spawning - m.execute(f'chmod 700 {self.libexecdir}/cockpit-session') - self.addCleanup(m.execute, f'chmod 4750 {self.libexecdir}/cockpit-session') + # cockpit-ws has to run as root for this, as cockpit-session isn't suid root any more + m.execute(f"systemd-run --unit=custom-ws.service {self.libexecdir}/cockpit-ws --no-tls") + self.addCleanup(m.execute, "systemctl stop custom-ws.service") + m.wait_for_cockpit_running() - m.start_cockpit() - self.login_and_go("/system") + b.login_and_go(None) + b.wait_text('#current-username', 'admin') + b.logout() + + # Arch logs this to the journal + self.allow_journal_messages("cockpit-session:.*Activate the web console with.*") @testlib.skipWsContainer("no local config with cockpit/ws") @testlib.nondestructive @@ -1269,6 +1281,8 @@ ProtocolHeader = X-Forwarded-Proto """ % {"origin": m.forward["443"]}, append=True) m.execute("setsebool -P httpd_can_network_connect on") + # these tests are running the cockpit-ws binary directly, not via cockpit.socket + m.execute("systemctl start cockpit-session.socket") self.allow_journal_messages("audit.*bool=httpd_can_network_connect.*val=1.*") def callProxyCurl(self, path, *args): From 110b45f6108bb468ef86d4e794ca290fd8c52c59 Mon Sep 17 00:00:00 2001 From: Allison Karlitskaya Date: Mon, 10 Jan 2022 18:20:45 +0100 Subject: [PATCH 4/5] cockpit-session: stop installing setuid root systemd spawns this for us now, so we don't need the setuid bit anymore. Clean up the statoverride in the Debian packaging on upgrades. --- src/session/Makefile-session.am | 5 ----- tools/arch/PKGBUILD | 2 -- tools/cockpit.spec | 2 +- tools/debian/cockpit-ws.postinst | 7 +++++-- tools/debian/cockpit-ws.postrm | 2 -- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/session/Makefile-session.am b/src/session/Makefile-session.am index 31c82cc0ac8..44fca44144a 100644 --- a/src/session/Makefile-session.am +++ b/src/session/Makefile-session.am @@ -20,8 +20,3 @@ cockpit_session_SOURCES = \ src/session/session-utils.h \ src/session/session.c \ $(NULL) - -# set up cockpit-session to be setuid root, but only runnable by cockpit-session -install-exec-hook:: - chown -f root:cockpit-wsinstance $(DESTDIR)$(libexecdir)/cockpit-session || true - chmod -f 4750 $(DESTDIR)$(libexecdir)/cockpit-session || true diff --git a/tools/arch/PKGBUILD b/tools/arch/PKGBUILD index 6722c7b5341..693465e0111 100644 --- a/tools/arch/PKGBUILD +++ b/tools/arch/PKGBUILD @@ -60,8 +60,6 @@ package_cockpit() { rm -rf "$pkgdir"/usr/{src,lib/firewalld} install -Dm644 "$srcdir"/cockpit.pam "$pkgdir"/etc/pam.d/cockpit - echo "z /usr/lib/cockpit/cockpit-session - - cockpit-wsinstance -" >> "$pkgdir"/usr/lib/tmpfiles.d/cockpit-ws.conf - # remove unused plugins rm -rf "$pkgdir"/usr/share/cockpit/{selinux,playground,sosreport} \ "$pkgdir"/usr/share/metainfo/org.cockpit_project.cockpit_{selinux,sosreport}.metainfo.xml diff --git a/tools/cockpit.spec b/tools/cockpit.spec index a47fe05ef70..ca23a65f3b7 100644 --- a/tools/cockpit.spec +++ b/tools/cockpit.spec @@ -399,7 +399,7 @@ authentication via sssd/FreeIPA. %{_libexecdir}/cockpit-desktop %{_libexecdir}/cockpit-certificate-ensure %{_libexecdir}/cockpit-certificate-helper -%attr(4750, root, cockpit-wsinstance) %{_libexecdir}/cockpit-session +%{_libexecdir}/cockpit-session %{_datadir}/cockpit/branding %{_datadir}/selinux/packages/%{selinuxtype}/%{name}.pp.bz2 %{_mandir}/man8/%{name}_session_selinux.8cockpit.* diff --git a/tools/debian/cockpit-ws.postinst b/tools/debian/cockpit-ws.postinst index f97f74d5636..9017d6829a2 100644 --- a/tools/debian/cockpit-ws.postinst +++ b/tools/debian/cockpit-ws.postinst @@ -3,8 +3,11 @@ set -e #DEBHELPER# -if ! dpkg-statoverride --list /usr/lib/cockpit/cockpit-session >/dev/null; then - dpkg-statoverride --update --add root cockpit-wsinstance 4750 /usr/lib/cockpit/cockpit-session +# remove dpkg-statoverride on upgrade +if dpkg-statoverride --list /usr/lib/cockpit/cockpit-session >/dev/null; then + dpkg-statoverride --remove /usr/lib/cockpit/cockpit-session + chmod 755 /usr/lib/cockpit/cockpit-session + chgrp root /usr/lib/cockpit/cockpit-session fi # restart cockpit.service on package upgrades, if it's already running diff --git a/tools/debian/cockpit-ws.postrm b/tools/debian/cockpit-ws.postrm index 7dec1d5d016..8267b0d55ca 100644 --- a/tools/debian/cockpit-ws.postrm +++ b/tools/debian/cockpit-ws.postrm @@ -8,6 +8,4 @@ if [ "$1" = purge ]; then [ -L /etc/motd.d/cockpit ] && rm /etc/motd.d/cockpit || true [ -L /etc/issue.d/cockpit.issue ] && rm /etc/issue.d/cockpit.issue || true rm -f /etc/cockpit/disallowed-users - - dpkg-statoverride --remove /usr/lib/cockpit/cockpit-session fi From 72bfb6cb435765e9e2f97da2ba6583018751dcb3 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Wed, 20 Nov 2024 14:16:09 +0100 Subject: [PATCH 5/5] session: Only support for cert auth with cockpit-session.socket This avoids an alternative code path which is unlikely to happen in practice, and which we don't test anywhere. --- src/session/client-certificate.c | 53 ++++++++++++++++---------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/session/client-certificate.c b/src/session/client-certificate.c index 51b37f74df1..25479c3f3be 100644 --- a/src/session/client-certificate.c +++ b/src/session/client-certificate.c @@ -354,40 +354,41 @@ cockpit_session_client_certificate_map_user (const char *client_certificate_file exit_init_problem ("authentication-unavailable", "No https instance certificate present"); } - /* check if stdin is a Unix socket; then we are being called via cockpit-session@.service and need to - * check the cgroup of our peer */ + /* We only support being called via cockpit-session.socket (i.e. Unix socket). + * We need to check the cgroup of cockpit-ws (our peer) */ struct ucred ucred; socklen_t ucred_len = sizeof ucred; - int ws_pid_dirfd; - if (getsockopt (STDIN_FILENO, SOL_SOCKET, SO_PEERCRED, &ucred, &ucred_len) == 0 && + if (getsockopt (STDIN_FILENO, SOL_SOCKET, SO_PEERCRED, &ucred, &ucred_len) != 0 || /* this is an inout parameter, be extra suspicious */ - ucred_len >= sizeof ucred) + ucred_len < sizeof ucred) { - debug ("unix socket mode, reading cgroup from peer pid %d", ucred.pid); - ws_pid_dirfd = open_proc_pid (ucred.pid); - unsigned long long ws_start_time = get_proc_pid_start_time (ws_pid_dirfd); + debug ("failed to read stdin peer credentials: %m; not in socket mode?"); + warnx ("Certificate authentication only supported with cockpit-session.socket"); + exit_init_problem ("authentication-unavailable", "Certificate authentication only supported with cockpit-session.socket"); + } - int my_pid_dirfd = open_proc_pid (getpid ()); - unsigned long long my_start_time = get_proc_pid_start_time (my_pid_dirfd); - close (my_pid_dirfd); + /* this is an inout parameter, be extra suspicious */ + assert (ucred_len >= sizeof ucred); + debug ("unix socket mode, reading cgroup from peer pid %d", ucred.pid); + int ws_pid_dirfd = open_proc_pid (ucred.pid); + unsigned long long ws_start_time = get_proc_pid_start_time (ws_pid_dirfd); - debug ("peer start time: %llu, my start time: %llu", ws_start_time, my_start_time); + int my_pid_dirfd = open_proc_pid (getpid ()); + unsigned long long my_start_time = get_proc_pid_start_time (my_pid_dirfd); + close (my_pid_dirfd); - /* Guard against pid recycling: If a malicious user captures ws, keeps the socket in a forked child and exits - * the original pid, they can trap a different user to login, get the old pid (pointing ot their cgroup), and - * capture their session. To prevent that, require that ws must have started earlier than ourselves. */ - if (my_start_time < ws_start_time) - { - warnx ("start time of this process (%llu) is older than cockpit-ws (%llu), pid recycling attack?", - my_start_time, ws_start_time); - close (ws_pid_dirfd); - exit_init_problem ("access-denied", "implausible cockpit-ws start time"); - } + debug ("peer start time: %llu, my start time: %llu", ws_start_time, my_start_time); + + /* Guard against pid recycling: If a malicious user captures ws, keeps the socket in a forked child and exits + * the original pid, they can trap a different user to login, get the old pid (pointing ot their cgroup), and + * capture their session. To prevent that, require that ws must have started earlier than ourselves. */ + if (my_start_time < ws_start_time) + { + warnx ("start time of this process (%llu) is older than cockpit-ws (%llu), pid recycling attack?", + my_start_time, ws_start_time); + close (ws_pid_dirfd); + exit_init_problem ("access-denied", "implausible cockpit-ws start time"); } - else { - debug ("failed to read stdin peer credentials: %m; not in socket mode, reading cgroup from my own pid %d", getpid ()); - ws_pid_dirfd = open_proc_pid (getpid ()); - } size_t ws_cgroup_length; char *ws_cgroup = read_proc_pid_cgroup (ws_pid_dirfd, &ws_cgroup_length);