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] [day] [month] [year] [list]

Date:	Fri, 27 May 2016 16:27:07 -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):
> "Serge E. Hallyn" <serge@...lyn.com> writes:
> 
> > 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.
> 
> Hmm.
> 
> While there is an obvious conflict the two patches are doing different
> things.
> 
> s_user_ns is the owning user namespace of the filesystem.  And as such
> it is fine to write the old capability in that context.

Ok, right, so I should simply allow that in my patchset.  (Though it still
makes me nervous.)

To make that elegant the flow will need to change a bit, so easiest will
be to indeed wait for the s_user_ns patchset.

> You are making it possible to write the capability in child user
> namespaces, and I presume not allowing stomping a capability set by
> a more privileged user.
> 
> Unless you update your code to decide to write a v2 capability if rootid
> is zero and v3 otherwise the code will still have interesting
> entanglement issues.  Even then the code needs to look at s_user_ns to
> see what rootid should be.
> 
> Earlier today I pushed a for-testing branch to my user-namespace.git
> tree and that has the start of the s_user_ns stuff that I am pretty much
> ready to merge at this point.  I still need to rebase onto 4.7-rc1 and
> retest before I get farther.  But I am serious about getting this stuff
> reviewed and merged into my tree and into Linus' tree next merge window.
> 
> It is way past time.
> 
> Eric