[PATCH linux-next] security: Fix side effects of default BPF LSM hooks

[KP Singh]

On Fri, Jun 10, 2022 at 2:44 AM Alexei Starovoitov
<alexei.starovoitov at gmail.com> wrote:
>
> On Thu, Jun 9, 2022 at 4:46 PM KP Singh <kpsingh at kernel.org> wrote:
> >
> > BPF LSM currently has a default implementation for each LSM hooks which
> > return a default value defined in include/linux/lsm_hook_defs.h. These
> > hooks should have no functional effect when there is no BPF program
> > loaded to implement the hook logic.
> >
> > Some LSM hooks treat any return value of the hook as policy decision
> > which results in destructive side effects.
> >
> > This issue and the effects were reported to me by Jann Horn:
> >
> > For a system configured with CONFIG_BPF_LSM and the bpf lsm is enabled
> > (via lsm= or CONFIG_LSM) an unprivileged user can vandalize the system
> > by removing the security.capability xattrs from binaries, preventing
> > them from working normally:
> >
> > $ getfattr -d -m- /bin/ping
> > getfattr: Removing leading '/' from absolute path names
> > security.capability=0sAQAAAgAgAAAAAAAAAAAAAAAAAAA=
> >
> > $ setfattr -x security.capability /bin/ping
> > $ getfattr -d -m- /bin/ping
> > $ ping 1.2.3.4
> > $ ping google.com
> > $ echo $?
> > 2
> >
> > The above reproduces with:
> >
> > cat /sys/kernel/security/lsm
> > capability,apparmor,bpf
>
> Why is this bpf related?
> apparmor doesn't have that hook,
> while capability returns 0.
> So bpf's default==0 doesn't change the situation.
>
> Just
> cat /sys/kernel/security/lsm
> capability
>
> would reproduce the issue?
> what am I missing?

capability does not define the inode_removexattr LSM hook:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/commoncap.c#n1449

It's only when the return value of the hook is 1, it checks for
cap_inode_removexattr.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/security.c#n1408

Only 3 LSMs define the hook (bpf, smack and selinux):

fgrep -R LSM_HOOK_INIT *  | grep inode_removexattr
selinux/hooks.c: LSM_HOOK_INIT(inode_removexattr, selinux_inode_removexattr),
smack/smack_lsm.c: LSM_HOOK_INIT(inode_removexattr, smack_inode_removexattr),

The BPF LSM default hooks intend to provide no side-effects when the
LSM is enabled and
for the hooks that the patch updates, there is a side-effect.