[PATCH] LSM: allow an LSM to disable all hooks at once

[Paul Moore]

On Thu, Dec 12, 2019 at 1:10 PM Stephen Smalley <sds at tycho.nsa.gov> wrote:
>
> On 12/12/19 1:09 PM, Paul Moore wrote:
> > On Thu, Dec 12, 2019 at 12:57 PM Stephen Smalley <sds at tycho.nsa.gov> wrote:
> >> On 12/12/19 12:54 PM, Paul Moore wrote:
> >>> On Thu, Dec 12, 2019 at 8:14 AM Stephen Smalley <sds at tycho.nsa.gov> wrote:
> >>>> On 12/12/19 6:49 AM, Ondrej Mosnacek wrote:
> >>>>> On Wed, Dec 11, 2019 at 8:12 PM Stephen Smalley <sds at tycho.nsa.gov> wrote:
> >>>>>> On 12/11/19 1:35 PM, Casey Schaufler wrote:
> >>>>>>> On 12/11/2019 8:42 AM, Kees Cook wrote:
> >>>>>>>> On Wed, Dec 11, 2019 at 09:29:10AM -0500, Stephen Smalley wrote:
> >>>>>>>>> On 12/11/19 9:08 AM, Ondrej Mosnacek wrote:
> >>>
> >>> ...
> >>>
> >>>>>> selinux_state.initialized reflects whether a policy has
> >>>>>> been loaded.  With a few exceptions in certain hook functions, it is
> >>>>>> only checked by the security server service functions
> >>>>>> (security/selinux/ss/services.c) prior to accessing the policydb.  So
> >>>>>> there is a lot of SELinux processing that would still occur in that
> >>>>>> situation unless we added if (!selinux_state.initialized) return 0;
> >>>>>> checks to all the hook functions, which would create the same exposure
> >>>>>> and would further break the SELinux-enabled case (we need to perform
> >>>>>> some SELinux processing pre-policy-load to allocate blobs and track what
> >>>>>> tasks and objects require delayed security initialization when policy
> >>>>>> load finally occurs).
> >>>>>
> >>>>> I think what Casey was suggesting is to add another flag that would
> >>>>> switch from "no policy loaded, but we expect it to be loaded
> >>>>> eventually" to "no policy loaded and we don't expect/allow it to be
> >>>>> loaded any more", which is essentially equivalent to checking
> >>>>> selinux_enabled in each hook, which you had already brought up.
> >>>>
> >>>> Yep.  if (!selinux_enabled) return 0; or if (selinux_state.disabled)
> >>>> return 0; under #ifdef CONFIG_SECURITY_SELINUX_DISABLE in every hook
> >>>> might be the best option until it can be removed altogether; avoids
> >>>> impacting the LSM framework or any other security module, preserves the
> >>>> existing functionality, fairly low overhead on the SELinux-disabled case.
> >>>
> >>> Just so I'm understanding this thread correctly, the above change
> >>> (adding enabled checks to each SELinux hook implementation) is only
> >>> until Fedora can figure out a way to deprecate and remove the runtime
> >>> disable?
> >>
> >> That's my understanding.  In the interim, Android kernels should already
> >> be disabling CONFIG_SECURITY_SELINUX_DISABLE and other distros may
> >> choose to disable it as long as they don't care about supporting SELinux
> >> runtime disable.
> >
> > Okay, I just wanted to make sure I wasn't missing something.
> > Honestly, I'd rather Fedora just go ahead and do whatever it is they
> > need to do to turn off CONFIG_SECURITY_SELINUX_DISABLE (it sounds like
> > they have a plan and are working on it), I'm not overly excited about
> > temporarily cluttering up the code with additional "enabled" checks
> > when the status quo works, even if it is less than ideal.
>
> The status quo is producing kernel crashes, courtesy of LSM stacking
> changes...

How prevalent are these crashes?

This also only happens when disabling SELinux at runtime, yes?
Something we've advised against for some time now and are working to
eliminate?  Let's just get rid of the runtime disable *soon*, and if
we need a stop-gap fix let's just go with the hook reordering since
that seems to minimize the impact, if not resolve it.

I'm not going to comment on the stacking changes.

-- 
paul moore
www.paul-moore.com