Re: [RFC PATCH] security, capability: pass object information to security_capable

[Casey Schaufler <casey@schaufler-ca.com>]

On 7/16/2019 7:21 AM, Andy Lutomirski wrote:
> On Tue, Jul 16, 2019 at 7:03 AM Serge E. Hallyn <serge@hallyn.com> wrote:
>> On Fri, Jul 12, 2019 at 12:54:02PM -0700, Casey Schaufler wrote:
>>> On 7/12/2019 11:25 AM, Stephen Smalley wrote:
>>>> On 7/12/19 1:58 PM, Casey Schaufler wrote:
>>>>> On 7/12/2019 10:34 AM, Nicholas Franck wrote:
>>>>>> At present security_capable does not pass any object information
>>>>>> and therefore can neither audit the particular object nor take it
>>>>>> into account. Augment the security_capable interface to support
>>>>>> passing supplementary data. Use this facility initially to convey
>>>>>> the inode for capability checks relevant to inodes. This only
>>>>>> addresses capable_wrt_inode_uidgid calls; other capability checks
>>>>>> relevant to inodes will be addressed in subsequent changes. In the
>>>>>> future, this will be further extended to pass object information for
>>>>>> other capability checks such as the target task for CAP_KILL.
>>>>> This seems wrong to me. The capability system has nothing to do
>>>>> with objects. Passing object information through security_capable()
>>>>> may be convenient, but isn't relevant to the purpose of the interface.
>>>>> It appears that there are very few places where the object information
>>>>> is actually useful.
>>>> A fair number of capabilities are checked upon some attempted object access (often right after comparing UIDs or other per-object state), and the particular object can be very helpful in both audit and in access control.  More below.
>>> I'm not disagreeing with that. What I'm saying is that the capability
>>> check interface is not the right place to pass that information. The
>>> capability check has no use for the object information. I would much
>> I've had to argue this before while doing the namespaced file
>> capabilities implementation.  Perhaps this would be worth writing something
>> more formal about.  My main argument is, even if we want to claim that the
>> capabilities model is and should be object agnostic, the implementation
>> of user namespaces (currently) is such that the whole view of the user's
>> privilege must include information which is stored with the object.
>>
>> There are various user namespaces.
>>
>> The Linux capabilities ( :-) ) model is user namespaced.  It must be, in
>> order to be useful.  If we're going to use file capabilities in distros,
>> and distros are going to run in containers, then the capabilities must
>> be namespaced.  Otherwise, capabilities will not be used, and heck, should
>> just be dropped.
>>
>> The only way to find out which user namespace has privilege over an inode
>> is to look at the inode.
>>
>> Therefore, object information is needed.
> Agreed.  The concept in the kernel is "capability over a namespace."
>
> That being said, sticking a flexible object type into ns_capable()
> seems prematurely general to me.  How about adding
> security_capable_wrt_inode_uidgid() and allowing LSMs to hook that?
> The current implementation would go into commoncap.  The obvious
> extensions I can think of are security_dac_read_search(..., inode,
> ...) and security_dac_override(..., inode, ...).  (Or dentry or
> whatever is appropriate.)

Would you have an LSM interface for each capability then?
security_sysadmin()? security_chown()? Or do you want to add
security_hey_look_here_is_yet_another_special_case() for
each if () in the kernel?

Sorry, I got carried away. I've been wallowing in the LSM
for too long not to be sensitive to just how fragile the
whole thing is. Adding a bunch more single use interfaces
isn't going to help it be useful in the long run. Please,
let's not go hog wild adding LSM functions. Please.

>
> If this patch were restructured like that, the semantics would be
> obvious, and it would arguably be a genuine cleanup instead of a whole
> new mechanism of unknown scope.