On Wed, Sep 29, 2021 at 4:24 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> NOTE: This patch intentionally omits any "Fixes:" metadata or stable
> tagging since it removes a SELinux access control check; while
> removing the control point is the right thing to do moving forward,
> removing it in stable kernels could be seen as a regression.
>
> The original SELinux lockdown implementation in 59438b46471a
> ("security,lockdown,selinux: implement SELinux lockdown") used the
> current task's credentials as both the subject and object in the
> SELinux lockdown hook, selinux_lockdown(). Unfortunately that
> proved to be incorrect in a number of cases as the core kernel was
> calling the LSM lockdown hook in places where the credentials from
> the "current" task_struct were not the correct credentials to use
> in the SELinux access check.
>
> Attempts were made to resolve this by adding a credential pointer
> to the LSM lockdown hook as well as suggesting that the single hook
> be split into two: one for user tasks, one for kernel tasks; however
> neither approach was deemed acceptable by Linus. Faced with the
> prospect of either changing the subj/obj in the access check to a
> constant context (likely the kernel's label) or removing the SELinux
> lockdown check entirely, the SELinux community decided that removing
> the lockdown check was preferable.
>
> The supporting changes to the general LSM layer are left intact, this
> patch only removes the SELinux implementation.
>
> Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
I would probably also remove LSM_AUDIT_DATA_LOCKDOWN, but I don't care
enough to argue about it :)
Acked-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
/me goes off to prepare a patch to remove the lockdown test from the
testsuite...
> ---
> security/selinux/hooks.c | 30 ------------------------------
> security/selinux/include/classmap.h | 2 --
> 2 files changed, 32 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 6517f221d52c..11ebbbd65823 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7013,34 +7013,6 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
> }
> #endif
>
> -static int selinux_lockdown(enum lockdown_reason what)
> -{
> - struct common_audit_data ad;
> - u32 sid = current_sid();
> - int invalid_reason = (what <= LOCKDOWN_NONE) ||
> - (what == LOCKDOWN_INTEGRITY_MAX) ||
> - (what >= LOCKDOWN_CONFIDENTIALITY_MAX);
> -
> - if (WARN(invalid_reason, "Invalid lockdown reason")) {
> - audit_log(audit_context(),
> - GFP_ATOMIC, AUDIT_SELINUX_ERR,
> - "lockdown_reason=invalid");
> - return -EINVAL;
> - }
> -
> - ad.type = LSM_AUDIT_DATA_LOCKDOWN;
> - ad.u.reason = what;
> -
> - if (what <= LOCKDOWN_INTEGRITY_MAX)
> - return avc_has_perm(&selinux_state,
> - sid, sid, SECCLASS_LOCKDOWN,
> - LOCKDOWN__INTEGRITY, &ad);
> - else
> - return avc_has_perm(&selinux_state,
> - sid, sid, SECCLASS_LOCKDOWN,
> - LOCKDOWN__CONFIDENTIALITY, &ad);
> -}
> -
> struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
> .lbs_cred = sizeof(struct task_security_struct),
> .lbs_file = sizeof(struct file_security_struct),
> @@ -7349,8 +7321,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(perf_event_write, selinux_perf_event_write),
> #endif
>
> - LSM_HOOK_INIT(locked_down, selinux_lockdown),
> -
> /*
> * PUT "CLONING" (ACCESSING + ALLOCATING) HOOKS HERE
> */
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 084757ff4390..9859787e8e61 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -250,8 +250,6 @@ struct security_class_mapping secclass_map[] = {
> { COMMON_SOCK_PERMS, NULL } },
> { "perf_event",
> { "open", "cpu", "kernel", "tracepoint", "read", "write", NULL } },
> - { "lockdown",
> - { "integrity", "confidentiality", NULL } },
> { "anon_inode",
> { COMMON_FILE_PERMS, NULL } },
> { NULL }
>
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
