systemd: Lock down cockpit-ws and -tls #21305

martinpitt · 2024-11-22T14:02:40Z

Now that cockpit-ws does not directly fork cockpit-session, and sessions
run in their own service/cgroup, we can heavily lock down our webserver
(which is the weakest component in Cockpit). It only needs to talk to
cockpit-tls over stdin/out and cockpit-session over the Unix socket,
and call SSH for remote host support (so we can't lock down networking
completely).

DynamicUser= already implies the biggest restrictions, such as
ProtectSystem=full, ProtectHome, PrivateTmp, and more. We can even
work with ProtectSystem=strict (i.e. everything being read-only except
the service's /run directory and tmp dir).

Fixes #21299
https://issues.redhat.com/browse/COCKPIT-1206

allisonkarlitskaya

This is a good first chunk of stuff, but I think we can go a lot further.

allisonkarlitskaya · 2024-11-22T14:59:20Z

src/systemd/[email protected]

+SystemCallFilter=@system-service
+
+# cockpit-tls does all our outside networking
+PrivateNetwork=yes


Actually, I tried turning on PrivateNetwork in cockpit-tls and it didn't seem to break anything. That's not surprising: systemd hands its listener socket to it and its outbound connections are via UNIX sockets on the filesystem.

Which is to say: we should probably also add more sandboxing to cockpit-tls.

Aww dang -- we can't do that. cockpit-ws runs beiboot and ssh for remote hosts/bastion, and that does need networking. That's what's breaking all the multi-machine tests.

allisonkarlitskaya · 2024-11-22T15:01:01Z

src/systemd/cockpit-wsinstance-http.service.in

+RestrictAddressFamilies=AF_UNIX
+
+# extra protection for our TLS keys -- only cockpit-tls should read them
+InaccessiblePaths=-/etc/cockpit/ws-certs.d


I don't care for this at all. We're a dynamic user, so the only way we can read those is if they were world-readable. But then we could read everyone else's incorrectly world-readable data.

I'd prefer if you take a much more aggressive approach here and move to whitelisting: we should only be allowed to see:

/usr

/etc/cockpit/cockpit.conf

the /run/cockpit/session socket

Yup, I just pushed an initial attempt after some light testing. There are already some failures, so some work to do.

Locking this down further is nice of course. Need to see how that plays out with PrivateTmp=, i.e. which one wins.

.. plus, need to allow whatever ssh needs to read, i. e. at least /etc/ssh/. It likely also requires reading $HOME for ssh keys..

bastion host configurations have no business reading anyone's ssh keys from $HOME...

I started with InaccessiblePaths=/ but this is too much of a ratsnest. (Plus, it's not even documented to work, as these options don't have an ordering/priority). Think /etc/nsswitch.conf an ld.so.conf and whatnot. ProtectSystem=strict is a very good starting point, and together with ProtectHome (which is implied by DynamicUser) is a robust basis. You could spend endless hours tracking it down to a list of 50 files, but this will also change over time, is too specific to be reliable across different system configurations, and just isn't worth it IMHO -- having pretty much everything being read-only gives us what we need.

Think about some people (cough, cough) using a Yubikey for SSH auth and requiring extra configs/libraries.

Think about some people (cough, cough) using a Yubikey for SSH auth and requiring extra configs/libraries.

This wouldn't require access to the Yubikey on the bastion host and, indeed, we'd actually want to very strictly isolate SSH on the bastion host for exactly this reason.

But there are some plausible causes where ssh on the bastion host would need to see more of the system:

we might imagine ssh ends up using systemd-resolved and needing to talk to it via D-Bus. That's a whole lot of stuff that ends up getting pulled in.

we might imagine some sssd-type setup where we feed hostkeys to ssh in some dynamic way backed by some kind of a network database. This could end up using a whole lot of random stuff around.

So, rough mid/longterm plan:

get cockpit-beiboot separated from -ws via the same socket activation tricks

after ssh is out of the picture, we can go insane on restrictions on -ws

allisonkarlitskaya · 2024-11-22T16:16:20Z

src/systemd/[email protected]

+MemoryDenyWriteExecute=true
+SystemCallFilter=@system-service
+
+# cockpit-tls does all our outside web related networking, but ws also calls ssh


Indeed I forgot about this, but.... this is a bit sad when ConnectTo isn't enabled by default...

I wonder if we could do something about that. Isolating cockpit-ws to its own network namespace would be a pretty big win.

Clearly we need socket-activated beiboot 😅

Quite honestly, that sounds right. It'd isolate SSH sessions in the same manner as local PAM ones, and would allow us to fully lock down ws. (But I don't want to do that in this PR)

