user: allow setting multiple groups without user namespaces
authorPatrick Steinhardt <ps@pks.im>
Thu, 20 Jun 2019 09:45:27 +0000 (11:45 +0200)
committerPatrick Steinhardt <ps@pks.im>
Thu, 20 Jun 2019 10:12:16 +0000 (12:12 +0200)
When not using a user namespace, then we'll completely ignore
whether multiple groups have been specified by the user and only set
up the process's GID. With user namespaces, we in fact cannot set up
supplementary groups as we have set up "/proc/self/setgroups" to
deny any call to setgroups(2). But we can do better than that when
not using user namespaces, as we're free to use that syscall.

As nsjail(1) documents that "--group" can be specified multiple
times without mentioning that this won't work with
"--disable_clone_newuser", change the code to make that
constellation work.

user.cc

diff --git a/user.cc b/user.cc
index 674bf4c..84ee54c 100644 (file)
--- a/user.cc
+++ b/user.cc
@@ -242,18 +242,32 @@ bool initNsFromChild(nsjconf_t* nsjconf) {
        }
 
        /*
-        * Best effort because of /proc/self/setgroups
+        * Best effort because of /proc/self/setgroups. We deny
+        * setgroups(2) calls only if user namespaces are in use.
         */
-       LOG_D("setgroups(0, NULL)");
-       const gid_t* group_list = NULL;
-       if (setgroups(0, group_list) == -1) {
-               PLOG_D("setgroups(NULL) failed");
+       std::vector<gid_t> groups;
+       std::string groupsString = "[";
+       if (!nsjconf->clone_newuser && nsjconf->gids.size() > 1) {
+               for (auto it = nsjconf->gids.begin() + 1; it != nsjconf->gids.end(); it++) {
+                       groups.push_back(it->inside_id);
+                       groupsString += std::to_string(it->inside_id);
+                       if (it < nsjconf->gids.end() - 1)
+                               groupsString += ", ";
+               }
        }
+       groupsString += "]";
 
        if (!setResGid(nsjconf->gids[0].inside_id)) {
                PLOG_E("setresgid(%u)", nsjconf->gids[0].inside_id);
                return false;
        }
+
+       LOG_D("setgroups(%lu, %s)", groups.size(), groupsString.c_str());
+       if (setgroups(groups.size(), groups.data()) == -1) {
+               PLOG_D("setgroups(%lu, %s) failed", groups.size(), groupsString.c_str());
+               return false;
+       }
+
        if (!setResUid(nsjconf->uids[0].inside_id)) {
                PLOG_E("setresuid(%u)", nsjconf->uids[0].inside_id);
                return false;