One thing overlooked by commit 9cc46516ddf4 ("userns: Add a knob to
disable setgroups on a per user namespace basis") is that because
setgroups(2) no longer works in user namespaces it doesn't make any
sense to be returning weird group IDs that the process cannot do
anything with.
This code works the same weather or not setgroups is enabled.
Yes. Sorry, I explain the reasoning for this better below. But the point
is that "65534" is ambiguous in this context and can lead to confusion
for a userspace program.
However, you have a point that when setgroups is enabled then this code
will effectively hide groups which the process may wish to drop. I'm not
sure what you would like to do in this instance -- but I'd prefer if we
first agree on whether this is a worthwhile issue to solve in the kernel
or if userspace should just hack around it (which I've already partially
done[1]).
This change, along with the other changes made to require unprivileged
users to always disable setgroups(2), means that userspace programs such
as apt break inside rootless containers. While this change does change
the userspace ABI, any userspace program that has to deal with
getgroups(2) would have to filter out these "fake" group IDs anyway.
This just makes it so that less applications will have to handle this
broken API.
Is it broken? Unless I am mistaken if we have a 16bit getgroups
call and we 32bit group ids getgroups it behaves exactly the same way.
When I say "broken" what I mean is that getgroups(2) is telling
userspace that "you are a member of group 65534". But you now have two
different (confusing) cases that can result:
1. That group is not mapped. Which means that anything that application
assumes about getgroups(2) returning sane results now needs to be
cross-checked with /proc/self/uid_map and checking whether setgroups
will actually work. Apt is an example of such a program -- it attempts
to drop all privileges using setgroups(2) because getgroups(2) tells it
that has some supplementary groups.
2. That group _is_ mapped. Now the application has no way of knowing
whether it actually is in group 65534 (aside from experimenting with
files and trying to see what its _actual_ groups are).
Note that in both cases it is not simple to be completely sure whether
the 65534 that getgroups(2) returned actually means "unmapped" or
The value we is (u16)-2. The traditional linux group id for this
purpose.
Okay, but 65534 is a valid group ID as well. So while in principle it's
reserved for this purpose, returning an "invalid" group ID that is
actually a valid value is just confusing.
In all other contexts the best we can do for applications has been to
return the user id or group id that says the value you are looking for
does not fit in this context. Which makes me suspect this is not the
right solution for getgroups.
While with getuid() and getgid() I understand this logic (though I
strongly feel there should be an EUNMAPPED, I understand why it doesn't
exist). With get{u,g}id() there is an implicit statement that
overflow_{u,g}id should be semantically equivalent to "unmapped {U,G}ID".
However, this doesn't (IMO) apply to getgroups(2). getgroups(2) tells
you what groups you are a member of, and there is a semantic difference
to "you are a member of group #overflow_gid" and "you are in an unmapped
group".
I don't know why apt breaks. You have not described that. Perhaps apt
is seeing something misconfigured and complaining properly.
I investigated this quite a bit while trying to get rootless containers
to work with runC. After reading the code again, apt actually does two
things:
1. Unconditionally does setgroups(1, &some_gid). This obviously breaks
in rootless containers, but can be fixed.
2. It then has some checks to verify that it has dropped privileges
correctly. Now, you _can_ disable these checks but I would prefer not to
(many other programs have similar code to make sure they are safely
dropping privileges). If you look in apt-pkg/contrib/fileutl.cc you'll
see this [abbreviated] function:
bool DropPrivileges()
{
/* ... */
// Verify that the user isn't still in any supplementary groups
long const ngroups_max = sysconf(_SC_NGROUPS_MAX);
std::unique_ptr<gid_t[]> gidlist(new gid_t[ngroups_max]);
if (unlikely(gidlist == NULL))
return _error->Error("Allocation of a list of size %lu for getgroups
failed", ngroups_max);
ssize_t gidlist_nr;
if ((gidlist_nr = getgroups(ngroups_max, gidlist.get())) < 0)
return _error->Errno("getgroups", "Could not get new groups (%lu)",
ngroups_max);
for (ssize_t i = 0; i < gidlist_nr; ++i)
if (gidlist[i] != pw->pw_gid)
return _error->Error("Could not switch group, user %s is still in
group %d", toUser.c_str(), gidlist[i]);
/* ... */
}
Which will fail in rootless containers, and there's nothing you can do
as a user other than disable the checks and modify the apt source code.
There are almost certainly more examples (searching github found things
like qmail also appear to have some similar logic, and I think some
components of Varnish might too).
Also, I found that this "secure coding standards" document recommends
this sort of practice when writing software that needs to drop
privileges[2]. I'm sure there exists plenty of userspace software which
implements this kind of logic.
The reason you haven't seen bug reports (or people shouting on the ML)
for this is that people are still not really running rootless containers
yet -- the work I'm doing in runC is what's uncovering these sorts of
issues.
I can be persauded but I need a better argument than this change makes
one applicaiton work for me.
I need to get better at writing commit messages, sorry about that. As I
mentioned above, the biggest issue is the ambiguity of 65534. The fact
that apt doesn't react in a sane way is a symptom of that issue, and
there are almost certainly more cases of programs that act in this way.
[1]: https://github.com/cyphar/remainroot
[2]:
https://www.securecoding.cert.org/confluence/display/c/POS36-C.+Observe+correct+revocation+order+while+relinquishing+privileges
--
Aleksa Sarai
Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
