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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(Sorry for the late reply.)

On Wed, Jun 10, 2020 at 4:11 PM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
> On Wed, Jun 10, 2020 at 10:03 AM Stephen Smalley
> <stephen.smalley.work@xxxxxxxxx> wrote:
> >
> > On Mon, Jun 8, 2020 at 6:13 PM Stephen Smalley
> > <stephen.smalley.work@xxxxxxxxx> wrote:
> > >
> > > On Mon, Jun 8, 2020 at 5:35 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > >
> > > > On Thu, Jun 4, 2020 at 10:49 AM Stephen Smalley
> > > > <stephen.smalley.work@xxxxxxxxx> wrote:
> > > > > On Tue, Jun 2, 2020 at 8:52 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > > > > On Tue, Jun 2, 2020 at 8:47 AM Paul Moore <paul@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > > > > > > ---
> > > > > > >  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.

Good point about the installer. I have already started working on
preparing Fedora for the runtime disable removal, but so far I'm only
at the beginning. Updating anaconda to add selinux=0 to the kernel
params instead of using /etc/selinux/config will be one of the main
steps.

> > > > 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 also prefer to rather go somewhere in this direction rather than
introducing the delay. I was kinda OK with the delay at first, but as
Stephen points out, it would punish users rather than distros, even
though users are (normally) not the ones that make a conscious
decision to use the runtime disable.

> > 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.

Yes, I was under the impression that some changes in libselinux are
needed before this works transparently, but apparently it already does
the right thing now. In that case I'd say that it may be better to
skip adding sleeps etc. and just remove the feature at some point. But
please let's wait with that for a while longer so we can prepare
Fedora for it first. It's hard to tell at this point how long that
will take, but it could be several months.

Then again, the sleep might be helpful to wake up potential non-Fedora
users (if any) and in Fedora we can always apply a revert as a
downstream patch until things are sorted. So if you guys really want
it, I think we can deal with it.

> 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.

We definitely don't plan on backporting it to existing major RHEL
releases. However, I'd like to prepare Fedora for a life without the
runtime disable (and to disable the kernel config option as the final
step) before the next major release is branched off of Fedora, so any
future major releases will hopefully already ship with SELINUX_DISABLE
turned off/removed.

--
Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux