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]

Message-ID: <87y46vcn3a.fsf@x220.int.ebiederm.org>
Date:	Fri, 27 May 2016 14:33:45 -0500
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	"Serge E. Hallyn" <serge@...lyn.com>
Cc:	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

"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.

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