linux-kernel - Re: [PATCH] user-namespaced file capabilities

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 27 May 2016 14:10:34 -0500
From:	"Serge E. Hallyn" <serge@...lyn.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	"Serge E. Hallyn" <serge@...lyn.com>,
	LKML <linux-kernel@...r.kernel.org>, Jann Horn <jann@...jh.net>,
	Seth Forshee <seth.forshee@...onical.com>,
	LSM <linux-security-module@...r.kernel.org>,
	"Andrew G. Morgan" <morgan@...nel.org>,
	Kees Cook <keescook@...omium.org>,
	Michael Kerrisk-manpages <mtk.manpages@...il.com>,
	Linux API <linux-api@...r.kernel.org>,
	Andy Lutomirski <luto@...capital.net>,
	Linux Containers <containers@...ts.linux-foundation.org>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>
Subject: Re: [PATCH] user-namespaced file capabilities - now with even more
 magic

Quoting Eric W. Biederman (ebiederm@...ssion.com):
> > @@ -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