Re: [PATCH] libselinux: Use sestatus if open

[Mike Palmiotto <mike.palmiotto@crunchydata.com>]

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.

-- 
Mike Palmiotto
https://crunchydata.com