On Thu, Mar 16, 2023 at 2:01 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> We originally promised that the SELinux 'checkreqprot' functionality
> would be removed no sooner than June 2021, and now that it is March
> 2023 it seems like it is a good time to do the final removal. The
> deprecation notice in the kernel provides plenty of detail on why
> 'checkreqprot' is not desirable, with the key point repeated below:
>
> This was a compatibility mechanism for legacy userspace and
> for the READ_IMPLIES_EXEC personality flag. However, if set to
> 1, it weakens security by allowing mappings to be made executable
> without authorization by policy. The default value of checkreqprot
> at boot was changed starting in Linux v4.4 to 0 (i.e. check the
> actual protection), and Android and Linux distributions have been
> explicitly writing a "0" to /sys/fs/selinux/checkreqprot during
> initialization for some time.
>
> Along with the official deprecation notice, we have been discussing
> this on-list and directly with several of the larger SELinux-based
> distros and everyone is happy to see this feature finally removed.
> In an attempt to catch all of the smaller, and DIY, Linux systems
> we have been writing a deprecation notice URL into the kernel log,
> along with a growing ssleep() penalty, when admins enabled
> checkreqprot at runtime or via the kernel command line. We have
> yet to have anyone come to us and raise an objection to the
> deprecation or planned removal.
>
> It is worth noting that while this patch removes the checkreqprot
> functionality, it leaves the user visible interfaces (kernel command
> line and selinuxfs file) intact, just inert. This should help
> prevent breakages with existing userspace tools that correctly, but
> unnecessarily, disable checkreqprot at boot or runtime. Admins
> that attempt to enable checkreqprot will be met with a removal
> message in the kernel log.
>
> Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
Acked-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
I was wondering if we could remove the reqprot parameter altogether from the
mmap/mprotect hooks but looks like IMA is using it.
> ---
> .../sysfs-selinux-checkreqprot | 3 +++
> security/selinux/Kconfig | 23 -------------------
> security/selinux/hooks.c | 20 ++++------------
> security/selinux/include/security.h | 9 ++++----
> security/selinux/selinuxfs.c | 13 ++++-------
> 5 files changed, 17 insertions(+), 51 deletions(-)
> rename Documentation/ABI/{obsolete => removed}/sysfs-selinux-checkreqprot (90%)
>
> diff --git a/Documentation/ABI/obsolete/sysfs-selinux-checkreqprot b/Documentation/ABI/removed/sysfs-selinux-checkreqprot
> similarity index 90%
> rename from Documentation/ABI/obsolete/sysfs-selinux-checkreqprot
> rename to Documentation/ABI/removed/sysfs-selinux-checkreqprot
> index ed6b52ca210f..f599a0a87e8b 100644
> --- a/Documentation/ABI/obsolete/sysfs-selinux-checkreqprot
> +++ b/Documentation/ABI/removed/sysfs-selinux-checkreqprot
> @@ -4,6 +4,9 @@ KernelVersion: 2.6.12-rc2 (predates git)
> Contact: selinux@xxxxxxxxxxxxxxx
> Description:
>
> + REMOVAL UPDATE: The SELinux checkreqprot functionality was removed in
> + March 2023, the original deprecation notice is shown below.
> +
> The selinuxfs "checkreqprot" node allows SELinux to be configured
> to check the protection requested by userspace for mmap/mprotect
> calls instead of the actual protection applied by the kernel.
> diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
> index 9e921fc72538..4ea946123255 100644
> --- a/security/selinux/Kconfig
> +++ b/security/selinux/Kconfig
> @@ -70,29 +70,6 @@ config SECURITY_SELINUX_AVC_STATS
> /sys/fs/selinux/avc/cache_stats, which may be monitored via
> tools such as avcstat.
>
> -config SECURITY_SELINUX_CHECKREQPROT_VALUE
> - int "NSA SELinux checkreqprot default value"
> - depends on SECURITY_SELINUX
> - range 0 1
> - default 0
> - help
> - This option sets the default value for the 'checkreqprot' flag
> - that determines whether SELinux checks the protection requested
> - by the application or the protection that will be applied by the
> - kernel (including any implied execute for read-implies-exec) for
> - mmap and mprotect calls. If this option is set to 0 (zero),
> - SELinux will default to checking the protection that will be applied
> - by the kernel. If this option is set to 1 (one), SELinux will
> - default to checking the protection requested by the application.
> - The checkreqprot flag may be changed from the default via the
> - 'checkreqprot=' boot parameter. It may also be changed at runtime
> - via /sys/fs/selinux/checkreqprot if authorized by policy.
> -
> - WARNING: this option is deprecated and will be removed in a future
> - kernel release.
> -
> - If you are unsure how to answer this question, answer 0.
> -
> config SECURITY_SELINUX_SIDTAB_HASH_BITS
> int "NSA SELinux sidtab hashtable size"
> depends on SECURITY_SELINUX
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index db6d8b68b543..9a58971f9a16 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -136,17 +136,13 @@ static int __init selinux_enabled_setup(char *str)
> __setup("selinux=", selinux_enabled_setup);
> #endif
>
> -static unsigned int selinux_checkreqprot_boot =
> - CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE;
> -
> static int __init checkreqprot_setup(char *str)
> {
> unsigned long checkreqprot;
>
> if (!kstrtoul(str, 0, &checkreqprot)) {
> - selinux_checkreqprot_boot = checkreqprot ? 1 : 0;
> if (checkreqprot)
> - pr_err("SELinux: checkreqprot set to 1 via kernel parameter. This is deprecated and will be rejected in a future kernel release.\n");
> + pr_err("SELinux: checkreqprot set to 1 via kernel parameter. This is no longer supported.\n");
> }
> return 1;
> }
> @@ -3712,7 +3708,8 @@ static int selinux_mmap_addr(unsigned long addr)
> return rc;
> }
>
> -static int selinux_mmap_file(struct file *file, unsigned long reqprot,
> +static int selinux_mmap_file(struct file *file,
> + unsigned long reqprot __always_unused,
> unsigned long prot, unsigned long flags)
> {
> struct common_audit_data ad;
> @@ -3727,23 +3724,17 @@ static int selinux_mmap_file(struct file *file, unsigned long reqprot,
> return rc;
> }
>
> - if (checkreqprot_get())
> - prot = reqprot;
> -
> return file_map_prot_check(file, prot,
> (flags & MAP_TYPE) == MAP_SHARED);
> }
>
> static int selinux_file_mprotect(struct vm_area_struct *vma,
> - unsigned long reqprot,
> + unsigned long reqprot __always_unused,
> unsigned long prot)
> {
> const struct cred *cred = current_cred();
> u32 sid = cred_sid(cred);
>
> - if (checkreqprot_get())
> - prot = reqprot;
> -
> if (default_noexec &&
> (prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
> int rc = 0;
> @@ -7202,9 +7193,6 @@ static __init int selinux_init(void)
>
> memset(&selinux_state, 0, sizeof(selinux_state));
> enforcing_set(selinux_enforcing_boot);
> - if (CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE)
> - pr_err("SELinux: CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE is non-zero. This is deprecated and will be rejected in a future kernel release.\n");
> - checkreqprot_set(selinux_checkreqprot_boot);
> selinux_avc_init();
> mutex_init(&selinux_state.status_lock);
> mutex_init(&selinux_state.policy_mutex);
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index cb38d1894cfc..7e9511841134 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -95,7 +95,6 @@ struct selinux_state {
> #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
> bool enforcing;
> #endif
> - bool checkreqprot;
> bool initialized;
> bool policycap[__POLICYDB_CAP_MAX];
>
> @@ -145,14 +144,16 @@ static inline void enforcing_set(bool value)
>
> static inline bool checkreqprot_get(void)
> {
> - return READ_ONCE(selinux_state.checkreqprot);
> + /* setting checkreqprot to a non-zero value is no longer supported */
> + return 0;
> }
>
> static inline void checkreqprot_set(bool value)
> {
> - if (value)
> + if (value) {
> pr_err("SELinux: https://github.com/SELinuxProject/selinux-kernel/wiki/DEPRECATE-checkreqprot\n";);
> - WRITE_ONCE(selinux_state.checkreqprot, value);
> + pr_err("SELinux: setting checkreqprot to '1' is no longer supported\n");
> + }
> }
>
> #ifdef CONFIG_SECURITY_SELINUX_DISABLE
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 08164d074e56..68688bc84912 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -728,23 +728,20 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf,
> if (IS_ERR(page))
> return PTR_ERR(page);
>
> - length = -EINVAL;
> - if (sscanf(page, "%u", &new_value) != 1)
> + if (sscanf(page, "%u", &new_value) != 1) {
> + length = -EINVAL;
> goto out;
> + }
> + length = count;
>
> if (new_value) {
> char comm[sizeof(current->comm)];
>
> memcpy(comm, current->comm, sizeof(comm));
> - pr_err("SELinux: %s (%d) set checkreqprot to 1. This is deprecated and will be rejected in a future kernel release.\n",
> + pr_err("SELinux: %s (%d) set checkreqprot to 1. This is no longer supported.\n",
> comm, current->pid);
> }
>
> - checkreqprot_set((new_value ? 1 : 0));
> - if (new_value)
> - ssleep(15);
> - length = count;
> -
> selinux_ima_measure_state();
>
> out:
> --
> 2.40.0
>
