daemon: Attempt to map the “kvm” group inside the build user namespace.

Fixes <https://issues.guix.gnu.org/77862>.

Previously, the ‘guix-daemon’ account (for unprivileged execution) would
typically have “kvm” as a supplementary group, but that group would not
be mapped in the build user namespace.  Consequently, attempts to
‘chown’ a file to that supplementary group would fail with EINVAL.

The test suites of Coreutils, Python, and Go (among others) exercise
this chown-to-supplementary-group behavior, so they would all fail when
started by the unprivileged ‘guix-daemon’ even though they succeed when
started by ‘guix-daemon’ running as root.

Thanks to keinflue <keinflue@posteo.net> and Reepca Russelstein
<reepca@russelstein.xyz> for helping out.

* nix/libstore/build.cc (initializeUserNamespace): Add ‘extraGIDs’ and
‘haveCapSetGID’ parameters.  Invoke ‘newgidmap’ when ‘extraGIDs’ is
non-empty and ‘haveCapSetGID’ is false.  Honor ‘extraGIDs’ when
‘haveCapSetGID’ is true.
(maxGroups, guestKVMGID): New variables.
(kvmGIDMapping): New function.
(DerivationGoal::startBuilder): Set ‘ctx.lockMountsMapAll’ in the
CLONE_NEWUSER case.  Pass ‘extraGIDs’ to ‘initializeUserNamespace’.
* tests/store.scm ("kvm GID is mapped"): New test.

Change-Id: I10ba710fc1b9ca1e3cd3122be1ec8ede5df18b40

nix/libstore/build.cc

@@ -25,6 +25,7 @@
 #include <sys/utsname.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <grp.h>
 #include <errno.h>
 #include <stdio.h>
 #include <cstring>
@@ -1646,15 +1647,81 @@ static void initializeUserNamespace(pid_t child,
 				    uid_t hostUID = getuid(),
 				    gid_t hostGID = getgid(),
                                     uid_t guestUID = guestUID,
-                                    gid_t guestGID = guestGID)
+                                    gid_t guestGID = guestGID,
+				    const std::vector<std::pair<gid_t, gid_t>> extraGIDs = {},
+				    bool haveCapSetGID = false)
 {
     writeFile("/proc/" + std::to_string(child) + "/uid_map",
 	      (format("%d %d 1") % guestUID % hostUID).str());
 
-    writeFile("/proc/" + std::to_string(child) + "/setgroups", "deny");
+    if (!haveCapSetGID && !extraGIDs.empty()) {
+	try {
+	    Strings args = {
+		std::to_string(child),
+		std::to_string(guestGID), std::to_string(hostGID), "1"
+	    };
+	    for (auto &pair: extraGIDs) {
+		args.push_back(std::to_string(pair.second));
+		args.push_back(std::to_string(pair.first));
+		args.push_back("1");
+	    }
 
-    writeFile("/proc/" + std::to_string(child) + "/gid_map",
+	    runProgram("newgidmap", true, args);
-	      (format("%d %d 1") % guestGID % hostGID).str());
+	    printMsg(lvlChatty,
+		     format("mapped %1% extra GIDs into namespace of PID %2%")
+		     % extraGIDs.size() % child);
+
+	    return;
+	} catch (const ExecError &e) {
+	    ignoreException();
+	}
+    }
+
+    if (!haveCapSetGID)
+	writeFile("/proc/" + std::to_string(child) + "/setgroups", "deny");
+
+    auto content = (format("%d %d 1\n") % guestGID % hostGID).str();
+    if (haveCapSetGID) {
+	for (auto &mapping: extraGIDs) {
+	    content += (format("%d %d 1\n") % mapping.second % mapping.first).str();
+	}
+    }
+    writeFile("/proc/" + std::to_string(child) + "/gid_map", content);
+}
+
+/* Maximum number of supplementary groups per user account.  */
+static const size_t maxGroups = 64;
+
+/* Return the ID of the "kvm" group or -1 if it does not exist or is not part
+   of the current user's supplementary groups.  */
+static gid_t kvmGID()
+{
+    struct group *kvm = getgrnam("kvm");
+    if (kvm == NULL) return -1;
+
+    gid_t groups[maxGroups];
+    int count = getgroups(maxGroups, groups);
+    if (count < 0) return -1;
+
+    for (int i = 0; i < count; i++) {
+	if (groups[i] == kvm->gr_gid) return kvm->gr_gid;
+    }
+
+    return -1;
+}
+
+/* GID of the "kvm" group in guest user namespaces.  */
+static gid_t guestKVMGID = 40000;
+
+static std::vector<std::pair<gid_t, gid_t>> kvmGIDMapping()
+{
+    gid_t kvm = kvmGID();
+    if (kvm == (gid_t) -1)
+	return {};
+    else {
+	std::pair<gid_t, gid_t> mapping(kvm, guestKVMGID);
+	return { mapping };
+    }
 }
 
 #if CHROOT_ENABLED
