On 7/16/2019 7:21 AM, Andy Lutomirski wrote: > On Tue, Jul 16, 2019 at 7:03 AM Serge E. Hallyn <serge@xxxxxxxxxx> 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.
