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

Serge Hallyn
2012-11-19T21:19:19Z

Quoting Eric W. Biederman (ebiederm@xmission.com):
> Serge Hallyn <serge.hallyn@canonical.com> writes:
>
> > Quoting Eric W. Biederman (ebiederm@xmission.com):
> >> Serge Hallyn <serge.hallyn@canonical.com> writes:
> >>
> >> > Quoting Eric W. Biederman (ebiederm@xmission.com):
> >> >> From: "Eric W. Biederman" <ebiederm@xmission.com>
> >> >>
> >> >> 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@ubuntu.com>

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