@@ -2931,13 +2998,29 @@ void DerivationGoal::startBuilder()
                               enableRouteLocalnetAction);
         }
 
+	if ((ctx.cloneFlags & CLONE_NEWUSER) != 0) {
+	    /* Have the 'lockMounts' phase re-map supplementary GIDs such as
+	       that of the "kvm" group.  */
+	    ctx.lockMountsMapAll = true;
+	}
+
         pid = cloneChild(ctx);
 
         if(childSetupSocket >= 0) childSetupSocket.close();
 
         if ((ctx.cloneFlags & CLONE_NEWUSER) != 0) {
-            /* Initialize the UID/GID mapping of the builder.  */
+	     /* Initialize the UID/GID mapping of the child process.
-            initializeUserNamespace(pid);
+
+		Try hard to map the "kvm" GID inside the user namespace ("kvm"
+	        is usually the only supplementary group of the 'guix-daemon'
+	        privilege separation user) so that package test suites that
+	        expect to be able to chown to supplementary groups can do so
+	        (without that mapping, attempts to chown to the supplementary
+	        group fail with EINVAL).  */
+	    auto extraGIDs = kvmGIDMapping();
+	    initializeUserNamespace(pid,
+				    getuid(), getgid(),
+				    guestUID, guestGID, extraGIDs);
             writeFull(parentSetupSocket, (unsigned char*)"go\n", 3);
         }
 

tests/store.scm

@@ -476,6 +476,29 @@
     (build-derivations %store (list d))
     (call-with-input-file (derivation->output-path d) read)))
 
+(unless (and (unprivileged-user-namespace-supported?)
+             (false-if-exception
+              (= (stat:gid (stat "/dev/kvm"))
+                 (group:gid (getgrnam "kvm"))))
+             (= 1 (status:exit-val (system* "newgidmap"))))
+  (test-skip 1))
+(test-assert "kvm GID is mapped"
+  ;; Ensure that the "kvm" GID is mapped into the build user namespace such
+  ;; that chown'ing a file to that GID works as expected.  See
+  ;; <https://issues.guix.gnu.org/77862>.
+  (let ((d (build-expression->derivation
+            %store "chown-to-supplementary-group"
+            `(let ((st (stat "/dev/kvm")))
+               ',(gettimeofday)
+               (pk 'supplementary-groups (getgroups))
+               (pk 'kvm-group (stat:gid st))
+               (unless (member (stat:gid st) (vector->list (getgroups)))
+                 (error "supplementary groups lack 'kvm' GID"))
+               (mkdir "test")
+               (chown "test" (getuid) (stat:gid st))
+               (mkdir %output)))))
+    (build-derivations %store (list d))))
+
 (unless (unprivileged-user-namespace-supported?)
   (test-skip 1))
 (test-equal "inputs are read-only"