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

[Casey Schaufler]

On 12/12/2019 10:11 AM, Stephen Smalley 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...

... that went in 4+ years ago and are just now showing problems.
Either something's changed or no one has really cared in the interim.