Skip to content

Commit

Permalink
systemd: further cleanups for DynamicUser setup
Browse files Browse the repository at this point in the history
We need to have separate units to create dynamic users for our socket
ownership[^1] (since this feature is not supported for `.socket` units in
systemd) but we don't need to create our service users that way.

Move to using `DynamicUser=yes` directly in our relevant `.service`
files.  In the case of `cockpit.service` this allows us to drop the
separate `cockpit-ws-user.service`.  For the `cockpit-wsinstance-*`
services we do that instead of running as the
`cockpit-wsinstance-socket`, which was a bit broken.

`SupplementaryGroups=` seems not to work well together with
`DynamicUser=true`[^2][^3] so use `Group=` instead, putting
`cockpit-tls` and `cockpit-ws` into the group of the socket that it
needs to connect to.

We allow systemd to automatically name the `cockpit-ws` users, for which
it picks the usernames `cockpit-wsinstance-https` and
`cockpit-wsinstance-http`.  Disappointingly, it doesn't use per-instance
names for these (which is the documented behaviour[^4]). If we attempt
to force the issue by providing our own `User=` containing `%i` then we
get a hint about why that might be the case: the created usernames are
far too long, causing issues which prevent the service from starting
(since our instance IDs are sha256 digests).

We *don't* allow systemd to pick the name for the `cockpit.service` unit
because it would select the username `cockpit` and it would use this
username even if such a (real) user already existed on the system, which
seems a) somewhat possible, and b) quite bad.  Force it to use a
less-likely-to-exist name: `cockpit-systemd-service`.  This also means we
no longer use the username `cockpit-ws` for anything at all (which might
be present on old systems, and might have access to read the TLS
certificate and private keys, under old configurations).

Now that we no longer depend on a separate unit to create the users that
our services run as we can also remove the redundant `Requires=` and
`After=` lines for the socket users that we depend on.  We already
`Requires=`/`After=` the sockets themselves and the sockets have those
dependencies on the users.

[^1]: systemd/systemd#23067
[^2]: systemd/systemd#26636
[^3]: systemd/systemd#8219
[^4]: https://0pointer.net/blog/dynamic-users-with-systemd.html (search for `[email protected]`)
  • Loading branch information
allisonkarlitskaya authored and martinpitt committed Nov 22, 2024
1 parent 12c2b1e commit 9d8226d
Show file tree
Hide file tree
Showing 11 changed files with 19 additions and 40 deletions.
1 change: 0 additions & 1 deletion src/systemd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ nodist_systemdunit_DATA = \
dist_systemdunit_DATA = \
src/systemd/cockpit-session.socket \
src/systemd/cockpit-session-socket-user.service \
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
14 changes: 0 additions & 14 deletions src/systemd/cockpit-ws-user.service

This file was deleted.

9 changes: 4 additions & 5 deletions src/systemd/cockpit-wsinstance-http.service.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@
Description=Cockpit Web Service http instance
Documentation=man:cockpit-ws(8)
BindsTo=cockpit.service
Requires=cockpit-session.socket cockpit-session-socket-user.service
After=cockpit-session.socket cockpit-session-socket-user.service
Requires=cockpit-session.socket
After=cockpit-session.socket

[Service]
ExecStart=@libexecdir@/cockpit-ws --no-tls --port=0
User=cockpit-wsinstance-socket
Group=cockpit-wsinstance-socket
SupplementaryGroups=cockpit-session-socket
DynamicUser=true
Group=cockpit-session-socket
4 changes: 2 additions & 2 deletions src/systemd/cockpit-wsinstance-http.socket
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ 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 cockpit-wsinstance-socket-user.service
After=cockpit-ws-user.service cockpit-wsinstance-socket-user.service
Requires=cockpit-wsinstance-socket-user.service
After=cockpit-wsinstance-socket-user.service

