Re: [PATCH] libselinux: Use sestatus if open

[Stephen Smalley <stephen.smalley.work@gmail.com>]

On Wed, Jul 15, 2020 at 2:52 PM Mike Palmiotto
<mike.palmiotto@crunchydata.com> wrote:
>
> On Wed, Jul 15, 2020 at 1:10 PM Mike Palmiotto
> <mike.palmiotto@crunchydata.com> wrote:
> >
> > On Wed, Jul 15, 2020 at 12:28 PM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > >
> > > On Wed, Jul 15, 2020 at 12:04 PM Mike Palmiotto
> > > <mike.palmiotto@crunchydata.com> wrote:
> > > >
> > > > On Tue, Jul 14, 2020 at 6:42 PM Mike Palmiotto
> > > > <mike.palmiotto@crunchydata.com> wrote:
> > > > >
> > > > > On Tue, Jul 14, 2020 at 5:35 PM Stephen Smalley
> > > > > <stephen.smalley.work@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Jul 14, 2020 at 5:20 PM Mike Palmiotto
> > > > > > <mike.palmiotto@crunchydata.com> wrote:
> > > > <snip>
> > > > > > > Do you think we should go ahead and completely swap in sestatus? I was
> > > > > > > just worried about breaking userspace object managers that are
> > > > > > > currently using netlink threads by default, for instance
> > > > > > > systemd-dbusd.  I can spend some more time getting those to work with
> > > > > > > the status page if you think that's worthwhile.
> > > > > >
> > > > > > I'd be interested in understanding the impact of such a change on
> > > > > > existing userspace object managers.  If we can switch the default
> > > > > > behavior for applications that are not explicitly using
> > > > > > avc_netlink_*() interfaces themselves (e.g. they are only using
> > > > > > selinux_check_access or avc_has_perm), then that would be beneficial
> > > > > > since I think it fully removes the need for a system call on the AVC
> > > > > > cache-hit code path.
> > > > >
> > > > > I'll have to do a bit of digging to see how this will affect dbus, et
> > > > > al. On first blush, it looks like they're just doing
> > > > > avc_netlink_check_nb() in their watch thread. Presumably other object
> > > > > managers are doing something similar so we would just need to make
> > > > > sure there is a netlink fd available.
> > > >
> > > > So it looks like dbus (at least) is directly checking for a netlink
> > > > socket[1], so just doing away with the avc_netlink_open call wouldn't
> > > > work out. My thinking is we have two options:
> > > >
> > > > 1) Add a new seopt to use sestatus and let userspace object managers opt-in
> > > >
> > > > 2) Call both selinux_status_open and avc_netlink_open in
> > > > avc_init_internal. This would satisfy the hard requirement for a
> > > > netlink socket. Then we can default to using sestatus in all of the
> > > > netlink processing paths, as you suggested in your last reply. We
> > > > could
> > > >
> > > > Option 2 seems better from the standpoint of using sestatus by
> > > > default, but it looks like recvfrom will never be called and the
> > > > messages will just sit in kernel memory.
> > > >
> > > > I'm inclined to go with option 1 at this point.
> > > >
> > > > [1] https://gitlab.freedesktop.org/dbus/dbus/-/blob/master/bus/selinux.c#L269
> > >
> > > Couldn't we change avc_netlink_acquire_fd() to test whether fd hasn't
> > > yet been set (i.e. == -1) and call avc_netlink_open() in that case?
> > > Then dbus would still gets its netlink fd as expected but we wouldn't
> > > need to open it inside of avc_init?
> >
> > Much better. I'll send a new patch up shortly.
>
> Okay, maybe not that shortly. I tried your suggestion and dbus doesn't
> appear to receive the netlink messages.
>
> Initially I figured it had something to do with
> avc_create_thread(avc_netlink_loop) call still being in
> avc_init_internal, so I exposed the thread pointer in avc_internal.h
> and moved the call into avc_netlink_open, which seemed more
> appropriate. Still no dice.
>
> I'm probably doing something wrong -- I'll figure it out, but it's
> going to take me longer than I thought to track this down.

Recommend writing a little test program to exercise it first without
bringing in all of the dbus baggage.  Don't think Fedora is even using
dbusd anymore, just dbus-broker.
If you get stuck, feel free to send your patch and I can take a look too.