bug#77862: guix-daemon run as non-root sets up /etc/group incorrectly in build container

[Ludovic Courtès]

Hi,

keinflue <keinflue@posteo.net> writes:

>>   2. I’m confused as to what makes the Coreutils test suite fail.
>
> The result from getgroups includes both the primary gid 30000 and a
> supplementary gid 65534 (where the repeated 65534 are the overflow gid
> produced by viewing supplementary gids that aren't mapped into the
> user namespace via /proc/[pid]/gid_map).
> Coreutils sees this and so assumes that it can do the equivalent of
>
> touch testfile
> chgrp 65534 testfile
>
> to create a file owned by group 30000 initially and to then change
> group ownership of that file to 65534. Normally an unprivileged user
> is allowed to change group ownership of files they own between groups
> that they are member of, so this would always succeed outside a user
> namespace context.
>
> However, any uid/gid used inside the user namespace is translated back
> to the host namespace via the uid/gid_map before permission
> checks. But in this case because 65534 doesn't map back to any gid in
> the host namespace, the syscall will fail.

Oh right, got it.

> If there is no supplementary group reported by getgroups at all, then
> coreutils just skips the test and it is ok again. Probably the
> coreutils test case should remove any gid reported by getgroups that
> is equal to the overflow gid before making that decision.
>
> Dropping all supplementary groups from the build process (after
> unshare and before writing "deny" to /proc/pid/setgroups) would make
> it so that this test case is always skipped by having getgroups only
> report 30000, however that would also drop the kvm group as mentioned
> above and is also not permitted in all environments (e.g. when the
> parent namespace already set /proc/[pid]/setgroups to "deny").
>
> So I think that instead either all supplementary groups of the user or
> at least the kvm group specifically needs to be mapped via
> /proc/[pid]/gid_map. When doing so getgroups would report 30000 and
> 984 (assuming identity gid map for 984) in your test case above and
> the coreutils test case would work again, because
>
> chgrp 984 testfile
>
> would then succeed with 984 mapping back to the host namespace to a
> supplementary group of the process.

Having reread user_namespaces(7), I don’t think we can change the set of
supplementary groups at all: that effectively requires root privileges.

So the best we can do is map the “kvm” group inside the user namespace.

> From a point of reproducibility and information leakage into the build
> container I think however that it would be preferable to not retain
> supplementary groups if possible. In contrast to the privileged build
> with a distinct build user that the can be given desired supplementary
> groups at will, the unprivileged environment may be one where the
> supplementary groups of the user running the daemon can't easily be
> changed to what is supposed to be seen in the build environment.

I agree, though in practice, the daemon will usually run under a
dedicated user anyway: this is what ‘guix-install.sh’ does on other
distros, and this is what happens with (privileged? #t) on Guix System.
In these cases, there’d be no observable difference.

The observable difference (namely getgroups(2) returning a list of
unmapped GIDs) would be when people run the daemon as their own user,
which is currently inconvenient.

> - I also noticed that the build container /etc/group is written with
>   65534 assumed as overflow gid. I am not sure whether anyone actually
>   does this, but the overflow uid/gid are technically configurable
>   (and retrievable) via sysctl entries
>   (/proc/sys/kernel/overflow(uid|gid)). 65534 is just the default
>  value.

Note that the “nobody” UID in /etc/passwd dates back to before the
unprivileged daemon implementation.  It just happens to match the
default overflow UID, but I agree we should use the right one here.

> - I also noticed that the operating-system defaults do not write an
>   entry for the overflow gid to /etc/group (while they do for the
>   overflow uid to /etc/passwd). I think such an entry should exist by
>   default as well. The entry for /etc/passwd also assumes the default
>   overflow uid of 65534. This isn't only relevant for a user namespace
>   context, but also file systems that can't map the whole range of
>  Linux uids/gids.

I’m not sure this needs to be changed because it’s not all that
different from the preexisting situation where “kvm” would not have an
entry in /etc/group.

Thanks,
Ludo’.