Re: [PATCH v3 0/1] Relax restrictions on user.* xattr
From: Eric W. Biederman
Date: Wed Sep 08 2021 - 10:21:33 EST
Miklos Szeredi <miklos@xxxxxxxxxx> writes:
> On Tue, 7 Sept 2021 at 23:40, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>>
>> On Mon, Sep 06, 2021 at 04:56:44PM +0200, Miklos Szeredi wrote:
>> > On Mon, 6 Sept 2021 at 16:39, Dr. David Alan Gilbert
>> > <dgilbert@xxxxxxxxxx> wrote:
>> >
>> > > IMHO the real problem here is that the user/trusted/system/security
>> > > 'namespaces' are arbitrary hacks rather than a proper namespacing
>> > > mechanism that allows you to create new (nested) namespaces and associate
>> > > permissions with each one.
>> >
>> > Indeed.
>> >
>> > This is what Eric Biederman suggested at some point for supporting
>> > trusted xattrs within a user namespace:
>> >
>> > | For trusted xattrs I think it makes sense in principle. The namespace
>> > | would probably become something like "trusted<ns-root-uid>.".
>> >
>> > Theory sounds simple enough. Anyone interested in looking at the details?
>>
>> So this namespaced trusted.* xattr domain will basically avoid the need
>> to have CAP_SYS_ADMIN in init_user_ns, IIUC. I guess this is better
>> than giving CAP_SYS_ADMIN in init_user_ns.
>
> That's the objective, yes. I think the trick is getting filesystems
> to store yet another xattr type.
Using the uid of the root user of a user namespace is probably the best
idea we have so far for identifying a user namespace in persistent
on-disk meta-data. We ran into a little trouble using that idea
for file capabilities.
The key problem was there are corner cases where some nested user
namespaces have the same root user id as their parent namespaces. This
has the potential to allow privilege escalation if the creator of the
user namespace does not have sufficient capabilities.
The solution we adopted can be seen in db2e718a4798 ("capabilities:
require CAP_SETFCAP to map uid 0").
That solution is basically not allowing the creation of user namespaces
that could have problems. I think use trusted xattrs this way the code
would need to treat CAP_SYS_ADMIN the same way it currently treats
CAP_SETFCAP.
Eric