[Socket]
ListenStream=/run/cockpit/wsinstance/http.sock
Expand Down
4 changes: 2 additions & 2 deletions src/systemd/cockpit-wsinstance-https-factory.socket
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ 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 cockpit-wsinstance-socket-user.service
After=cockpit-ws-user.service cockpit-wsinstance-socket-user.service
Requires=cockpit-wsinstance-socket-user.service
After=cockpit-wsinstance-socket-user.service

[Socket]
ListenStream=/run/cockpit/wsinstance/https-factory.sock
Expand Down
9 changes: 4 additions & 5 deletions src/systemd/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@
Description=Cockpit Web Service https instance %I
Documentation=man:cockpit-ws(8)
BindsTo=cockpit.service
Requires=cockpit-session.socket cockpit-session-socket-user.service
After=cockpit-session.socket cockpit-session-socket-user.service
Requires=cockpit-session.socket
After=cockpit-session.socket

[Service]
Slice=system-cockpithttps.slice
ExecStart=@libexecdir@/cockpit-ws --for-tls-proxy --port=0
User=cockpit-wsinstance-socket
Group=cockpit-wsinstance-socket
SupplementaryGroups=cockpit-session-socket
DynamicUser=yes
Group=cockpit-session-socket
4 changes: 2 additions & 2 deletions src/systemd/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ BindsTo=cockpit.service
# the services are resource-limited by system-cockpithttps.slice
BindsTo=cockpit-wsinstance-https@%i.service
# ensure our DynamicUser exists
Requires=cockpit-ws-user.service cockpit-wsinstance-socket-user.service
After=cockpit-ws-user.service cockpit-wsinstance-socket-user.service
Requires=cockpit-wsinstance-socket-user.service
After=cockpit-wsinstance-socket-user.service

[Socket]
ListenStream=/run/cockpit/wsinstance/https@%i.sock
Expand Down
10 changes: 4 additions & 6 deletions src/systemd/cockpit.service.in
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@ 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 cockpit-wsinstance-socket-user.service
After=cockpit-ws-user.service cockpit-wsinstance-socket-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]
RuntimeDirectory=cockpit/tls
ExecStartPre=+@libexecdir@/cockpit-certificate-ensure --for-cockpit-tls
ExecStart=@libexecdir@/cockpit-tls
User=cockpit-ws
Group=cockpit-ws
SupplementaryGroups=cockpit-wsinstance-socket
DynamicUser=yes
# otherwise systemd uses 'cockpit' even if it exists as a normal user account
User=cockpit-systemd-service
Group=cockpit-wsinstance-socket
NoNewPrivileges=true
ProtectSystem=strict
ProtectHome=true
Expand Down
2 changes: 1 addition & 1 deletion test/verify/check-connection
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ class TestConnection(testlib.MachineCase):
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")
m.execute("systemctl stop cockpit-wsinstance-socket-user")
expect_actives(ws_socket=True, instance_sockets=False, http_instances=[])

# https mode
Expand Down
1 change: 0 additions & 1 deletion tools/cockpit.spec
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,6 @@ authentication via sssd/FreeIPA.
%{_unitdir}/cockpit.service
%{_unitdir}/cockpit-motd.service
%{_unitdir}/cockpit.socket
%{_unitdir}/cockpit-ws-user.service
%{_unitdir}/cockpit-session-socket-user.service
%{_unitdir}/cockpit-session.socket
%{_unitdir}/[email protected]
Expand Down
1 change: 0 additions & 1 deletion tools/debian/cockpit-ws.install
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ ${env:deb_systemdsystemunitdir}/cockpit.socket
${env:deb_systemdsystemunitdir}/cockpit-session.socket
${env:deb_systemdsystemunitdir}/[email protected]
${env:deb_systemdsystemunitdir}/cockpit-session-socket-user.service
${env:deb_systemdsystemunitdir}/cockpit-ws-user.service
${env:deb_systemdsystemunitdir}/cockpit-wsinstance-http.service
${env:deb_systemdsystemunitdir}/cockpit-wsinstance-http.socket
${env:deb_systemdsystemunitdir}/[email protected]
Expand Down

0 comments on commit 9d8226d

Please sign in to comment.