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

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

[Paul Moore]

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(+)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 4781314c2510..07af1334d9c9 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -30,6 +30,7 @@
 #include <linux/uaccess.h>
 #include <linux/kobject.h>
 #include <linux/ctype.h>
+#include <linux/delay.h>
 
 /* selinuxfs pseudo filesystem for exporting the security policy API.
    Based on the proc code and the fs/nfsd/nfsctl.c code. */
@@ -287,6 +288,7 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
 	 *       kernel releases until eventually it is removed
 	 */
 	pr_err("SELinux:  Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n");
+	ssleep(5);
 
 	if (count >= PAGE_SIZE)
 		return -ENOMEM;

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

[Paul Moore]

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.

> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 4781314c2510..07af1334d9c9 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -30,6 +30,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/kobject.h>
>  #include <linux/ctype.h>
> +#include <linux/delay.h>
>
>  /* selinuxfs pseudo filesystem for exporting the security policy API.
>     Based on the proc code and the fs/nfsd/nfsctl.c code. */
> @@ -287,6 +288,7 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
>          *       kernel releases until eventually it is removed
>          */
>         pr_err("SELinux:  Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n");
> +       ssleep(5);
>
>         if (count >= PAGE_SIZE)
>                 return -ENOMEM;
>

-- 
paul moore
www.paul-moore.com

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

[Stephen Smalley]

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'm not sure those end users will even
realize why they suddenly have a delay in their boot times since they
may not think to look at the kernel boot logs or miss the message
among the numerous messages logged already.  I guess the question is
whether there is precedent for adding this kind of delay to call
attention to a need for change by users; if there is, it would be good
if we could cite that precedent in the commit message to help avoid
later complaints.  It would also be good if we had a more thorough
wiki page or something that documented for users how to fix the
problem and referred users to that as part of the message (similar to
./arch/x86/kernel/cpu/bugs.c: pr_info("Reading
https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html
might help you decide.\n");).  Just saying add selinux=0 in the
message might not be sufficient since users may not know how to modify
their kernel command line options, especially persistently.  Allegedly
the difficulty in updating kernel boot parameters on some platforms
was the original motivation for adding runtime disable instead of just
using selinux=0.

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

[Paul Moore]

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?

-- 
paul moore
www.paul-moore.com

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

[Stephen Smalley]

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.

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

[William Roberts]

On Mon, Jun 8, 2020 at 5:16 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.

+1. I've been following this and was going to reply with what Stephen
is saying, just less eloquently.

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

[Stephen Smalley]

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.

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

[Stephen Smalley]

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.

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

[Ondrej Mosnacek]

(Sorry for the late reply.)

On Wed, Jun 10, 2020 at 4:11 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> 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.

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.

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

[Paul Moore]

On Thu, Jun 11, 2020 at 9:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> 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 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.

...

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

I'm glad to hear Fedora is making changes to move away from the
runtime disable, please keep us updated about once a month so we know
where things are at with Fedora.

As I mentioned previously, I'm okay with postponing the delay so long
as Fedora is making progress - and according to Ondrej they are - so
I'm okay with holding off for now.

-- 
paul moore
www.paul-moore.com

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

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 3722 bytes --]

On Fri, Jun 12, 2020 at 03:28:26PM -0400, Paul Moore wrote:
> On Thu, Jun 11, 2020 at 9:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > 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 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.
> 
> ...
> 
> > 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.
> 
> I'm glad to hear Fedora is making changes to move away from the
> runtime disable, please keep us updated about once a month so we know
> where things are at with Fedora.
> 
> As I mentioned previously, I'm okay with postponing the delay so long
> as Fedora is making progress - and according to Ondrej they are - so
> I'm okay with holding off for now.
>

I've used kernel built without CONFIG_SECURITY_SELINUX_DISABLE from Ondrej's COPR
https://copr.fedorainfracloud.org/coprs/omos/drop-selinux-disable/ and tried few
scenarios:

1. selinux=0 on kernel command line

everything works as expected

2. SELINUX=disabled in /etc/selinux/config

system boots, userspace considers SELinux disabled, /sys/fs/selinux is not
mounted. The only noticeable change
is in process list:

$ ps Z
LABEL                               PID TTY      STAT   TIME COMMAND
kernel                              552 pts/0    Ss     0:00 -bash
kernel                              574 pts/0    R+     0:00 ps Z

If I get it right, SELinux is enabled but it's not initialized and SELinux
checks are not processed - always return 0 as allowed. So there should be no
real externally visible difference between selinux=0 and SELINUX=disabled

3. no /etc/selinux/config

SELinux is disabled in userspace but /sys/fs/selinux in mounted. It's due to
check in libselinux which doesn't umount /sys/fs/selinux when there's no config
file. Maybe this could be improved.


So I my findings are correct, it should be quite straight and easy change for
the distribution. Even though userspace tools like anaconda and ansible still
uses /etc/selinux/config to disable SELinux, it will have similar effect as
selinux=0. But it doesn't mean we will not try to change them to set selinux=0.


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 :)


