[RFC PATCH] lsm: fixup the inode xattr capability handling

[Casey Schaufler]

On 5/3/2024 8:36 AM, Paul Moore wrote:
> On Fri, May 3, 2024 at 11:26 AM Casey Schaufler <casey at schaufler-ca.com> wrote:
>> On 5/2/2024 5:58 PM, Paul Moore 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,
>> https://lore.kernel.org/lkml/20231215221636.105680-1-casey@schaufler-ca.com/T/#mac61625dc1983d13ee5e8015fd22e1165381f079
>>
>> ... or am I missing something?
> Yes, that patch, as well as some of the others that have been posted,
> change the ordering of the access control checks, moving the LSM-based
> checks ahead of the capability-based checks.  The patch I'm proposing
> here not only preserves the current ordering, but it sticks with our
> access control convention of DAC-before-LSM.

Fair enough.