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

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

On Wed, Jun 10, 2020 at 10:03 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Jun 8, 2020 at 6:13 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Mon, Jun 8, 2020 at 5:35 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On Thu, Jun 4, 2020 at 10:49 AM Stephen Smalley
> > > <stephen.smalley.work@gmail.com> wrote:
> > > > On Tue, Jun 2, 2020 at 8:52 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > > On Tue, Jun 2, 2020 at 8:47 AM Paul Moore <paul@paul-moore.com> wrote:
> > > > > >
> > > > > > We deprecated the SELinux runtime disable functionality in Linux
> > > > > > v5.6, add a five second sleep to anyone using it to help draw their
> > > > > > attention to the deprecation.
> > > > > >
> > > > > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > > > > > ---
> > > > > >  security/selinux/selinuxfs.c |    2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > Warning: while trivial, I've done no testing beyond a quick compile
> > > > > yet.  I'm posting this now to see what everyone thinks about starting
> > > > > to make it a bit more painful to use the runtime disable
> > > > > functionality.
> > > >
> > > > I'm concerned about how users will experience and respond to this
> > > > change (and Linus too).  Currently SELinux runtime disable is the
> > > > method used by distro installers (at least Fedora/RHEL and
> > > > derivatives) when SELinux-disabled is selected at install time and it
> > > > is the approach documented in distro documentation for how to disable
> > > > SELinux.  Hence, we'd be inflicting pain on the end users for what is
> > > > essentially a distro choice.
> > >
> > > I delayed my response in hopes the Fedora folks would also comment,
> > > but I'm not seeing anything.
> > >
> > > All this patch does is start executing on the deprecation path we laid
> > > out when we marked the functionality as deprecated.  When we decided
> > > to do this we had buy-in from the Fedora folks (the only ones who
> > > still use this option);  if this is a problem for them then I would
> > > like to understand what changed, and why.  If it is a matter of this
> > > coming too quickly, that's okay, we can push this out another release
> > > or two.  We can even drop the sleep down to a second or two.  Both the
> > > timing of introducing the delay, and the length of the delay itself,
> > > aren't important to me; it's the fact that we are adding a delay and
> > > moving forward on the deprecation (just as we said we would).
> > >
> > > What were you envisioning when we marked this as deprecated Stephen?
> > > If not this, what were you thinking we would do?
> >
> > I feel like we've already communicated the fact that it is being
> > deprecated to those who need to know (Fedora maintainers), and we
> > already have it displaying an error message for those who look at
> > kernel logs.  So I was fine with just waiting some number of kernel
> > release cycles (not sure what is typical for these kinds of things)
> > and then just changing selinux_write_disable() to just return 0
> > without doing anything and dropping the selinux_disable() code and the
> > config option.  I think we'll want it to return 0 rather than an error
> > so that systemd will still unmount selinuxfs and act as if SELinux has
> > been disabled (which in turn will case everything else to act as if
> > SELinux has been disabled).  The kernel will be in permissive mode
> > with no policy loaded in that situation, so except for some corner
> > cases everything should just work.  That seems the least disruptive
> > path for end users.  Distro maintainers will hopefully get around to
> > using selinux=0 instead but that may lag.
>
> I just tested with building a kernel with
> CONFIG_SECURITY_SELINUX_DISABLE=n and setting SELINUX=disabled in
> /etc/selinux/config, and the system came up with selinuxfs unmounted,
> sestatus and friends think SELinux is disabled, but it is enabled just
> permissive with no policy.  I double checked the logic in systemd and
> libselinux (selinux_init_load_policy()) and it does handle an error
> return from writing to /sys/fs/selinux/disable gracefully.  So I guess
> we can have it return an error without breaking userspace.

Ondrej might want to check that it doesn't break RHEL either but I
wouldn't really expect this to get back-ported to RHEL anyway unless
they want the additional hardening gain from being able to make the
LSM hooks read-only after initialization.