Package: guix;
Reported by: keinflue <keinflue <at> posteo.net>
Date: Thu, 17 Apr 2025 11:22:03 UTC
Severity: important
View this message in rfc822 format
From: Ludovic Courtès <ludo <at> gnu.org> To: keinflue <keinflue <at> posteo.net> Cc: 77862 <at> debbugs.gnu.org, Reepca Russelstein <reepca <at> russelstein.xyz> Subject: bug#77862: guix-daemon run as non-root sets up /etc/group incorrectly in build container Date: Mon, 19 May 2025 15:37:20 +0200
[Message part 1 (text/plain, inline)]
Hello, (Cc: Reepca.) keinflue <keinflue <at> posteo.net> writes: > It seems that the "chown to overflowgid" issue is somewhat > widespread. I also see the testsuite for go (bootstrap) failing in the > same way. I'd guess most implementations of "chown" system call > wrappers in various languages will have test cases like this that fail > to anticipate user namespaces. I will let my system build keep running > a bit longer and will then post the list of packages I found with log > excerpts here. I think it would be best to support chown-to-supplementary-group even with the unprivileged daemon (specifically for the case where guix-daemon runs as a dedicated user, and that this user has one supplementary group, kvm). The attached patch tries to do that, by calling out to ‘newuidmap’, and under the assumption that /etc/subgid allows mapping the ‘kvm’ group. It does the job (a build process can chown to ‘kvm’), but I couldn’t get the GID mapping preserved across the ‘unshare’ call (the call that is made to “lock” mounts), hence the “#if 0” there. The problem is that when we call ‘unshare’, the ‘newgidmap’ setuid binary is not longer accessible because we’re already in a chroot, so it seems that we cannot preserve the GID map. Thoughts? Ludo’.
[Message part 2 (text/x-patch, inline)]
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index a1f39d9a8b..83754b06bd 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -13,6 +13,7 @@
#include <map>
#include <sstream>
#include <algorithm>
+#include <iostream>
#include <limits.h>
#include <time.h>
@@ -23,6 +24,7 @@
#include <sys/utsname.h>
#include <fcntl.h>
#include <unistd.h>
+#include <grp.h>
#include <errno.h>
#include <stdio.h>
#include <cstring>
@@ -1635,15 +1637,77 @@ static const gid_t guestGID = 30000;
/* Initialize the user namespace of CHILD. */
static void initializeUserNamespace(pid_t child,
uid_t hostUID = getuid(),
- gid_t hostGID = getgid())
+ gid_t hostGID = getgid(),
+ 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");
+ }
+ 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();
+ }
+ std::cerr << "here i am\n";
+ }
- writeFile("/proc/" + std::to_string(child) + "/gid_map",
- (format("%d %d 1") % guestGID % hostGID).str());
+ 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);
+}
+
+/* Return the ID of the "kvm" group or -1 if it does not exist or is not part
+ of the current users supplementary groups. */
+static gid_t kvmGID()
+{
+ struct group *kvm = getgrnam("kvm");
+ if (kvm == NULL) return -1;
+
+ size_t max = 64;
+ gid_t groups[max];
+ int count = getgroups(max, 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;
+}
+
+static gid_t guestKVMGID = 40000;
+
+static std::vector<std::pair<gid_t, gid_t>> kvmGIDMapping()
+{
+ gid_t kvm = kvmGID();
+ if (kvm == -1)
+ return {};
+ else {
+ std::pair<gid_t, gid_t> mapping(kvm, guestKVMGID);
+ return { mapping };
+ }
}
void DerivationGoal::startBuilder()
@@ -2016,8 +2080,9 @@ void DerivationGoal::startBuilder()
readiness.readSide.close();
if ((flags & CLONE_NEWUSER) != 0) {
/* Initialize the UID/GID mapping of the child process. */
- initializeUserNamespace(pid);
- writeFull(readiness.writeSide, (unsigned char*)"go\n", 3);
+ auto extraGIDs = kvmGIDMapping();
+ initializeUserNamespace(pid, getuid(), getgid(), extraGIDs);
+ writeFull(readiness.writeSide, (unsigned char*)"go\n", 3);
}
readiness.writeSide.close();
} else
@@ -2269,10 +2334,12 @@ void DerivationGoal::runChild()
auto uid = getuid();
auto gid = getgid();
+#if 0 // FIXME!
if (unshare(CLONE_NEWNS | CLONE_NEWUSER) == -1)
throw SysError(format("creating new user and mount namespaces"));
- initializeUserNamespace(getpid(), uid, gid);
+ auto extraGIDs = { std::pair<gid_t, gid_t>(guestKVMGID, guestKVMGID) };
+ initializeUserNamespace(getpid(), uid, gid, extraGIDs, true);
/* Check that mounts within the build environment are "locked"
together and cannot be separated from within the build
@@ -2282,6 +2349,7 @@ void DerivationGoal::runChild()
check that this is what we get. */
int ret = umount(tmpDirInSandbox.c_str());
assert(ret == -1 && errno == EINVAL);
+#endif
}
}
#endif
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 56f116046c..b2c9b9f639 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -1081,9 +1081,11 @@ string runProgram(Path program, bool searchPath, const Strings & args)
/* Wait for the child to finish. */
int status = pid.wait(true);
- if (!statusOk(status))
+ std::cerr << "status not ok: " << status << "\n";
+ if (!statusOk(status)) {
throw ExecError(format("program `%1%' %2%")
% program % statusToString(status));
+ }
return result;
}
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.