[RFC] userspace: netlink/sestatus feature parity

[RFC] userspace: netlink/sestatus feature parity

[Mike Palmiotto]

In looking at the userspace AVC netlink and sestatis code, I noticed
there are a few discrepancies between the two mechanisms. Considering
sestatus is intended (AFAICT) to be a swap-in replacement for netlink,
I'd expect all of the same code paths to be covered. This doesn't seem
to be the case.

One such difference is the handling of `setenforce` events in
`selinux_status_updated/setenforce()`. While netlink updates the
internal `avc_enforcing` state, `selinux_status_updated/setenforce()`
do not.

Any userspace object manager wishing to use sestatus with the internal
AVC is not guaranteed to have accurate state during calls to
`avc_has_perm_noaudit`, unless they reach out to netlink. sestatus was
initially implemented for use in sepgsql, which did not require use of
`avc_has_perm_noaudit`.

To more robustly support use of sestatus, I'm proposing that we
improve upon the sestatus code by having it update/reset AVC internal
state (avc_enforcing, for example) on status events.

Would such a patch be of interest? Or would it be simpler to just
update `avc_has_perm_noaudit` to query sestatus for enforcing, rather
than refer to `avc_enforcing`?

A few questions further questions in case this improvement is of interest:

1) Should there be separate callbacks for netlink counterparts in
sestatus, or is the existing infrastructure suitable for implementing
handling of those events?

2) With netlink we're guaranteed sequential processing of events. The
same is not true for mmap()'ed status updates. Do we care about the
order in which events are processed?

Thanks in advance,
-- 
Mike Palmiotto
https://crunchydata.com

Re: [RFC] userspace: netlink/sestatus feature parity

[Stephen Smalley]

On Mon, Jun 29, 2020 at 3:59 PM Mike Palmiotto
<mike.palmiotto@crunchydata.com> wrote:
>
> In looking at the userspace AVC netlink and sestatis code, I noticed
> there are a few discrepancies between the two mechanisms. Considering
> sestatus is intended (AFAICT) to be a swap-in replacement for netlink,
> I'd expect all of the same code paths to be covered. This doesn't seem
> to be the case.
>
> One such difference is the handling of `setenforce` events in
> `selinux_status_updated/setenforce()`. While netlink updates the
> internal `avc_enforcing` state, `selinux_status_updated/setenforce()`
> do not.
>
> Any userspace object manager wishing to use sestatus with the internal
> AVC is not guaranteed to have accurate state during calls to
> `avc_has_perm_noaudit`, unless they reach out to netlink. sestatus was
> initially implemented for use in sepgsql, which did not require use of
> `avc_has_perm_noaudit`.
>
> To more robustly support use of sestatus, I'm proposing that we
> improve upon the sestatus code by having it update/reset AVC internal
> state (avc_enforcing, for example) on status events.
>
> Would such a patch be of interest?

Yes, this makes sense to me.

> Or would it be simpler to just
> update `avc_has_perm_noaudit` to query sestatus for enforcing, rather
> than refer to `avc_enforcing`?

The current AVC interface allows an object manager to explicitly set
the enforcing mode and ignore the underlying system enforcing mode,
such that one can have an object manager running permissive while the
kernel and other userspace object managers running enforcing for
development purposes.  So we'd still need to support that scenario I
believe.  That said, providing an option to use sestatus instead of
netlink and potentially making that the default going forward might be
of interest as long as it doesn't break compatibility.  The sestatus
code already falls back to netlink if the kernel doesn't support the
status page.

>
> A few questions further questions in case this improvement is of interest:
>
> 1) Should there be separate callbacks for netlink counterparts in
> sestatus, or is the existing infrastructure suitable for implementing
> handling of those events?

I haven't looked closely but would guess that we could use the same callbacks.

> 2) With netlink we're guaranteed sequential processing of events. The
> same is not true for mmap()'ed status updates. Do we care about the
> order in which events are processed?

As long as the final state is correct, I don't think so, but I'm not
sure what scenario you are considering.

Re: [RFC] userspace: netlink/sestatus feature parity

[Mike Palmiotto]

On Mon, Jul 6, 2020 at 9:23 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Jun 29, 2020 at 3:59 PM Mike Palmiotto
> <mike.palmiotto@crunchydata.com> wrote:
<snip>
> >
> > Or would it be simpler to just
> > update `avc_has_perm_noaudit` to query sestatus for enforcing, rather
> > than refer to `avc_enforcing`?
>
> The current AVC interface allows an object manager to explicitly set
> the enforcing mode and ignore the underlying system enforcing mode,
> such that one can have an object manager running permissive while the
> kernel and other userspace object managers running enforcing for
> development purposes.  So we'd still need to support that scenario I
> believe.  That said, providing an option to use sestatus instead of
> netlink and potentially making that the default going forward might be
> of interest as long as it doesn't break compatibility.  The sestatus
> code already falls back to netlink if the kernel doesn't support the
> status page.
>
> >
> > A few questions further questions in case this improvement is of interest:
> >
> > 1) Should there be separate callbacks for netlink counterparts in
> > sestatus, or is the existing infrastructure suitable for implementing
> > handling of those events?
>
> I haven't looked closely but would guess that we could use the same callbacks.

This all sounds good to me. I'll make sure the existing functionality
remains unchanged and see if I can leverage the current callbacks.

>
> > 2) With netlink we're guaranteed sequential processing of events. The
> > same is not true for mmap()'ed status updates. Do we care about the
> > order in which events are processed?
>
> As long as the final state is correct, I don't think so, but I'm not
> sure what scenario you are considering.

I was likely just getting wrapped up in semantics. We really only care
that seqno is incremented once a state is processed.

Thanks for the feedback. I should be able to send a patch sometime this week.

--
Mike Palmiotto
https://crunchydata.com