Re: [RFC PATCH] selinux: runtime disable is deprecated, add some ssleep() discomfort

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

On Thu, Sep 10, 2020 at 9:31 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Thu, Sep 10, 2020 at 7:39 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > On Wed, Aug 19, 2020 at 9:07 PM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > > On Wed, Aug 19, 2020 at 1:15 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> > <snip>
> > > > So I've started to compose Fedora Change proposal
> > > >
> > > > https://fedoraproject.org/wiki/SELinux/Changes/Disable_CONFIG_SECURITY_SELINUX_DISABLE
> > > >
> > > > It's not complete yet, but I believe it contains basic information. I'd
> > > > appreciate if you can help me with text, phrases and references so that it would
> > > > be easy to sell it as security feature to Fedora community :)
> > >
> > > I'd simplify the Summary to be something like "Remove support for
> > > SELinux runtime disable so that the LSM hooks can be hardened via
> > > read-only-after-initialization protections.  Migrate users to using
> > > selinux=0 if they want to disable SELinux."
> >
> > FYI, the change proposal has now been announced to the Fedora devel community:
> > https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/YQIYMWKFQEWCILU7UZWXO3YFNS2PLDG4/
>
> With regard to concerns raised in that thread:
>
> 1) For (soft?) real-time users, one option (if the latency introduced
> by SELinux without a policy loaded is truly enough to affect their
> workloads) would be to insert a if
> (!selinux_initialized(&selinux_state)) return 0; at the beginning of
> most hooks. However, we can't do that everywhere (e.g. we still need
> to allocate/initialize security structures and maintain lists of
> superblocks and inodes allocated before policy load so that we can
> later fix up their labels during selinux_complete_init), and adding
> such checks would make selinux_state.initialized an even more
> attractive target for kernel exploits since it becomes another way to
> "disable" SELinux entirely.  You can of course already target it to
> disable policy checking but doing so tends to break certain things
> like security_sid_to_context/context_to_sid on SIDs other than the
> initial ones so it is not quite as attractive as enforcing currently.
> This assumes that these real-time workloads are not so sensitive that
> even the overhead of the indirect function call for the LSM hook
> pushes them over their tolerance.
>
> 2) For cases where an error is returned by SELinux that is not already
> governed by a selinux_initialized() or enforcing_enabled() check, we
> just need to ensure that all such cases are gated by such a check. We
> fixed that recently for the removexattr security.selinux case and
> there were some earlier cases fixed with respect to setting labels
> before policy load.  The specific concern raised in the thread
> appeared to be due to denials silenced via dontaudit rules, which
> won't happen if there is no policy loaded so I don't think that's
> relevant.  There are other cases where SELinux might return an error
> if a new case is introduced in another kernel subsystem without
> updating SELinux to handle it, e.g. a new type for
> selinux_perf_event_open(), a new obj_type in selinux_path_notify().
> It would be better if we could introduce build-time guards to catch
> these as we have done for e.g. new capabilities, new socket address
> families, new netlink message types, in order to ensure that they are
> always in sync.

On second look, selinux_perf_event_open() is ok because the type
values are specifically (and only) for the security hook, so anyone
adding new PERF_SECURITY_* types should see the need to update the
hook implementation too.  selinux_path_notify() case is different.