Petr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Stephen Smalley]

On Wed, Aug 19, 2020 at 1:15 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> I've used kernel built without CONFIG_SECURITY_SELINUX_DISABLE from Ondrej's COPR
> https://copr.fedorainfracloud.org/coprs/omos/drop-selinux-disable/ and tried few
> scenarios:
>
> 1. selinux=0 on kernel command line
>
> everything works as expected
>
> 2. SELINUX=disabled in /etc/selinux/config
>
> system boots, userspace considers SELinux disabled, /sys/fs/selinux is not
> mounted. The only noticeable change
> is in process list:
>
> $ ps Z
> LABEL                               PID TTY      STAT   TIME COMMAND
> kernel                              552 pts/0    Ss     0:00 -bash
> kernel                              574 pts/0    R+     0:00 ps Z

Hmm...is ps checking is_selinux_enabled()?  Or just always reading
/proc/pid/attr/current (or calling getpidcon(3))?  Under what
conditions was it displaying "-" here before?

> If I get it right, SELinux is enabled but it's not initialized and SELinux
> checks are not processed - always return 0 as allowed. So there should be no
> real externally visible difference between selinux=0 and SELINUX=disabled

There are some corner cases currently, e.g. you can't remove the
security.selinux xattr if SELinux is enabled currently, and there are
various hardcoded error cases in the SELinux hook functions that could
potentially occur.  Beyond that there is the memory and runtime
overhead.  Getting people to start using selinux=0 if they want to
disable SELinux is definitely preferable.

> 3. no /etc/selinux/config
>
> SELinux is disabled in userspace but /sys/fs/selinux in mounted. It's due to
> check in libselinux which doesn't umount /sys/fs/selinux when there's no config
> file. Maybe this could be improved.

Yes, we should fix that.

> So I my findings are correct, it should be quite straight and easy change for
> the distribution. Even though userspace tools like anaconda and ansible still
> uses /etc/selinux/config to disable SELinux, it will have similar effect as
> selinux=0. But it doesn't mean we will not try to change them to set selinux=0.
>
>
> 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."

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

[Stephen Smalley]

On Wed, Aug 19, 2020 at 3: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:
> > I've used kernel built without CONFIG_SECURITY_SELINUX_DISABLE from Ondrej's COPR
> > https://copr.fedorainfracloud.org/coprs/omos/drop-selinux-disable/ and tried few
> > scenarios:
> >
> > 1. selinux=0 on kernel command line
> >
> > everything works as expected
> >
> > 2. SELINUX=disabled in /etc/selinux/config
> >
> > system boots, userspace considers SELinux disabled, /sys/fs/selinux is not
> > mounted. The only noticeable change
> > is in process list:
> >
> > $ ps Z
> > LABEL                               PID TTY      STAT   TIME COMMAND
> > kernel                              552 pts/0    Ss     0:00 -bash
> > kernel                              574 pts/0    R+     0:00 ps Z
>
> Hmm...is ps checking is_selinux_enabled()?  Or just always reading
> /proc/pid/attr/current (or calling getpidcon(3))?  Under what
> conditions was it displaying "-" here before?
>
> > If I get it right, SELinux is enabled but it's not initialized and SELinux
> > checks are not processed - always return 0 as allowed. So there should be no
> > real externally visible difference between selinux=0 and SELINUX=disabled
>
> There are some corner cases currently, e.g. you can't remove the
> security.selinux xattr if SELinux is enabled currently, and there are
> various hardcoded error cases in the SELinux hook functions that could
> potentially occur.  Beyond that there is the memory and runtime
> overhead.  Getting people to start using selinux=0 if they want to
> disable SELinux is definitely preferable.

We could try to eliminate those error cases by checking early for
selinux_initialized(state) in more of the hooks and bailing
immediately with success in that case, but we'd have to go through and
identify where we need that.

>
> > 3. no /etc/selinux/config
> >
> > SELinux is disabled in userspace but /sys/fs/selinux in mounted. It's due to
> > check in libselinux which doesn't umount /sys/fs/selinux when there's no config
> > file. Maybe this could be improved.
>
> Yes, we should fix that.
>
> > So I my findings are correct, it should be quite straight and easy change for
> > the distribution. Even though userspace tools like anaconda and ansible still
> > uses /etc/selinux/config to disable SELinux, it will have similar effect as
> > selinux=0. But it doesn't mean we will not try to change them to set selinux=0.
> >
> >
> > 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."

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

