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

[Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>]

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org):
> Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
> 
> > Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org):
> >> Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
> >> 
> >> > Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org):
> >> >> 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.
> 
> > It still seems to me no less flexible to require being in the parent
> > ns, so
> >
> >> >> +	if ((seq_ns != ns) && (seq_ns != ns->parent))
> >> >> +		return -EPERM;
> >
> > would become
> >
> >> >> +	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>

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