Re: [PATCH] user-namespaced file capabilities - now with even more magic

[Serge E. Hallyn]

Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
> > @@ -657,8 +898,11 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name,
> >  		       const void *value, size_t size, int flags)
> >  {
> >  	if (!strcmp(name, XATTR_NAME_CAPS)) {
> > -		if (!capable(CAP_SETFCAP))
> > +		/* Note - we want to use Seth's newer code here instead
> > */
> ^^^^^^^^^^^^^^^  What are you referring to here?  current_in_userns?

Referring specifically to

http://kernel.ubuntu.com/git/ubuntu/ubuntu-yakkety.git/commit/security/commoncap.c?id=e1804ed91602bc8ead616c9616de70096b139fa7

I just need to think about what precisely we want the rule to be here.

It's possible we just drop Seth's patch, as mine already allows writing
capabilities (though not v2) when not in init_user_ns, so his patch isn't
needed.

Seth's patch makes it possible to write v2 capabilitie (which are not
namespaced) to a file in non-init user-ns if the userns mounted the fs.

Mine does not allow that, ever, but will silently write a v3 capability.

Seth's patch never allows writing a file capability unlesss the whole
block device was mountd by the caller's user-ns.  Mine allows writing
v3 capabilities to such files.

So yeah, maybe mine simiply obviates the need for Seths' patch.

> > +		if (current_user_ns() == &init_user_ns && !capable(CAP_SETFCAP))
> >  			return -EPERM;
> > +		/* for non-init userns we'll check permission later in
> > +		 * cap_setxattr_make_nscap() */
> >  		return 0;
> >  	}
> >  
> > @@ -683,7 +927,11 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name,
> >  int cap_inode_removexattr(struct dentry *dentry, const char *name)
> >  {
> >  	if (!strcmp(name, XATTR_NAME_CAPS)) {
> > -		if (!capable(CAP_SETFCAP))
> > +		/* Note - we want to use Seth's newer code here instead */
> ^^^^^^^^^^^^^^^  What are you referring to here?  current_in_userns?
> > +		struct inode *inode = d_backing_inode(dentry);
> > +		if (!inode)
> > +			return -EINVAL;
> > +		if (!capable_wrt_inode_uidgid(inode, CAP_SETFCAP))
> >  			return -EPERM;
> >  		return 0;
> >  	}
> > @@ -1078,6 +1326,7 @@ struct security_hook_list capability_hooks[] = {
> >  	LSM_HOOK_INIT(bprm_secureexec, cap_bprm_secureexec),
> >  	LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv),
> >  	LSM_HOOK_INIT(inode_killpriv, cap_inode_killpriv),
> > +	LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity),
> >  	LSM_HOOK_INIT(mmap_addr, cap_mmap_addr),
> >  	LSM_HOOK_INIT(mmap_file, cap_mmap_file),
> >  	LSM_HOOK_INIT(task_fix_setuid, cap_task_fix_setuid),
> 
> Eric