Re: [PATCH review 12/16] userns: For /proc/self/{uid, gid}_map derive the lower userns from the struct file

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org):

Serge Hallyn [off-list ref] writes:

quoted

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org):

quoted

Serge Hallyn [off-list ref] writes:

quoted

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org):

quoted

From: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

To keep things sane in the context of file descriptor passing derive the
user namespace that uids are mapped into from the opener of the file
instead of from current.

When writing to the maps file the lower user namespace must always
be the parent user namespace, or setting the mapping simply does
not make sense.  Enforce that the opener of the file was in
the parent user namespace or the user namespace whose mapping
is being set.

Is there a reasonable use case for writing from the ns whose mapping
is being set?  Are you expecting cases where the child opens the file
and passes it back to the parent to set the mappings?

Passing the open mappings file no.  Although by using seq_user_ns I do
make certain the semantics are correct if the file descriptor is passed,
but I did that on general principles.

I expect a process in the user namespace to be able to meaningfully set
the mapping to some the current uid and the current gid.

Sorry, I think a word is missing there.  To be precise (bc I haven't
thought about this much before as it's not my target goal :) you're
saying if I'm uid 1000 gid 1000, I can create a new user namespace
and, from inside that new userns (where I'm first uid/gid -1) I can
map any uid+gid in the container to 1000 in the parent ns?  Or is there
something more?

Only that for now.  I had once imagined magic would happen in the
background to verify the parent.

quoted

It still seems to me no less flexible to require being in the parent
ns, so

quoted

quoted

quoted

+	if ((seq_ns != ns) && (seq_ns != ns->parent))
+		return -EPERM;

would become

quoted

quoted

quoted

+	if (seq_ns != ns->parent)
+		return -EPERM;

In practice when playing around it is the difference between.
unshare -U /bin/bash
echo 0 1000 1 > /proc/self/uid_map

And the need to pre-plan something.  You can set the uid_map from the
parent in a shell script but it is a real pain.  So for just messing
around allowing seq_ns == ns is a real advantage.

Heh, ok - I almost always want >1 uid mapped, but I can see the
advantage.

Thanks.

I don't recall whether I put this in originally, but

Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>

quoted

I also wonder if -EINVAL would be a more appropriate choice here.
We're trying to keep things sane, rather than saying "not allowed"
for its own sake.

A different error code might be better.

I suppose strictly speaking (looking at errno-base.h) it would be EBADF?

-serge