[Casey Schaufler]

On 8/19/2020 12:16 PM, Stephen Smalley wrote:
> On Wed, Aug 19, 2020 at 3: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:
>>> I've used kernel built without CONFIG_SECURITY_SELINUX_DISABLE from Ondrej's COPR
>>> https://copr.fedorainfracloud.org/coprs/omos/drop-selinux-disable/ and tried few
>>> scenarios:
>>>
>>> 1. selinux=0 on kernel command line
>>>
>>> everything works as expected
>>>
>>> 2. SELINUX=disabled in /etc/selinux/config
>>>
>>> system boots, userspace considers SELinux disabled, /sys/fs/selinux is not
>>> mounted. The only noticeable change
>>> is in process list:
>>>
>>> $ ps Z
>>> LABEL                               PID TTY      STAT   TIME COMMAND
>>> kernel                              552 pts/0    Ss     0:00 -bash
>>> kernel                              574 pts/0    R+     0:00 ps Z
>> Hmm...is ps checking is_selinux_enabled()?  Or just always reading
>> /proc/pid/attr/current (or calling getpidcon(3))?  Under what
>> conditions was it displaying "-" here before?

The ps utility reads /proc/pid/attr/current directly. As a result,
it works for Smack as well as SELinux. Adding an SELinux state check
would have to be done carefully so as not to break the Smack functionality.
The id utility does an SELinux check that unnecessarily prevents the -Z
option from working with Smack.

>>
>>> If I get it right, SELinux is enabled but it's not initialized and SELinux
>>> checks are not processed - always return 0 as allowed. So there should be no
>>> real externally visible difference between selinux=0 and SELINUX=disabled
>> There are some corner cases currently, e.g. you can't remove the
>> security.selinux xattr if SELinux is enabled currently, and there are
>> various hardcoded error cases in the SELinux hook functions that could
>> potentially occur.  Beyond that there is the memory and runtime
>> overhead.  Getting people to start using selinux=0 if they want to
>> disable SELinux is definitely preferable.
> We could try to eliminate those error cases by checking early for
> selinux_initialized(state) in more of the hooks and bailing
> immediately with success in that case, but we'd have to go through and
> identify where we need that.
>
>>> 3. no /etc/selinux/config
>>>
>>> SELinux is disabled in userspace but /sys/fs/selinux in mounted. It's due to
>>> check in libselinux which doesn't umount /sys/fs/selinux when there's no config
>>> file. Maybe this could be improved.
>> Yes, we should fix that.
>>
>>> So I my findings are correct, it should be quite straight and easy change for
>>> the distribution. Even though userspace tools like anaconda and ansible still
>>> uses /etc/selinux/config to disable SELinux, it will have similar effect as
>>> selinux=0. But it doesn't mean we will not try to change them to set selinux=0.
>>>
>>>
>>> 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."

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

[Stephen Smalley]

On 8/19/20 3:16 PM, Stephen Smalley wrote:

> On Wed, Aug 19, 2020 at 3: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:
>> There are some corner cases currently, e.g. you can't remove the
>> security.selinux xattr if SELinux is enabled currently, and there are
>> various hardcoded error cases in the SELinux hook functions that could
>> potentially occur.  Beyond that there is the memory and runtime
>> overhead.  Getting people to start using selinux=0 if they want to
>> disable SELinux is definitely preferable.
> We could try to eliminate those error cases by checking early for
> selinux_initialized(state) in more of the hooks and bailing
> immediately with success in that case, but we'd have to go through and
> identify where we need that.

I did a quick look through error cases in the hook functions and it 
appeared that the only case where we would return an error that isn't 
already protected by a selinux_initialized() test or a test of enforcing 
mode is the removexattr() check.  So I just posted a patch to lift that 
restriction if policy hasn't been loaded. Hopefully there aren't any 
other user-visible differences.

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

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

