Re: [PATCH 02/14] allow root in container to copy namespaces (v3)

From: Serge E. Hallyn
Date: Thu Aug 04 2011 - 18:01:32 EST


Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
> "Serge E. Hallyn" <serge.hallyn@xxxxxxxxxxxxx> writes:
>
> > Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
> >> The dangers of changing the namespace of a process remain the same,
> >> confused suid programs. I don't believe there are any unique new
> >> dangers.
> >>
> >> Not allowing joining namespaces you already have a copy of is just
> >> a matter of making it hard to get things wrong.
> >>
> >> I would feel more a bit more comfortable if the way we did this was
> >> to move all of the capable calls into the per namespace methods
> >> and then changed them one namespace at a time. I don't think
> >
> > The patch belows moves them into the per namespace methods, for
> > what it's worth. If you like I can change them, for now, to
> > 'capable(CAP_SYS_ADMIN)' targeted at init_user_ns, but if we're
> > targetting at the userns owning the destination namespace, it
> > seems this must be sufficient...
>
> I like the was this was done. I was mostly thinking of the non
> setns case when I was talking about moving the calls.

Oh, you mean unshare and copy namespaces?

(The flow on those paths is scary to touch :)

> >> there are any fundmanetal dangers of allowing unshare without
> >> the global CAP_SYS_ADMIN, but it would be good to be able to audit
> >
> > If you have suspicions that there may in fact be dangers, then
> > perhaps this whole patch should be delayed, and copy_namespaces()
> > and unshare_nsproxy_namespaces() should continue to check global
> > CAP_SYS_ADMIN? The only part which would remain would be the
> > moving of the setns capable check into the per-ns ->install
> > method, but it would check the global CAP_SYS_ADMIN?
>
> Yes. I am in favor of delaying this and making the changes one
> namespace at a time. I don't think there are real dangers but I do
> think we should try and think through the possible dangers.

Ok, so for now here is a patch to fold into the previous one
which I think sets us at a reasonable point.