/usr/libexec/cockpit-session has wrong owner in deployment

[martinpitt]

In https://github.com/cockpit-project/cockpit/issues/21201 we got a report about failing to deploy a bootc container with the cockpit-ws package:

FROM quay.io/centos-bootc/centos-bootc:stream9
RUN dnf -y install cockpit-ws cockpit-bridge
RUN systemctl enable cockpit.socket
EOF

You can build yourself, but for convenience I built that on github:

bootc switch ghcr.io/martinpitt/workstation-bootc@sha256:8f6f12fa630e49541281d32825ca16e534da7cfe0014074fc3d3fcc2550d648e

This command works, but after boot, cockpit-ws' cockpit-session program has a broken owner:

# id cockpit-wsinstance
uid=981(cockpit-wsinstance) gid=981(cockpit-wsinstance) groups=981(cockpit-wsinstance)

# ls -l /usr/libexec/cockpit-session 
-rwsr-x---. 1 root 995 57120 Jan  1  1970 /usr/libexec/cockpit-session

It needs to have group "cockpit-wsinstance".

A temporary workaround is

bootc usr-overlay
chgrp cockpit-wsinstance /usr/libexec/cockpit-session

but of course that doesn't stick, and enabling the overlay is ugly.

[martinpitt]

@sigulete just did a response in the original bug, which I'm forwarding here:

I dug deeper into this issue and it doesn’t seem to be a bootc bug. It is just the nature of how the workflow works.

Bootc relies on OCI containers to build and transport an image to be deployed in what we will call the server for the sake of this example. Each time the container image gets re-built, it will create a brand new deployment that will replace everything in /usr and everything that hasn't been locally changed in /etc.

Some applications including cockpit get an UID and GID assigned within a range. Sometimes, it is the same for various iterations until it is not, and the allocated UID/GID in the container for the new image will eventually differ from the previous one. As a consequence the file /etc/passwd in the container will be adjusted based on this UID/GID allocation.

If /etc/passwd was not changed in the server, then it will be replaced by bootc during the deployment of an upgrade, and all works fine. But if this file was locally modified, then the local version will prevail and it won't be replaced by the one from the container. If the administrator creates, modifies or removes users in the server, then /etc/passwd will be modified and it will never get updated.

I looked at my deployment and I found that it is not common to have files in /usr with non-root USERID or GROUPID. In my case, I found 10 files with non-root GROUPID and with the exception of cockpit all the rest are using system users, which are not defined in /etc/passwd. Consequently cockpit was the only application prone to this issue.

I resolved the problem creating my user within the container build, so /etc/passwd won't get modified locally after deployment and it will always be managed and updated by bootc. But this is something that won't be possible for a multiuser environment or similar use case.

You mentioned before that cockpit will eventually move to Dynamic Users, I suppose it will resolve this issue.

[sigulete]

I want to report that there are a few nuances with this approach, for some reason the /home/$USER directory is not being copied from the container to the server during installation when using ISO image (created by bootc-image-builder). I used the following script to fix it for now:

USER_NAME=<username>
USER_DIR=/var/home/$USER_NAME

if [ ! -d $USER_DIR ]
then
  mkdir $USER_DIR
  cp -r /etc/skel/.* $USER_DIR
  chown -R $USER_NAME:$USER_NAME $USER_DIR
fi

[cgwalters]

There's a lot more in https://containers.github.io/bootc/building/users-and-groups.html but yes basically stop using useradd for system services.

[cgwalters]

Also setuid binaries are bad

[martinpitt]

Well, cockpit-ws primarily wants to use /usr/lib/sysusers.d/cockpit-wsinstance.conf to create the sysuser, but that doesn't work yet in rpm-land, see https://github.com/rpm-software-management/rpm/issues/3073 (it was fixed upstream, but isn't available widely yet in Fedora and RHEL land). But also, the system user actually exists, it's just not applied correctly to the binary ownership.

Also setuid binaries are bad

Yup, I hear you. Getting rid of it is a ginormous pile of work (see linked PR above), but also, it's the file ownership here, not the suid-ness (that's fine).

[cgwalters]

But also, the system user actually exists, it's just not applied correctly to the binary ownership.

Yeah there's a conceptual conflict between sysusers and files owned by images (I suspect for the same reason this gets trickier with RPM).

I think for this case ideally we fatally error out at build time and require a static allocation at build time; the most practical is nss-altfiles; we just don't document today how to extend what we have in the base image for it.

---

That said, there is an entirely different approach which is basically have /usr/libexec/cockpit-session be owned by root and not setuid, and have a systemd-tmpfiles snippet (or maybe needs to be a unit) which copies it to /run and makes it setuid there and chowns it.

For everyone else's reference the PR Martin is talking about for cockpit that would drop the suid binary looks to be https://github.com/cockpit-project/cockpit/pull/16811

[cgwalters]

but also, it's the file ownership here, not the suid-ness (that's fine).

I think they are related, no? If the binary wasn't setuid it could just be owned by root:root like any other...

[martinpitt]

That said, there is an entirely different approach which is basically have /usr/libexec/cockpit-session be owned by root and not setuid, and have a systemd-tmpfiles snippet (or maybe needs to be a unit)

That's a hilariously clever and evil 😈 idea, thanks 🤓 ! That would make packaging dramatically easier, and also avoid all this ownership mess, and even unblock delivering this as a sysext. Need to check how this plays out with SELinux, and of course it does feel rather root-kit-y to have a writable suid root binary in /run... But it's worth discussing at least.

(Or finally carve out two weeks to fix all the fallout from making cockpit-session a systemd .path unit...)

[cgwalters]

Need to check how this plays out with SELinux, and of course it does feel rather root-kit-y to have a writable suid root binary in /run...

Dunno, on package systems the binary is writable in /usr/bin too...