| Message ID | 159110207843.57260.5661475689740939480.stgit@chester (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [RFC] selinux: runtime disable is deprecated, add some ssleep() discomfort | expand |
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; >
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.
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?
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.
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.
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.
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.
(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.
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.
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 ;)
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.
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
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."
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."
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."
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.
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.
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.
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?
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 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.
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.
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?
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.
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;
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(+)