linux-kernel - Re: [PATCH v2] xattr: Enable security.capability in user namespaces

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: <7a39e8a6-a33b-f6a8-3fd5-6211c075ab91@linux.vnet.ibm.com>
Date:   Mon, 17 Jul 2017 16:50:22 -0400
From:   Stefan Berger <stefanb@...ux.vnet.ibm.com>
To:     Vivek Goyal <vgoyal@...hat.com>,
        Stefan Berger <StefanBergerstefanb@...ux.vnet.ibm.com>
Cc:     ebiederm@...ssion.com, containers@...ts.linux-foundation.org,
        lkp@...org, linux-kernel@...r.kernel.org, zohar@...ux.vnet.ibm.com,
        tycho@...ker.com, serge@...lyn.com,
        James.Bottomley@...senPartnership.com,
        christian.brauner@...lbox.org, amir73il@...il.com,
        linux-security-module@...r.kernel.org, casey@...aufler-ca.com
Subject: Re: [PATCH v2] xattr: Enable security.capability in user namespaces

On 07/17/2017 02:58 PM, Vivek Goyal wrote:
> On Tue, Jul 11, 2017 at 11:05:11AM -0400, Stefan Berger wrote:
>
> [..]
>> +/*
>> + * xattr_list_userns_rewrite - Rewrite list of xattr names for user namespaces
>> + *                             or determine needed size for attribute list
>> + *                             in case size == 0
>> + *
>> + * In a user namespace we do not present all extended attributes to the
>> + * user. We filter out those that are in the list of userns supported xattr.
>> + * Besides that we filter out those with @uid=<uid> when there is no mapping
>> + * for that uid in the current user namespace.
>> + *
>> + * @list:        list of 0-byte separated xattr names
>> + * @size:        the size of the list; may be 0 to determine needed list size
>> + * @list_maxlen: allocated buffer size of list
>> + */
>> +static ssize_t
>> +xattr_list_userns_rewrite(char *list, ssize_t size, size_t list_maxlen)
>> +{
>> +	char *nlist = NULL;
>> +	size_t s_off, len, nlen;
>> +	ssize_t d_off;
>> +	char *name, *newname;
>> +
>> +	if (!list || size < 0 || current_user_ns() == &init_user_ns)
> size will never be less than 0 here. Only caller calls this function only
> if size is >0. So can we remove this?

Correct.

>
> What about case of "!list". So if user space called listxattr(foo, NULL,
> 0), we want to return the size of buffer as if all the xattrs will be
> returned to user space. But in practice we probably will filter out some
> xattrs so actually returned string will be smaller than size reported
> previously.

This case of size=0 is a problem in userns. Depending on the mapping of 
the userid's the list can expand. A security.foo@...=100 can become 
security.foo@...=100000, if the mapping is set up so that uid 100 on the 
host becomes uid 100000 inside the container. So for now we only have 
security.capability and the way I solved this is by allocating a 65k 
buffer when calling from a userns. In this buffer where we gather the 
xattr names and then walk them to determine the size that's needed for 
the buffer by simulating the rewriting. It's not nice but I don't know 
of any other solution.


>
> Looks like that's the intent of "!list" condition here. Just wanted to
> make sure, hence asking.

Thanks for asking. I thought I had this case covered, but obviously I 
did not.

>
> BTW, I am testing this with overlayfs and trying to figure out if
> switching of creds will create issues. Simple operations like listxattr
> and getxattr and setxattr so far worked for me. And reason seems to be
> that name transformation we are doing in top layer based on creds of
> caller (and not based on creds of mounter).
>
> Vivek
>

    Stefan