linux-kernel - Re: LPC 2020 Hackroom Session: summary and next steps for isolated user namespaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ft6act3c.fsf@redhat.com>
Date:   Mon, 19 Oct 2020 14:12:39 +0200
From:   Giuseppe Scrivano <gscrivan@...hat.com>
To:     "Serge E. Hallyn" <serge@...lyn.com>
Cc:     Josh Triplett <josh@...htriplett.org>,
        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

"Serge E. Hallyn" <serge@...lyn.com> writes:

> On Tue, Oct 13, 2020 at 05:17:36PM +0200, Giuseppe Scrivano wrote:
>> "Serge E. Hallyn" <serge@...lyn.com> writes:
>> 
>> > 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?
>> 
>> it was just part of a private discussion; but was 4 years ago so we can
>> probably revisit and accept the additional complexity since setgroups()
>> is still an issue with user namespaces.
>> 
>> 
>> >> 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.
>> 
>> I forgot to send a link to a second patch :-) that completes the feature:
>> https://github.com/giuseppe/linux/commit/1c5fe726346b216293a527719e64f34e6297f0c2
>> 
>> When the new mode is used, the gids that are not known in the userns do
>> not show up in userspace.
>
> Ah, right - and of course those gids better not be mapped into the namespace :)
>
> But so, this is the patch you said you agreed was not worth the extra
> complexity?

yes, these two patches are what looked too complex at that time.  The
problem still exists though, we could perhaps reconsider if the
extra-complexity is acceptable to address it.

Regards,
Giuseppe