Re: [PATCH v2] selinux: deprecate disabling SELinux and runtime

From: Stephen Smalley <sds@tycho.nsa.gov>
To: Paul Moore <paul@paul-moore.com>, selinux@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Subject: Re: [PATCH v2] selinux: deprecate disabling SELinux and runtime
Date: Tue, 7 Jan 2020 09:35:15 -0500	[thread overview]
Message-ID: <43f27f76-f3ca-7ea2-7820-da56bb53fd0e@tycho.nsa.gov> (raw)
In-Reply-To: <157836784986.560897.13893922675143903084.stgit@chester>

On 1/6/20 10:30 PM, Paul Moore wrote:
> Deprecate the CONFIG_SECURITY_SELINUX_DISABLE functionality.  The
> code was originally developed to make it easier for Linux
> distributions to support architectures where adding parameters to the
> kernel command line was difficult.  Unfortunately, supporting runtime
> disable meant we had to make some security trade-offs when it came to
> the LSM hooks, as documented in the Kconfig help text:
> 
>    NOTE: selecting this option will disable the '__ro_after_init'
>    kernel hardening feature for security hooks.   Please consider
>    using the selinux=0 boot parameter instead of enabling this
>    option.
> 
> Fortunately it looks as if that the original motivation for the
> runtime disable functionality is gone, and Fedora/RHEL appears to be
> the only major distribution enabling this capability at build time
> so we are now taking steps to remove it entirely from the kernel.
> The first step is to mark the functionality as deprecated and print
> an error when it is used (what this patch is doing).  As Fedora/RHEL
> makes progress in transitioning the distribution away from runtime
> disable, we will introduce follow-up patches over several kernel
> releases which will block for increasing periods of time when the
> runtime disable is used.  Finally we will remove the option entirely
> once we believe all users have moved to the kernel cmdline approach.
> 
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

checkpatch.pl has two warnings on this patch, one about the new 
Documentation/ABI/obsolete/sysfs-selinux-disable file not being listed 
in MAINTAINERS (looks like others are) and one about the comment block 
style being wrong.  Also not entirely sure if the file should be 
sysfs-selinux-disable or selinuxfs-disable; it gets mounted under sysfs 
but isn't part of it per se.  Otherwise, LGTM.

> ---
>   Documentation/ABI/obsolete/sysfs-selinux-disable |   26 ++++++++++++++++++++++
>   security/selinux/Kconfig                         |    3 +++
>   security/selinux/selinuxfs.c                     |    6 +++++
>   3 files changed, 35 insertions(+)
>   create mode 100644 Documentation/ABI/obsolete/sysfs-selinux-disable
> 
> diff --git a/Documentation/ABI/obsolete/sysfs-selinux-disable b/Documentation/ABI/obsolete/sysfs-selinux-disable
> new file mode 100644
> index 000000000000..c340278e3cf8
> --- /dev/null
> +++ b/Documentation/ABI/obsolete/sysfs-selinux-disable
> @@ -0,0 +1,26 @@
> +What:		/sys/fs/selinux/disable
> +Date:		April 2005 (predates git)
> +KernelVersion:	2.6.12-rc2 (predates git)
> +Contact:	selinux@vger.kernel.org
> +Description:
> +
> +	The selinuxfs "disable" node allows SELinux to be disabled at runtime
> +	prior to a policy being loaded into the kernel.  If disabled via this
> +	mechanism, SELinux will remain disabled until the system is rebooted.
> +
> +	The preferred method of disabling SELinux is via the "selinux=0" boot
> +	parameter, but the selinuxfs "disable" node was created to make it
> +	easier for systems with primitive bootloaders that did not allow for
> +	easy modification of the kernel command line.  Unfortunately, allowing
> +	for SELinux to be disabled at runtime makes it difficult to secure the
> +	kernel's LSM hooks using the "__ro_after_init" feature.
> +
> +	Thankfully, the need for the SELinux runtime disable appears to be
> +	gone, the default Kconfig configuration disables this selinuxfs node,
> +	and only one of the major distributions, Fedora, supports disabling
> +	SELinux at runtime.  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.
> +
> +	More information on /sys/fs/selinux/disable can be found under the
> +	CONFIG_SECURITY_SELINUX_DISABLE Kconfig option.
> diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
> index 996d35d950f7..580ac24c7aa1 100644
> --- a/security/selinux/Kconfig
> +++ b/security/selinux/Kconfig
> @@ -42,6 +42,9 @@ config SECURITY_SELINUX_DISABLE
>   	  using the selinux=0 boot parameter instead of enabling this
>   	  option.
>   
> +	  WARNING: this option is deprecated and will be removed in a future
> +	  kernel release.
> +
>   	  If you are unsure how to answer this question, answer N.
>   
>   config SECURITY_SELINUX_DEVELOP
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 278417e67b4c..adbe2dd35202 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -281,6 +281,12 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
>   	int new_value;
>   	int enforcing;
>   
> +	/* NOTE: we are now officially considering runtime disable as
> +	 *       deprecated, and using it will become increasingly painful
> +	 *       (e.g. sleeping/blocking) as we progress through future
> +	 *       kernel releases until eventually it is removed */
> +	pr_err("SELinux:  Runtime disable is deprecated, use selinux=0 on the kernel cmdline.\n");
> +
>   	if (count >= PAGE_SIZE)
>   		return -ENOMEM;
>   
> 