Re: [PATCH 2/2] pidfd: add pidfdfs

[Christian Brauner <brauner@kernel.org>]

On Fri, Feb 23, 2024 at 12:55:07PM +0100, Christian Brauner wrote:
> > Apologies if this has already been reported or fixed but I did not see
> > anything on the mailing list.
> > 
> > On next-20240221 and next-20240222, with CONFIG_FS_PID=y, some of my
> > services such as abrtd, dbus, and polkit fail to start on my Fedora
> > machines, which causes further isssues like failing to start network
> > interfaces with NetworkManager. I can easily reproduce this in a Fedora
> > 39 QEMU virtual machine, which has:
> > 
> >   # systemctl --version
> >   systemd 254 (254.9-1.fc39)
> 
> If something fails for completely inexplicable reasons:
> 
> Feb 23 12:09:58 fed1 audit[353]: AVC avc:  denied  { read write open } for  pid=353 comm="systemd-userdbd" path="pidfd:[709]" dev="pidfs" ino=709 scontext=system_u:system_r:systemd_userdbd_t:>
> 
> >   +SELINUX
> 
> pidfd creation can now be mediated by LSMs since we can finally go
> through the regular open path. That wasn't possible before but LSM
> mediation ability had been requested a few times.
> 
> In short, we have to update the selinux policy for Fedora. (Fwiw, went
> through the same excercise with nsfs back then.)
> 
> I've created a pull-request here:
> 
> https://github.com/fedora-selinux/selinux-policy/pull/2050
> 
> and filed an issue here:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=2265630
> 
> We have sufficient time to get this resolved and I was assured that this
> would be resolved. If we can't get it resolved in a timely manner we'll
> default to N for a while until everything's updated but I'd like to
> avoid that. I'll track that issue.

So I want to provide more context since I took the time to track this
all down in detail.

The failure you are seeing is indeed an selinux denial as I've pointed
out. The core failure is dbus-broker. That cascades into all the other
services failing. When dbus-broker fails to start polkit and all the
others won't be able to work because they depend on dbus-broker.

The reason for dbus-broker failing is because it doesn't handle failures
for SO_PEERPIDFD correctly. Last kernel release (either v6.7 or v6.6,
I'm not completely sure right now) we introduced SO_PEERPIDFD (and
SCM_PIDFD). SO_PEERPIDFD allows dbus-broker and polkit and others to
receive a pidfd for the peer of an AF_UNIX socket. This is the first
time in the history of Linux that we can safely authenticate clients in
a race-free manner. :)

dbus-broker immediately made use of this but messed up the error
checking. It only allowed EINVAL as a valid failure for SO_PEERPIDFD.
That's obviously problematic not just because of LSM denials but because
of seccomp denials that would prevent SO_PEERPIDFD from working; or any
other new error code from there.

So this is catching a flawed implementation in dbus-broker as well. It
_has_ to fallback to the old pid-based authentication when SO_PEERPIDFD
doesn't work no matter the reasons otherwise it'll always risk such
failures.

So, the immediate fix separate from the selinux policy update is to fix
dbus-broker which we've done now:

https://github.com/bus1/dbus-broker/pull/343

That should make it into Fedora asap as well.

The selinux reference policy is also updated in addition to the Fedora
policy:

https://github.com/bus1/dbus-broker/pull/343

So overall that LSM denial should not have caused dbus-broker to fail.
It can never assume that a feature released one kernel ago like
SO_PEERPIDFD can be assumed to be available.