On Thu, Aug 20, 2020 at 12:58:31PM -0400, Stephen Smalley wrote:
> On 8/19/20 3:16 PM, Stephen Smalley wrote:
> 
> > On Wed, Aug 19, 2020 at 3: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:
> > > There are some corner cases currently, e.g. you can't remove the
> > > security.selinux xattr if SELinux is enabled currently, and there are
> > > various hardcoded error cases in the SELinux hook functions that could
> > > potentially occur.  Beyond that there is the memory and runtime
> > > overhead.  Getting people to start using selinux=0 if they want to
> > > disable SELinux is definitely preferable.
> > We could try to eliminate those error cases by checking early for
> > selinux_initialized(state) in more of the hooks and bailing
> > immediately with success in that case, but we'd have to go through and
> > identify where we need that.
> 
> I did a quick look through error cases in the hook functions and it appeared
> that the only case where we would return an error that isn't already
> protected by a selinux_initialized() test or a test of enforcing mode is the
> removexattr() check.  So I just posted a patch to lift that restriction if
> policy hasn't been loaded. Hopefully there aren't any other user-visible
> differences.
> 

Thank you.

I'll be next 3 days offline but I'll document it and test it on Monday.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Ondrej Mosnacek]

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/

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

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

[Stephen Smalley]

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/

Speaking of this, I noticed that Documentation/ABI/README says that
files under obsolete should say when to expect the interface to be
removed, and at least a couple of them do, e.g.
sysfs-class-net-batman-adv:This ABI is deprecated and will be removed
after 2021.

Should we add similar lines to the two sysfs-selinux-* files, and if
so, what target date should we propose for each?

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

[Paul Moore]

On Thu, Sep 10, 2020 at 8:34 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/
>
> Speaking of this, I noticed that Documentation/ABI/README says that
> files under obsolete should say when to expect the interface to be
> removed, and at least a couple of them do, e.g.
> sysfs-class-net-batman-adv:This ABI is deprecated and will be removed
> after 2021.
>
> Should we add similar lines to the two sysfs-selinux-* files, and if
> so, what target date should we propose for each?

Sorry, I overlooked the updates to this thread in my inbox until I saw
the LWN article today and revisited this thread.

The lack of a specific date in the disable sysctl was a deliberate
omission on my part as when the commit was made it wasn't clear when
Fedora would be ready to make the transition.  As we documented in the
the sysfs-selinux-disable obsolescence notice:

  "Fedora is in the process of removing the selinuxfs "disable"
   node and once that is complete we will start the slow process
   of removing this code from the kernel."

As far as the checkreqprot notice is concerned, it probably would be a
good idea to outline a process for its eventual removal.  It isn't
quite the same as the runtime disable issue since the distro work
should all be done at this point, it's just a matter of finally
blocking any "1" writes.  The deprecation made its first appearance in
v5.7, which was released in June 2020, and a year seems like a
reasonable amount of time for this so perhaps we target summer 2021?

-- 
paul moore
www.paul-moore.com

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

[Stephen Smalley]

On Wed, Sep 23, 2020 at 2:33 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Sep 10, 2020 at 8:34 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > > Speaking of this, I noticed that Documentation/ABI/README says that
> > files under obsolete should say when to expect the interface to be
> > removed, and at least a couple of them do, e.g.
> > sysfs-class-net-batman-adv:This ABI is deprecated and will be removed
> > after 2021.
> >
> > Should we add similar lines to the two sysfs-selinux-* files, and if
> > so, what target date should we propose for each?
>
> Sorry, I overlooked the updates to this thread in my inbox until I saw
> the LWN article today and revisited this thread.
>
> The lack of a specific date in the disable sysctl was a deliberate
> omission on my part as when the commit was made it wasn't clear when
> Fedora would be ready to make the transition.  As we documented in the
> the sysfs-selinux-disable obsolescence notice:
>
>   "Fedora is in the process of removing the selinuxfs "disable"
>    node and once that is complete we will start the slow process
>    of removing this code from the kernel."
>
> As far as the checkreqprot notice is concerned, it probably would be a
> good idea to outline a process for its eventual removal.  It isn't
> quite the same as the runtime disable issue since the distro work
> should all be done at this point, it's just a matter of finally
> blocking any "1" writes.  The deprecation made its first appearance in
> v5.7, which was released in June 2020, and a year seems like a
> reasonable amount of time for this so perhaps we target summer 2021?

Sounds good to me.

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

[Stephen Smalley]

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.

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

[Stephen Smalley]

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.

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

[Stephen Smalley]

On Thu, Sep 10, 2020 at 10:36 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Thu, Sep 10, 2020 at 9:31 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > 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.

For selinux_path_notify(), there are in fact other fsnotify object
types defined in fsnotify_backend.h but
fs/notify/fanotify_user.c:do_fanotify_mark() only ever sets obj_type
that is later passed to the hook to one of the three values it handles
(inode, vfsmount, sb).  Since that could potentially change in the
future, we should likely change the security framework to define its
own set of SECURITY_NOTIFY_OBJ_TYPE_* values and have the hook callers
explicitly translate to one of those.

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