martinpitt · 2024-11-24T10:29:11Z

For tracking back: commit 042df74 is a set of working restrictions which is green, but still without path restrictions. Trying these next.

Now that cockpit-ws does not directly fork cockpit-session, and sessions run in their own service/cgroup, we can heavily lock down our webserver (which is the weakest component in Cockpit). It only needs to talk to cockpit-tls over stdin/out and cockpit-session over the Unix socket, and call SSH for remote host support (so we can't lock down networking completely). `DynamicUser=` already implies the biggest restrictions, such as `ProtectSystem=full`, `ProtectHome`, `PrivateTmp`, and more. We can even work with `ProtectSystem=strict` (i.e. everything being read-only except the service's /run directory and tmp dir). Fixes cockpit-project#21299 https://issues.redhat.com/browse/COCKPIT-1206

Consistently use "yes" for boolean options. `DynamicUser` already implies `PrivateTmp` and `ProtectHome`.

cockpit-tls speaks to the outside world via socket activation, and to cockpit-ws via Unix socket. So it doesn't require much networking on its own.

allisonkarlitskaya

Honestly, this all looks like a huge win. I'm sure we could turn the screws even tighter, but let's get this stuff in now so that it makes the release.

Next steps:

beiboot isolation
network isolation for -ws
filesystem-based restrictions

allisonkarlitskaya · 2024-11-25T09:50:44Z

src/systemd/cockpit.service.in

+
+# cockpit-tls speaks to the outside world via socket activation, and to cockpit-ws via Unix socket
+PrivateIPC=yes
+PrivateNetwork=yes


I like that we deprive our network-facing component of the ability to interact with the network. This feels really nice.

martinpitt added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Nov 22, 2024

allisonkarlitskaya requested changes Nov 22, 2024

View reviewed changes

martinpitt force-pushed the ws-lockdown branch from 75feac2 to 042df74 Compare November 22, 2024 15:29

allisonkarlitskaya reviewed Nov 22, 2024

View reviewed changes

martinpitt removed the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Nov 24, 2024

martinpitt force-pushed the ws-lockdown branch from 042df74 to 9095b22 Compare November 24, 2024 11:06

martinpitt marked this pull request as ready for review November 24, 2024 13:01

martinpitt requested a review from allisonkarlitskaya November 24, 2024 13:02

martinpitt added 3 commits November 25, 2024 04:03

systemd: Clean up cockpit.service.in

ac83f28

Consistently use "yes" for boolean options. `DynamicUser` already implies `PrivateTmp` and `ProtectHome`.

systemd: Lock down cockpit-tls networking

fe6813f

cockpit-tls speaks to the outside world via socket activation, and to cockpit-ws via Unix socket. So it doesn't require much networking on its own.

martinpitt force-pushed the ws-lockdown branch from 9095b22 to fe6813f Compare November 25, 2024 03:06

martinpitt changed the title ~~systemd: Lock down cockpit-ws~~ systemd: Lock down cockpit-ws and -tls Nov 25, 2024

allisonkarlitskaya approved these changes Nov 25, 2024

View reviewed changes

allisonkarlitskaya merged commit 4aa8fb3 into cockpit-project:main Nov 25, 2024
85 checks passed

martinpitt deleted the ws-lockdown branch November 25, 2024 09:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

systemd: Lock down cockpit-ws and -tls #21305

systemd: Lock down cockpit-ws and -tls #21305

martinpitt commented Nov 22, 2024 •

edited

Loading

allisonkarlitskaya left a comment

allisonkarlitskaya Nov 22, 2024

martinpitt Nov 22, 2024

allisonkarlitskaya Nov 22, 2024

martinpitt Nov 22, 2024

martinpitt Nov 22, 2024

allisonkarlitskaya Nov 22, 2024

martinpitt Nov 24, 2024

martinpitt Nov 24, 2024

allisonkarlitskaya Nov 25, 2024

allisonkarlitskaya Nov 22, 2024

allisonkarlitskaya Nov 22, 2024

martinpitt Nov 24, 2024

martinpitt commented Nov 24, 2024

allisonkarlitskaya left a comment

allisonkarlitskaya Nov 25, 2024

systemd: Lock down cockpit-ws and -tls #21305

systemd: Lock down cockpit-ws and -tls #21305

Conversation

martinpitt commented Nov 22, 2024 • edited Loading

allisonkarlitskaya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpitt commented Nov 24, 2024

allisonkarlitskaya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpitt commented Nov 22, 2024 •

edited

Loading