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

[Casey Schaufler]

On 5/3/2024 9:26 AM, Paul Moore wrote:
> On Fri, May 3, 2024 at 11:51 AM Casey Schaufler <casey at schaufler-ca.com> wrote:
>> 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.
> Are you okay with the patch otherwise?  It's too late for v6.9, but if
> everyone is okay with this approach I'd like to merge this after the
> v6.10 merge window closes.

I'm not real happy with the shear size of the change, but I
don't see a better approach that meets the stated objectives.
You can add my Acked-by.

> I also need to track down the appropriate commits for the 'Fixes:'
> tag(s); I'm not entirely convinced that some of the other patches were
> targeting the proper commit ...
>