[Paul Moore]

On Wed, Jun 10, 2020 at 10:11 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> 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.

FWIW, my opinion regarding pay-for-support distros is that while I
would prefer not to break them, if the right thing for upstream and
community distros is to do thing "X", we should do thing "X".

IBM/RH has a bunch of people who get paid to make sure RHEL keeps
working, I trust they can manage RHEL just fine ;)

-- 
paul moore
www.paul-moore.com

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

[Paul Moore]

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

Regardless of what we do with this, I thought the deprecation commit
was pretty clear about adding a delay as part of the deprecation
process.  The most appropriate time to raise objections like this
would have been when the original patch was posted.

In other words, I expect people to review the commit descriptions
along with the patches.  When anyone adds an "Acked-by" or a
"Reviewed-by" tag, I take that to mean they have read not just the
patch, but the commit description as well and the person approves of
both.

-- 
paul moore
www.paul-moore.com

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

[Paul Moore]

We deprecated the SELinux runtime disable functionality in Linux
v5.6, and it is time to get a bit more serious about removing it.
Add a five second sleep to anyone using it to help draw their
attention to the deprecation and provide a URL which helps explain
things in more detail, including how to add kernel command line
parameters to some of the more popular Linux distributions.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 security/selinux/selinuxfs.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index f2f6203e0fff..3739155ee0db 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -293,6 +293,8 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
 	 *       kernel releases until eventually it is removed
 	 */
 	pr_err("SELinux:  Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n");
+	pr_err("SELinux:  https://github.com/SELinuxProject/selinux-kernel/wiki/DEPRECATE-runtime-disable\n");
+	ssleep(5);
 
 	if (count >= PAGE_SIZE)
 		return -ENOMEM;

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

[Paul Moore]

On Tue, Mar 1, 2022 at 5:53 PM Paul Moore <paul@paul-moore.com> wrote:
>
> We deprecated the SELinux runtime disable functionality in Linux
> v5.6, and it is time to get a bit more serious about removing it.
> Add a five second sleep to anyone using it to help draw their
> attention to the deprecation and provide a URL which helps explain
> things in more detail, including how to add kernel command line
> parameters to some of the more popular Linux distributions.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  security/selinux/selinuxfs.c |    2 ++
>  1 file changed, 2 insertions(+)

Comments are also welcome on the text in the URL shown as part of the
error message below.

> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index f2f6203e0fff..3739155ee0db 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -293,6 +293,8 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
>          *       kernel releases until eventually it is removed
>          */
>         pr_err("SELinux:  Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n");
> +       pr_err("SELinux:  https://github.com/SELinuxProject/selinux-kernel/wiki/DEPRECATE-runtime-disable\n");
> +       ssleep(5);
>
>         if (count >= PAGE_SIZE)
>                 return -ENOMEM;
>

-- 
paul-moore.com

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

[Casey Schaufler]

On 3/1/2022 2:53 PM, Paul Moore wrote:
> We deprecated the SELinux runtime disable functionality in Linux
> v5.6, and it is time to get a bit more serious about removing it.
> Add a five second sleep to anyone using it to help draw their
> attention to the deprecation and provide a URL which helps explain
> things in more detail, including how to add kernel command line
> parameters to some of the more popular Linux distributions.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Thank you for pressing on this. FWIW -
Acked-by: Casey Schaufler <casey@schaufler-ca.com>

> ---
>   security/selinux/selinuxfs.c |    2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index f2f6203e0fff..3739155ee0db 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -293,6 +293,8 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
>   	 *       kernel releases until eventually it is removed
>   	 */
>   	pr_err("SELinux:  Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n");
> +	pr_err("SELinux:  https://github.com/SELinuxProject/selinux-kernel/wiki/DEPRECATE-runtime-disable\n");
> +	ssleep(5);
>   
>   	if (count >= PAGE_SIZE)
>   		return -ENOMEM;
>

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

[Paul Moore]

On Tue, Mar 1, 2022 at 5:53 PM Paul Moore <paul@paul-moore.com> wrote:
>
> We deprecated the SELinux runtime disable functionality in Linux
> v5.6, and it is time to get a bit more serious about removing it.
> Add a five second sleep to anyone using it to help draw their
> attention to the deprecation and provide a URL which helps explain
> things in more detail, including how to add kernel command line
> parameters to some of the more popular Linux distributions.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  security/selinux/selinuxfs.c |    2 ++
>  1 file changed, 2 insertions(+)

Merged into selinux/next.

-- 
paul-moore.com