Date:   Tue, 13 Oct 2020 07:46:50 -0500
From:   "Serge E. Hallyn" <serge@...lyn.com>
To:     Giuseppe Scrivano <gscrivan@...hat.com>
Cc:     Josh Triplett <josh@...htriplett.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Christian Brauner <christian.brauner@...ntu.com>,
        containers@...ts.linux-foundation.org,
        Alexander Mihalicyn <alexander@...alicyn.com>,
        Mrunal Patel <mpatel@...hat.com>, Wat Lim <watl@...gle.com>,
        Aleksa Sarai <cyphar@...har.com>,
        Pavel Tikhomirov <ptikhomirov@...tuozzo.com>,
        Geoffrey Thomas <geofft@...reload.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Joseph Christopher Sible <jcsible@...t.org>,
        Mickaël Salaün <mic@...ikod.net>,
        Vivek Goyal <vgoyal@...hat.com>,
        Andy Lutomirski <luto@...capital.net>,
        Stephane Graber <stgraber@...ntu.com>,
        Kees Cook <keescook@...omium.org>,
        Sargun Dhillon <sargun@...gun.me>, linux-kernel@...r.kernel.org
Subject: Re: LPC 2020 Hackroom Session: summary and next steps for isolated
 user namespaces

On Mon, Oct 12, 2020 at 07:05:10PM +0200, Giuseppe Scrivano wrote:
> Josh Triplett <josh@...htriplett.org> writes:
> 
> > On Fri, Oct 09, 2020 at 11:26:06PM -0500, Serge E. Hallyn wrote:
> >> > 3. Find a way to allow setgroups() in a user namespace while keeping
> >> >    in mind the case of groups used for negative access control.
> >> >    This was suggested by Josh Triplett and Geoffrey Thomas. Their idea was to
> >> >    investigate adding a prctl() to allow setgroups() to be called in a user
> >> >    namespace at the cost of restricting paths to the most restrictive
> >> >    permission. So if something is 0707 it needs to be treated as if it's 0000
> >> >    even though the caller is not in its owning group which is used for negative
> >> >    access control (how these new semantics will interact with ACLs will also
> >> >    need to be looked into).
> >> 
> >> I should probably think this through more, but for this problem, would it
> >> not suffice to add a new prevgroups grouplist to the struct cred, maybe
> >> struct group_info *locked_groups, and every time an unprivileged task creates
> >> a new user namespace, add all its current groups to this list?
> >
> > So, effectively, you would be allowed to drop permissions, but
> > locked_groups would still be checked for restrictions?
> >
> > That seems like it'd introduce a new level of complexity (a new facet of
> > permission) to manage. Not opposed, but it does seem more complex than
> > just opting out of using groups for negative permissions.
> 
> I have played with something similar in the past.  At that time I've
> discussed it only privately with Eric and we agreed it wasn't worth the
> extra complexity:
> 
> https://github.com/giuseppe/linux/commit/7e0701b389c497472d11fab8570c153a414050af

Hi, you linked the setgroups patch, but do you also have a link to the
attempt which you deemed was not worth it?

> instead of a prctl, I've added a new mode to /proc/PID/setgroups that
> allows setgroups in a userns locking the current gids.
> 
> What do you think about using /proc/PID/setgroups instead of a new
> prctl()?

It's better than not having it, but two concerns -

1. some userspace, especially testsuites, could become confused by the fact
that they can't drop groups no matter how hard they try, since these will all
still show up as regular groups.
2. whereas in my lockgroups proposal, lock_groups would only be taken into account
for permission denial, this proposal would count for permission grants too.  This
means that if I have a group which is permitted to read /foo/topsecret, and I
start a program in a new user namespace expecting it to drop that permission,
I can't have that, right?  The new program, will always have that permission?

Powered by blists - more mailing lists