Skip to content

Commit

Permalink
tools: Use DynamicUser for cockpit.service
Browse files Browse the repository at this point in the history
Since commit 644116a, the webserver certificates don't have to
be owned by the cockpit-ws user/group any more. This allows us to use
`DynamicUser` for cockpit.service, which eliminates the persistent
`cockpit-ws` system user.

The ordering matters a lot here to avoid race conditions: The wsinstance
socket units need to start before cockpit.service (so the the first
incoming request through cockpit.socket can get forwarded to an
instance), but after the dynamic user got created. So we can't have
cockpit.service create that dynamic user by itself. Instead, create a
separate `cockpit-ws-user.service` helper unit that orders itself in
between. Thanks to Allison Karlitskaya for the idea!

Note that we can't yet eliminate `cockpit-wsinstance` as that's the
owner of our `cockpit-session` suid root binary.
  • Loading branch information
martinpitt committed May 7, 2024
1 parent 8cb9d14 commit fd2c7d4
Show file tree
Hide file tree
Showing 12 changed files with 35 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/systemd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ nodist_systemdunit_DATA = \

dist_systemdunit_DATA = \
src/systemd/cockpit-session.socket \
src/systemd/cockpit-ws-user.service \
src/systemd/system-cockpithttps.slice \
src/systemd/cockpit-wsinstance-http.socket \
src/systemd/cockpit-wsinstance-https-factory.socket \
Expand Down
12 changes: 12 additions & 0 deletions src/systemd/cockpit-ws-user.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[Unit]
Description=Dynamic user for cockpit-ws
Documentation=man:cockpit-ws(8)
BindsTo=cockpit.service

[Service]
DynamicUser=yes
User=cockpit-ws
Group=cockpit-ws
Type=oneshot
ExecStart=/bin/true
RemainAfterExit=yes
3 changes: 3 additions & 0 deletions src/systemd/cockpit-wsinstance-http.socket
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
Description=Socket for Cockpit Web Service http instance
Documentation=man:cockpit-ws(8)
BindsTo=cockpit.service
# ensure our DynamicUser exists
Requires=cockpit-ws-user.service
After=cockpit-ws-user.service

[Socket]
ListenStream=/run/cockpit/wsinstance/http.sock
Expand Down
3 changes: 3 additions & 0 deletions src/systemd/cockpit-wsinstance-https-factory.socket
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
Description=Socket for Cockpit Web Service https instance factory
Documentation=man:cockpit-ws(8)
BindsTo=cockpit.service
# ensure our DynamicUser exists
Requires=cockpit-ws-user.service
After=cockpit-ws-user.service

[Socket]
ListenStream=/run/cockpit/wsinstance/https-factory.sock
Expand Down
3 changes: 3 additions & 0 deletions src/systemd/cockpit-wsinstance-https@.socket
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ BindsTo=cockpit.service
# this also effectively prevents a DoS by starting arbitrarily many sockets, as
# the services are resource-limited by system-cockpithttps.slice
BindsTo=cockpit-wsinstance-https@%i.service
# ensure our DynamicUser exists
Requires=cockpit-ws-user.service
After=cockpit-ws-user.service

[Socket]
ListenStream=/run/cockpit/wsinstance/https@%i.sock
Expand Down
4 changes: 4 additions & 0 deletions src/systemd/cockpit.service.in
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ Description=Cockpit Web Service
Documentation=man:cockpit-ws(8)
Requires=cockpit.socket
Requires=cockpit-wsinstance-http.socket cockpit-wsinstance-https-factory.socket
# ensure our DynamicUser exists
Requires=cockpit-ws-user.service
After=cockpit-ws-user.service
# we need to start after the sockets so that we can instantly forward incoming requests
After=cockpit-wsinstance-http.socket cockpit-wsinstance-https-factory.socket

[Service]
Expand Down
7 changes: 7 additions & 0 deletions test/verify/check-connection
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,13 @@ class TestConnection(testlib.MachineCase):
self.assertIn("HTTP/1.1 200 OK", m.execute("curl --silent --head http://127.0.0.1:9090"))
expect_actives(ws_socket=True, instance_sockets=True, http_instances=["http"])

