Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: 2021-05-26 11:45:20
Also in:
bpf, linux-fsdevel, linuxppc-dev, lkml, netdev, selinux
On Mon, May 17, 2021 at 1:00 PM Michael Ellerman [off-list ref] wrote:
Ondrej Mosnacek [off-list ref] writes:quoted
Commit 59438b46471a ("security,lockdown,selinux: implement SELinux lockdown") added an implementation of the locked_down LSM hook to SELinux, with the aim to restrict which domains are allowed to perform operations that would breach lockdown. However, in several places the security_locked_down() hook is called in situations where the current task isn't doing any action that would directly breach lockdown, leading to SELinux checks that are basically bogus. Since in most of these situations converting the callers such that security_locked_down() is called in a context where the current task would be meaningful for SELinux is impossible or very non-trivial (and could lead to TOCTOU issues for the classic Lockdown LSM implementation), fix this by modifying the hook to accept a struct cred pointer as argument, where NULL will be interpreted as a request for a "global", task-independent lockdown decision only. Then modify SELinux to ignore calls with cred == NULL. Since most callers will just want to pass current_cred() as the cred parameter, rename the hook to security_cred_locked_down() and provide the original security_locked_down() function as a simple wrapper around the new hook. The callers migrated to the new hook, passing NULL as cred: 1. arch/powerpc/xmon/xmon.c Here the hook seems to be called from non-task context and is only used for redacting some sensitive values from output sent to userspace.It's hard to follow but it actually disables interactive use of xmon entirely if lockdown is in confidentiality mode, and disables modifications of the kernel in integrity mode. But that's not really that important, the patch looks fine. Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Thanks, Michael! James/Paul, is there anything blocking this patch from being merged? Especially the BPF case is causing real trouble for people and the only workaround is to broadly allow lockdown::confidentiality in the policy. -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.