From a92d98a7fa7d6a7f3c11643d2f725b618d05643f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Tue, 14 Oct 2025 15:13:25 +0200 Subject: [PATCH] =?UTF-8?q?daemon:=20Attempt=20to=20map=20the=20=E2=80=9Ck?= =?UTF-8?q?vm=E2=80=9D=20group=20inside=20the=20build=20user=20namespace.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes . 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 and Reepca Russelstein 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 | 95 ++++++++++++++++++++++++++++++++++++++++--- tests/store.scm | 23 +++++++++++ 2 files changed, 112 insertions(+), 6 deletions(-) diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index a48214a9c0a..f455343c189 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -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> 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", - (format("%d %d 1") % guestGID % hostGID).str()); + runProgram("newgidmap", true, args); + 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> kvmGIDMapping() +{ + gid_t kvm = kvmGID(); + if (kvm == (gid_t) -1) + return {}; + else { + std::pair 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. */ - initializeUserNamespace(pid); + /* Initialize the UID/GID mapping of the child process. + + 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); } diff --git a/tests/store.scm b/tests/store.scm index 16dcbf2396d..82fb7a96cea 100644 --- a/tests/store.scm +++ b/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 + ;; . + (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"