# cleans up instances when killing the DynamicUser helper unit
m.start_cockpit(tls=False)
self.assertIn("HTTP/1.1 200 OK", m.execute("curl --silent --head http://127.0.0.1:9090"))
expect_actives(ws_socket=True, instance_sockets=True, http_instances=["http"])
m.execute("systemctl stop cockpit-ws-user.service")
expect_actives(ws_socket=True, instance_sockets=False, http_instances=[])

# https mode

m.start_cockpit(tls=True)
Expand Down
3 changes: 0 additions & 3 deletions tools/arch/PKGBUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ makedepends=(krb5 libssh accountsservice json-glib glib-networking
python-build python-installer python-wheel)
source=("cockpit-${pkgver}.tar.xz"
"cockpit.pam"
"cockpit-ws.sysuser.conf"
"cockpit-wsinstance.sysuser.conf")
sha256sums=('SKIP'
'079bb6751214e642673f9e1212df2a17fed1a3cc6cfdd6375af2b68ed6ddd340'
'1ad9dad75858264778bd94799b60c651f7cc1c7f7fa1c54622174303e639287a'
'46ee8ecad7bc97ba588ab9471dde76e41c00daf40658902425626c3a1938b438')

prepare() {
Expand Down Expand Up @@ -61,7 +59,6 @@ package_cockpit() {
make DESTDIR="$pkgdir" install
rm -rf "$pkgdir"/usr/{src,lib/firewalld}
install -Dm644 "$srcdir"/cockpit.pam "$pkgdir"/etc/pam.d/cockpit
install -Dm644 "$srcdir"/cockpit-ws.sysuser.conf "$pkgdir"/usr/lib/sysusers.d/cockpit-ws.conf
install -Dm644 "$srcdir"/cockpit-wsinstance.sysuser.conf "$pkgdir"/usr/lib/sysusers.d/cockpit-wsinstance.conf

echo "z /usr/lib/cockpit/cockpit-session - - cockpit-wsinstance -" >> "$pkgdir"/usr/lib/tmpfiles.d/cockpit-ws.conf
Expand Down
1 change: 0 additions & 1 deletion tools/arch/cockpit-ws.sysuser.conf

This file was deleted.

3 changes: 1 addition & 2 deletions tools/cockpit.spec
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ authentication via sssd/FreeIPA.
%{_unitdir}/cockpit.service
%{_unitdir}/cockpit-motd.service
%{_unitdir}/cockpit.socket
%{_unitdir}/cockpit-ws-user.service
%{_unitdir}/cockpit-wsinstance-http.socket
%{_unitdir}/cockpit-wsinstance-http.service
%{_unitdir}/cockpit-wsinstance-https-factory.socket
Expand All @@ -419,8 +420,6 @@ authentication via sssd/FreeIPA.
%ghost %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{name}

%pre ws
getent group cockpit-ws >/dev/null || groupadd -r cockpit-ws
getent passwd cockpit-ws >/dev/null || useradd -r -g cockpit-ws -d /nonexisting -s /sbin/nologin -c "User for cockpit web service" cockpit-ws
getent group cockpit-wsinstance >/dev/null || groupadd -r cockpit-wsinstance
getent passwd cockpit-wsinstance >/dev/null || useradd -r -g cockpit-wsinstance -d /nonexisting -s /sbin/nologin -c "User for cockpit-ws instances" cockpit-wsinstance

Expand Down
1 change: 1 addition & 0 deletions tools/debian/cockpit-ws.install
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ tools/apparmor.d/cockpit-desktop etc/apparmor.d/
${env:deb_systemdsystemunitdir}/cockpit.service
${env:deb_systemdsystemunitdir}/cockpit-motd.service
${env:deb_systemdsystemunitdir}/cockpit.socket
${env:deb_systemdsystemunitdir}/cockpit-ws-user.service
${env:deb_systemdsystemunitdir}/cockpit-wsinstance-http.service
${env:deb_systemdsystemunitdir}/cockpit-wsinstance-http.socket
${env:deb_systemdsystemunitdir}/cockpit-wsinstance-https-factory@.service
Expand Down
1 change: 0 additions & 1 deletion tools/debian/cockpit-ws.postinst
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#!/bin/sh
set -e

adduser --system --group --home /nonexistent --no-create-home --quiet cockpit-ws
adduser --system --group --home /nonexistent --no-create-home --quiet cockpit-wsinstance

# change group of cockpit-session on upgrades (changed in version 203)
Expand Down

0 comments on commit fd2c7d4

Please sign in to comment.