> On 3 May 2024, at 02:58, Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > The current security_inode_setxattr() and security_inode_removexattr() > hooks rely on individual LSMs to either call into the associated > capability hooks (cap_inode_setxattr() or cap_inode_removexattr()), or > return a magic value of 1 to indicate that the LSM layer itself should > perform the capability checks. Unfortunately, with the default return > value for these LSM hooks being 0, an individual LSM hook returning a > 1 will cause the LSM hook processing to exit early, potentially > skipping a LSM. Thankfully, with the exception of the BPF LSM, none > of the LSMs which currently register inode xattr hooks should end up > returning a value of 1, and in the BPF LSM case, with the BPF LSM hooks > executing last there should be no real harm in stopping processing of > the LSM hooks. However, the reliance on the individual LSMs to either > call the capability hooks themselves, or signal the LSM with a return > value of 1, is fragile and relies on a specific set of LSMs being > enabled. This patch is an effort to resolve, or minimize, these > issues. > > Before we discuss the solution, there are a few observations and > considerations that we need to take into account: > * BPF LSM registers an implementation for every LSM hook, and that > implementation simply returns the hook's default return value, a > 0 in this case. We want to ensure that the default BPF LSM behavior > results in the capability checks being called. The BPF LSM never intended to add default callbacks from the very first day: https://lwn.net/ml/linux-kernel/20200224171305.GA21886@xxxxxxxxxxxx/ But, we went ahead with a "compromise" because were were going to make the LSM layer better and tackle this problem with the broader enhancements for the LSM layer. Little did I know this would take 4 years (and counting...) from then. If you want to go ahead with this change for other reasons, please feel free to. But, I don't want the BPF LSM default callbacks being cited as a reason here. - KP > * SELinux and Smack do not expect the traditional capability checks > to be applied to the xattrs that they "own". > * SELinux and Smack are currently written in such a way that the > xattr capability checks happen before any additional LSM specific > access control checks. SELinux does apply SELinux specific access > controls to all xattrs, even those not "owned" by SELinux. > * IMA and EVM also register xattr hooks but assume that the LSM layer > and specific LSMs have already authorized the basic xattr operation. [...]