On 6/10/2022 4:49 PM, KP Singh wrote:
On Fri, Jun 10, 2022 at 8:50 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
On 6/9/2022 4:46 PM, KP Singh 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
But not with SELinux as SELinux does the required check in its LSM hook:
cat /sys/kernel/security/lsm
capability,selinux,bpf
In this case security_inode_removexattr() calls
call_int_hook(inode_removexattr, 1, mnt_userns, dentry, name), which
expects a return value of 1 to mean "no LSM hooks hit" and 0 is
supposed to mean "the LSM decided to permit the access and checked
cap_inode_removexattr"
There are other security hooks that are similarly effected.
This shouldn't come as a surprise.
https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg2281257.html
is just one place where this sort of issue has been discussed.
In order to reliably fix this issue and also allow LSM Hooks and BPF
programs which implement hook logic to choose to not make a decision
in certain conditions (e.g. when BPF programs are used for auditing),
introduce a special return value LSM_HOOK_NO_EFFECT which can be used
by the hook to indicate to the framework that it does not intend to
make a decision.
The LSM infrastructure already has a convention of returning
-EOPNOTSUPP for this condition. Why add another value to check?'
This is not the case in call_int_hook currently.
If we can update the LSM infra to imply that -EOPNOTSUPP means
that the hook iteration can continue as that implies "no decision"
this would be okay as well.
It would be really nice if there was sufficient consistency in
the LSM infrastructure for this to make sense. The cases where
a module supplies a hook but only cares about the data some of
the time are rare, excepting for BPF. As I mention elsewhere,
general stacking is what you need.
