From c6617d6f2e206436bb3f3717d38dedabe1836ff7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 15:53:05 +0200 Subject: [PATCH 01/28] Remove `buildUser` from `DerivationBuilder` The use of a `buildUser` is an implementation detail of some types of sandboxes that shouldn't exposed. --- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/unix/build/derivation-builder.cc | 7 ++++++- .../unix/include/nix/store/build/derivation-builder.hh | 5 ----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 850d21bca..02f80b65e 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -895,8 +895,8 @@ Goal::Co DerivationGoal::tryToBuild() builder->startBuilder(); } catch (BuildError & e) { + builder.reset(); outputLocks.unlock(); - builder->buildUser.reset(); worker.permanentFailure = true; co_return done(BuildResult::InputRejected, {}, std::move(e)); } diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index e84e2db6e..a086f68ca 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -113,6 +113,11 @@ public: private: + /** + * User selected for running the builder. + */ + std::unique_ptr buildUser; + /** * The cgroup of the builder, if any. */ @@ -271,7 +276,7 @@ public: /** * Start building a derivation. */ - void startBuilder() override;; + void startBuilder() override; /** * Tear down build environment after the builder exits (either on diff --git a/src/libstore/unix/include/nix/store/build/derivation-builder.hh b/src/libstore/unix/include/nix/store/build/derivation-builder.hh index 81a574fd0..e16162b7a 100644 --- a/src/libstore/unix/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/unix/include/nix/store/build/derivation-builder.hh @@ -145,11 +145,6 @@ struct DerivationBuilderCallbacks */ struct DerivationBuilder : RestrictionContext { - /** - * User selected for running the builder. - */ - std::unique_ptr buildUser; - /** * The process ID of the builder. */ From 189fdfa7762a925b20e4eba030c125cc7b81a276 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 15:55:58 +0200 Subject: [PATCH 02/28] Remove duplicate comments on DerivationBuilderImpl overriden methods Having the exact same doc comments isn't very useful/maintainable. --- src/libstore/unix/build/derivation-builder.cc | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index a086f68ca..7903fe5df 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -263,30 +263,10 @@ private: public: - /** - * Set up build environment / sandbox, acquiring resources (e.g. - * locks as needed). After this is run, the builder should be - * started. - * - * @returns true if successful, false if we could not acquire a build - * user. In that case, the caller must wait and then try again. - */ bool prepareBuild() override; - /** - * Start building a derivation. - */ void startBuilder() override; - /** - * Tear down build environment after the builder exits (either on - * its own or if it is killed). - * - * @returns The first case indicates failure during output - * processing. A status code and exception are returned, providing - * more information. The second case indicates success, and - * realisations for each output of the derivation are returned. - */ std::variant, SingleDrvOutputs> unprepareBuild() override; private: @@ -318,10 +298,6 @@ private: public: - /** - * Stop the in-process nix daemon thread. - * @see startDaemon - */ void stopDaemon() override; private: @@ -353,15 +329,8 @@ private: public: - /** - * Delete the temporary directory, if we have one. - */ void deleteTmpDir(bool force) override; - /** - * Kill any processes running under the build user UID or in the - * cgroup of the build. - */ void killSandbox(bool getStats) override; private: From af1b580ff6bce3166246ee9dc5c5197a0182e31e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 17:12:54 +0200 Subject: [PATCH 03/28] DerivationBuilderImpl: Drop std::optional from derivationType No point in computing this lazily, since it's pretty much the first thing the DerivationBuilder does. --- src/libstore/unix/build/derivation-builder.cc | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 7903fe5df..027c4aa3b 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -107,6 +107,7 @@ public: : DerivationBuilderParams{std::move(params)} , store{store} , miscMethods{std::move(miscMethods)} + , derivationType(drv.type()) { } LocalStore & getLocalStore(); @@ -175,9 +176,9 @@ private: /** * The sort of derivation we are building. * - * Just a cached value, can be recomputed from `drv`. + * Just a cached value, computed from `drv`. */ - std::optional derivationType; + const DerivationType derivationType; /** * Stuff we need to pass to initChild(). @@ -445,9 +446,6 @@ void DerivationBuilderImpl::killSandbox(bool getStats) bool DerivationBuilderImpl::prepareBuild() { - /* Cache this */ - derivationType = drv.type(); - /* Are we doing a chroot build? */ { if (settings.sandboxMode == smEnabled) { @@ -464,7 +462,7 @@ bool DerivationBuilderImpl::prepareBuild() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = derivationType->isSandboxed() && !drvOptions.noChroot; + useChroot = derivationType.isSandboxed() && !drvOptions.noChroot; } auto & localStore = getLocalStore(); @@ -601,11 +599,10 @@ std::variant, SingleDrvOutputs> Derivation return std::move(builtOutputs); } catch (BuildError & e) { - assert(derivationType); BuildResult::Status st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - !derivationType->isSandboxed() || diskFull ? BuildResult::TransientFailure : + !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; return std::pair{std::move(st), std::move(e)}; @@ -1081,7 +1078,7 @@ void DerivationBuilderImpl::startBuilder() "nogroup:x:65534:\n", sandboxGid())); /* Create /etc/hosts with localhost entry. */ - if (derivationType->isSandboxed()) + if (derivationType.isSandboxed()) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -1309,7 +1306,7 @@ void DerivationBuilderImpl::startBuilder() ProcessOptions options; options.cloneFlags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | CLONE_PARENT | SIGCHLD; - if (derivationType->isSandboxed()) + if (derivationType.isSandboxed()) options.cloneFlags |= CLONE_NEWNET; if (usingUserNamespace) options.cloneFlags |= CLONE_NEWUSER; @@ -1515,7 +1512,7 @@ void DerivationBuilderImpl::initEnv() derivation, tell the builder, so that for instance `fetchurl' can skip checking the output. On older Nixes, this environment variable won't be set, so `fetchurl' will do the check. */ - if (derivationType->isFixed()) env["NIX_OUTPUT_CHECKED"] = "1"; + if (derivationType.isFixed()) env["NIX_OUTPUT_CHECKED"] = "1"; /* *Only* if this is a fixed-output derivation, propagate the values of the environment variables specified in the @@ -1526,7 +1523,7 @@ void DerivationBuilderImpl::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (!derivationType->isSandboxed()) { + if (!derivationType.isSandboxed()) { auto & impureEnv = settings.impureEnv.get(); if (!impureEnv.empty()) experimentalFeatureSettings.require(Xp::ConfigurableImpureEnv); @@ -1876,7 +1873,7 @@ void DerivationBuilderImpl::runChild() userNamespaceSync.readSide = -1; - if (derivationType->isSandboxed()) { + if (derivationType.isSandboxed()) { /* Initialise the loopback interface. */ AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP)); @@ -1952,7 +1949,7 @@ void DerivationBuilderImpl::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (!derivationType->isSandboxed()) { + if (!derivationType.isSandboxed()) { // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the From 3733f95ae5b6731af01e311fcee8a2e4d655986a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 19:42:07 +0200 Subject: [PATCH 04/28] DerivationBuilder: Move Linux/Darwin-specific code into subclasses --- .../unix/build/darwin-derivation-builder.cc | 181 +++ src/libstore/unix/build/derivation-builder.cc | 1054 ++++------------- .../unix/build/linux-derivation-builder.cc | 568 +++++++++ 3 files changed, 956 insertions(+), 847 deletions(-) create mode 100644 src/libstore/unix/build/darwin-derivation-builder.cc create mode 100644 src/libstore/unix/build/linux-derivation-builder.cc diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc new file mode 100644 index 000000000..3366403a7 --- /dev/null +++ b/src/libstore/unix/build/darwin-derivation-builder.cc @@ -0,0 +1,181 @@ +#ifdef __APPLE__ + +struct DarwinDerivationBuilder : DerivationBuilderImpl +{ + DarwinDerivationBuilder( + Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) + : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) + { + useChroot = true; + } + + void execBuilder(const Strings & args, const Strings & envStrs) override + { + posix_spawnattr_t attrp; + + if (posix_spawnattr_init(&attrp)) + throw SysError("failed to initialize builder"); + + if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) + throw SysError("failed to initialize builder"); + + if (drv.platform == "aarch64-darwin") { + // Unset kern.curproc_arch_affinity so we can escape Rosetta + int affinity = 0; + sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); + + cpu_type_t cpu = CPU_TYPE_ARM64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } else if (drv.platform == "x86_64-darwin") { + cpu_type_t cpu = CPU_TYPE_X86_64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } + + posix_spawn( + NULL, drv.builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); + } + + void setUser() override + { + DerivationBuilderImpl::setUser(); + + /* This has to appear before import statements. */ + std::string sandboxProfile = "(version 1)\n"; + + if (useChroot) { + + /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ + PathSet ancestry; + + /* We build the ancestry before adding all inputPaths to the store because we know they'll + all have the same parents (the store), and there might be lots of inputs. This isn't + particularly efficient... I doubt it'll be a bottleneck in practice */ + for (auto & i : pathsInChroot) { + Path cur = i.first; + while (cur.compare("/") != 0) { + cur = dirOf(cur); + ancestry.insert(cur); + } + } + + /* And we want the store in there regardless of how empty pathsInChroot. We include the innermost + path component this time, since it's typically /nix/store and we care about that. */ + Path cur = store.storeDir; + while (cur.compare("/") != 0) { + ancestry.insert(cur); + cur = dirOf(cur); + } + + /* Add all our input paths to the chroot */ + for (auto & i : inputPaths) { + auto p = store.printStorePath(i); + pathsInChroot[p] = p; + } + + /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be + * configurable */ + if (settings.darwinLogSandboxViolations) { + sandboxProfile += "(deny default)\n"; + } else { + sandboxProfile += "(deny default (with no-log))\n"; + } + + sandboxProfile += +# include "sandbox-defaults.sb" + ; + + if (!derivationType->isSandboxed()) + sandboxProfile += +# include "sandbox-network.sb" + ; + + /* Add the output paths we'll use at build-time to the chroot */ + sandboxProfile += "(allow file-read* file-write* process-exec\n"; + for (auto & [_, path] : scratchOutputs) + sandboxProfile += fmt("\t(subpath \"%s\")\n", store.printStorePath(path)); + + sandboxProfile += ")\n"; + + /* Our inputs (transitive dependencies and any impurities computed above) + + without file-write* allowed, access() incorrectly returns EPERM + */ + sandboxProfile += "(allow file-read* file-write* process-exec\n"; + + // We create multiple allow lists, to avoid exceeding a limit in the darwin sandbox interpreter. + // See https://github.com/NixOS/nix/issues/4119 + // We split our allow groups approximately at half the actual limit, 1 << 16 + const size_t breakpoint = sandboxProfile.length() + (1 << 14); + for (auto & i : pathsInChroot) { + + if (sandboxProfile.length() >= breakpoint) { + debug("Sandbox break: %d %d", sandboxProfile.length(), breakpoint); + sandboxProfile += ")\n(allow file-read* file-write* process-exec\n"; + } + + if (i.first != i.second.source) + throw Error( + "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin", + i.first, + i.second.source); + + std::string path = i.first; + auto optSt = maybeLstat(path.c_str()); + if (!optSt) { + if (i.second.optional) + continue; + throw SysError("getting attributes of required path '%s", path); + } + if (S_ISDIR(optSt->st_mode)) + sandboxProfile += fmt("\t(subpath \"%s\")\n", path); + else + sandboxProfile += fmt("\t(literal \"%s\")\n", path); + } + sandboxProfile += ")\n"; + + /* Allow file-read* on full directory hierarchy to self. Allows realpath() */ + sandboxProfile += "(allow file-read*\n"; + for (auto & i : ancestry) { + sandboxProfile += fmt("\t(literal \"%s\")\n", i); + } + sandboxProfile += ")\n"; + + sandboxProfile += drvOptions.additionalSandboxProfile; + } else + sandboxProfile += +# include "sandbox-minimal.sb" + ; + + debug("Generated sandbox profile:"); + debug(sandboxProfile); + + /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different + mechanisms to find temporary directories, so we want to open up a broader place for them to put their files, + if needed. */ + Path globalTmpDir = canonPath(defaultTempDir(), true); + + /* They don't like trailing slashes on subpath directives */ + while (!globalTmpDir.empty() && globalTmpDir.back() == '/') + globalTmpDir.pop_back(); + + if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") { + Strings sandboxArgs; + sandboxArgs.push_back("_GLOBAL_TMP_DIR"); + sandboxArgs.push_back(globalTmpDir); + if (drvOptions.allowLocalNetworking) { + sandboxArgs.push_back("_ALLOW_LOCAL_NETWORKING"); + sandboxArgs.push_back("1"); + } + char * sandbox_errbuf = nullptr; + if (sandbox_init_with_parameters( + sandboxProfile.c_str(), 0, stringsToCharPtrs(sandboxArgs).data(), &sandbox_errbuf)) { + writeFull( + STDERR_FILENO, + fmt("failed to configure sandbox: %s\n", sandbox_errbuf ? sandbox_errbuf : "(null)")); + _exit(1); + } + } + } +} + +#endif diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 027c4aa3b..8b1a2e0ff 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -92,8 +92,11 @@ MakeError(NotDeterministic, BuildError); * rather than incoming call edges that either should be removed, or * become (higher order) function parameters. */ -class DerivationBuilderImpl : public DerivationBuilder, DerivationBuilderParams +// FIXME: rename this to UnixDerivationBuilder or something like that. +class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilderParams { +protected: + Store & store; std::unique_ptr miscMethods; @@ -110,9 +113,7 @@ public: , derivationType(drv.type()) { } - LocalStore & getLocalStore(); - -private: +protected: /** * User selected for running the builder. @@ -140,32 +141,16 @@ private: */ Path tmpDirInSandbox; - /** - * Pipe for synchronising updates to the builder namespaces. - */ - Pipe userNamespaceSync; - - /** - * The mount namespace and user namespace of the builder, used to add additional - * paths to the sandbox as a result of recursive Nix calls. - */ - AutoCloseFD sandboxMountNamespace; - AutoCloseFD sandboxUserNamespace; - - /** - * On Linux, whether we're doing the build in its own user - * namespace. - */ - bool usingUserNamespace = true; - /** * Whether we're currently doing a chroot build. */ + // FIXME: remove bool useChroot = false; /** * The root of the chroot environment. */ + // FIXME: move Path chrootRootDir; /** @@ -219,9 +204,6 @@ private: */ OutputPathMap scratchOutputs; - uid_t sandboxUid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 1000 : 0) : buildUser->getUID(); } - gid_t sandboxGid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 100 : 0) : buildUser->getGID(); } - const static Path homeDir; /** @@ -260,7 +242,10 @@ private: /** * Whether we need to perform hash rewriting if there are valid output paths. */ - bool needsHashRewrite(); + virtual bool needsHashRewrite() + { + return true; + } public: @@ -270,6 +255,25 @@ public: std::variant, SingleDrvOutputs> unprepareBuild() override; +protected: + + /** + * Called by prepareBuild() to do any setup in the parent to + * prepare for a sandboxed build. + */ + virtual void prepareSandbox(); + + /** + * Open the slave side of the pseudoterminal and use it as stderr. + */ + void openSlave(); + + /** + * Called by prepareBuild() to start the child process for the + * build. Must set `pid`. The child must call runChild(). + */ + virtual void startChild(); + private: /** @@ -277,11 +281,15 @@ private: */ void initEnv(); +protected: + /** * Process messages send by the sandbox initialization. */ void processSandboxSetupMessages(); +private: + /** * Setup tmp dir location. */ @@ -305,6 +313,8 @@ private: void addDependency(const StorePath & path) override; +protected: + /** * Make a file owned by the builder. */ @@ -315,6 +325,28 @@ private: */ void runChild(); +private: + + /** + * Move the current process into the chroot, if any. Called early + * by runChild(). + */ + virtual void enterChroot() + { + } + + /** + * Change the current process's uid/gid to the build user, if + * any. Called by runChild(). + */ + virtual void setUser(); + + /** + * Execute the derivation builder process. Called by runChild() as + * its final step. Should not return unless there is an error. + */ + virtual void execBuilder(const Strings & args, const Strings & envStrs); + /** * Check that the derivation outputs all exist and register them * as valid. @@ -355,17 +387,6 @@ private: StorePath makeFallbackPath(OutputNameView outputName); }; -std::unique_ptr makeDerivationBuilder( - Store & store, - std::unique_ptr miscMethods, - DerivationBuilderParams params) -{ - return std::make_unique( - store, - std::move(miscMethods), - std::move(params)); -} - void handleDiffHook( uid_t uid, uid_t gid, const Path & tryA, const Path & tryB, @@ -403,18 +424,7 @@ void handleDiffHook( const Path DerivationBuilderImpl::homeDir = "/homeless-shelter"; -inline bool DerivationBuilderImpl::needsHashRewrite() -{ -#ifdef __linux__ - return !useChroot; -#else - /* Darwin requires hash rewriting even when sandboxing is enabled. */ - return true; -#endif -} - - -LocalStore & DerivationBuilderImpl::getLocalStore() +static LocalStore & getLocalStore(Store & store) { auto p = dynamic_cast(&store); assert(p); @@ -446,45 +456,6 @@ void DerivationBuilderImpl::killSandbox(bool getStats) bool DerivationBuilderImpl::prepareBuild() { - /* Are we doing a chroot build? */ - { - if (settings.sandboxMode == smEnabled) { - if (drvOptions.noChroot) - throw Error("derivation '%s' has '__noChroot' set, " - "but that's not allowed when 'sandbox' is 'true'", store.printStorePath(drvPath)); -#ifdef __APPLE__ - if (drvOptions.additionalSandboxProfile != "") - throw Error("derivation '%s' specifies a sandbox profile, " - "but this is only allowed when 'sandbox' is 'relaxed'", store.printStorePath(drvPath)); -#endif - useChroot = true; - } - else if (settings.sandboxMode == smDisabled) - useChroot = false; - else if (settings.sandboxMode == smRelaxed) - useChroot = derivationType.isSandboxed() && !drvOptions.noChroot; - } - - auto & localStore = getLocalStore(); - if (localStore.storeDir != localStore.config->realStoreDir.get()) { - #ifdef __linux__ - useChroot = true; - #else - throw Error("building using a diverted store is not supported on this platform"); - #endif - } - - #ifdef __linux__ - if (useChroot) { - if (!mountAndPidNamespacesSupported()) { - if (!settings.sandboxFallback) - throw Error("this system does not support the kernel namespaces that are required for sandboxing; use '--no-sandbox' to disable sandboxing"); - debug("auto-disabling sandboxing because the prerequisite namespaces are not available"); - useChroot = false; - } - } - #endif - if (useBuildUsers()) { if (!buildUser) buildUser = acquireUserLock(drvOptions.useUidRange(drv) ? 65536 : 1, useChroot); @@ -500,6 +471,7 @@ bool DerivationBuilderImpl::prepareBuild() std::variant, SingleDrvOutputs> DerivationBuilderImpl::unprepareBuild() { + // FIXME: get rid of this, rely on RAII. Finally releaseBuildUser([&](){ /* Release the build user at the end of this function. We don't do it right away because we don't want another build grabbing this @@ -507,9 +479,6 @@ std::variant, SingleDrvOutputs> Derivation buildUser.reset(); }); - sandboxMountNamespace = -1; - sandboxUserNamespace = -1; - /* Since we got an EOF on the logger pipe, the builder is presumed to have terminated. In fact, the builder could also have simply have closed its end of the pipe, so just to be sure, @@ -675,7 +644,7 @@ bool DerivationBuilderImpl::cleanupDecideWhetherDiskFull() so, we don't mark this build as a permanent failure. */ #if HAVE_STATVFS { - auto & localStore = getLocalStore(); + auto & localStore = getLocalStore(store); uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable struct statvfs st; if (statvfs(localStore.config->realStoreDir.get().c_str(), &st) == 0 && @@ -1028,118 +997,13 @@ void DerivationBuilderImpl::startBuilder() macOS 11+ has no /usr/lib/libSystem*.dylib */ pathsInChroot[i] = {i, true}; } - -#ifdef __linux__ - /* Create a temporary directory in which we set up the chroot - environment using bind-mounts. We put it in the Nix store - so that the build outputs can be moved efficiently from the - chroot to their final location. */ - auto chrootParentDir = store.Store::toRealPath(drvPath) + ".chroot"; - deletePath(chrootParentDir); - - /* Clean up the chroot directory automatically. */ - autoDelChroot = std::make_shared(chrootParentDir); - - printMsg(lvlChatty, "setting up chroot environment in '%1%'", chrootParentDir); - - if (mkdir(chrootParentDir.c_str(), 0700) == -1) - throw SysError("cannot create '%s'", chrootRootDir); - - chrootRootDir = chrootParentDir + "/root"; - - if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1) - throw SysError("cannot create '%1%'", chrootRootDir); - - if (buildUser && chown(chrootRootDir.c_str(), buildUser->getUIDCount() != 1 ? buildUser->getUID() : 0, buildUser->getGID()) == -1) - throw SysError("cannot change ownership of '%1%'", chrootRootDir); - - /* Create a writable /tmp in the chroot. Many builders need - this. (Of course they should really respect $TMPDIR - instead.) */ - Path chrootTmpDir = chrootRootDir + "/tmp"; - createDirs(chrootTmpDir); - chmod_(chrootTmpDir, 01777); - - /* Create a /etc/passwd with entries for the build user and the - nobody account. The latter is kind of a hack to support - Samba-in-QEMU. */ - createDirs(chrootRootDir + "/etc"); - if (drvOptions.useUidRange(drv)) - chownToBuilder(chrootRootDir + "/etc"); - - if (drvOptions.useUidRange(drv) && (!buildUser || buildUser->getUIDCount() < 65536)) - throw Error("feature 'uid-range' requires the setting '%s' to be enabled", settings.autoAllocateUids.name); - - /* Declare the build user's group so that programs get a consistent - view of the system (e.g., "id -gn"). */ - writeFile(chrootRootDir + "/etc/group", - fmt("root:x:0:\n" - "nixbld:!:%1%:\n" - "nogroup:x:65534:\n", sandboxGid())); - - /* Create /etc/hosts with localhost entry. */ - if (derivationType.isSandboxed()) - writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); - - /* Make the closure of the inputs available in the chroot, - rather than the whole Nix store. This prevents any access - to undeclared dependencies. Directories are bind-mounted, - while other inputs are hard-linked (since only directories - can be bind-mounted). !!! As an extra security - precaution, make the fake Nix store only writable by the - build user. */ - Path chrootStoreDir = chrootRootDir + store.storeDir; - createDirs(chrootStoreDir); - chmod_(chrootStoreDir, 01775); - - if (buildUser && chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) - throw SysError("cannot change ownership of '%1%'", chrootStoreDir); - - for (auto & i : inputPaths) { - auto p = store.printStorePath(i); - Path r = store.toRealPath(p); - pathsInChroot.insert_or_assign(p, r); - } - - /* If we're repairing, checking or rebuilding part of a - multiple-outputs derivation, it's possible that we're - rebuilding a path that is in settings.sandbox-paths - (typically the dependencies of /bin/sh). Throw them - out. */ - for (auto & i : drv.outputsAndOptPaths(store)) { - /* If the name isn't known a priori (i.e. floating - content-addressing derivation), the temporary location we use - should be fresh. Freshness means it is impossible that the path - is already in the sandbox, so we don't need to worry about - removing it. */ - if (i.second.second) - pathsInChroot.erase(store.printStorePath(*i.second.second)); - } - - if (cgroup) { - if (mkdir(cgroup->c_str(), 0755) != 0) - throw SysError("creating cgroup '%s'", *cgroup); - chownToBuilder(*cgroup); - chownToBuilder(*cgroup + "/cgroup.procs"); - chownToBuilder(*cgroup + "/cgroup.threads"); - //chownToBuilder(*cgroup + "/cgroup.subtree_control"); - } - -#else - if (drvOptions.useUidRange(drv)) - throw Error("feature 'uid-range' is not supported on this platform"); - #ifdef __APPLE__ - /* We don't really have any parent prep work to do (yet?) - All work happens in the child, instead. */ - #else - throw Error("sandboxing builds is not supported on this platform"); - #endif -#endif } else { if (drvOptions.useUidRange(drv)) throw Error("feature 'uid-range' is only supported in sandboxed builds"); } + prepareSandbox(); + if (needsHashRewrite() && pathExists(homeDir)) throw Error("home directory '%1%' exists; please remove it to assure purity of builds without sandboxing", homeDir); @@ -1218,194 +1082,52 @@ void DerivationBuilderImpl::startBuilder() if (unlockpt(builderOut.get())) throw SysError("unlocking pseudoterminal"); - /* Open the slave side of the pseudoterminal and use it as stderr. */ - auto openSlave = [&]() - { - AutoCloseFD builderOut = open(slaveName.c_str(), O_RDWR | O_NOCTTY); - if (!builderOut) - throw SysError("opening pseudoterminal slave"); - - // Put the pt into raw mode to prevent \n -> \r\n translation. - struct termios term; - if (tcgetattr(builderOut.get(), &term)) - throw SysError("getting pseudoterminal attributes"); - - cfmakeraw(&term); - - if (tcsetattr(builderOut.get(), TCSANOW, &term)) - throw SysError("putting pseudoterminal into raw mode"); - - if (dup2(builderOut.get(), STDERR_FILENO) == -1) - throw SysError("cannot pipe standard error into log file"); - }; - buildResult.startTime = time(0); - /* Fork a child to build the package. */ + /* Start a child process to build the derivation. */ + startChild(); -#ifdef __linux__ - if (useChroot) { - /* Set up private namespaces for the build: - - - The PID namespace causes the build to start as PID 1. - Processes outside of the chroot are not visible to those - on the inside, but processes inside the chroot are - visible from the outside (though with different PIDs). - - - The private mount namespace ensures that all the bind - mounts we do will only show up in this process and its - children, and will disappear automatically when we're - done. - - - The private network namespace ensures that the builder - cannot talk to the outside world (or vice versa). It - only has a private loopback interface. (Fixed-output - derivations are not run in a private network namespace - to allow functions like fetchurl to work.) - - - The IPC namespace prevents the builder from communicating - with outside processes using SysV IPC mechanisms (shared - memory, message queues, semaphores). It also ensures - that all IPC objects are destroyed when the builder - exits. - - - The UTS namespace ensures that builders see a hostname of - localhost rather than the actual hostname. - - We use a helper process to do the clone() to work around - clone() being broken in multi-threaded programs due to - at-fork handlers not being run. Note that we use - CLONE_PARENT to ensure that the real builder is parented to - us. - */ - - userNamespaceSync.create(); - - usingUserNamespace = userNamespacesSupported(); - - Pipe sendPid; - sendPid.create(); - - Pid helper = startProcess([&]() { - sendPid.readSide.close(); - - /* We need to open the slave early, before - CLONE_NEWUSER. Otherwise we get EPERM when running as - root. */ - openSlave(); - - try { - /* Drop additional groups here because we can't do it - after we've created the new user namespace. */ - if (setgroups(0, 0) == -1) { - if (errno != EPERM) - throw SysError("setgroups failed"); - if (settings.requireDropSupplementaryGroups) - throw Error("setgroups failed. Set the require-drop-supplementary-groups option to false to skip this step."); - } - - ProcessOptions options; - options.cloneFlags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | CLONE_PARENT | SIGCHLD; - if (derivationType.isSandboxed()) - options.cloneFlags |= CLONE_NEWNET; - if (usingUserNamespace) - options.cloneFlags |= CLONE_NEWUSER; - - pid_t child = startProcess([&]() { runChild(); }, options); - - writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); - _exit(0); - } catch (...) { - handleChildException(true); - _exit(1); - } - }); - - sendPid.writeSide.close(); - - if (helper.wait() != 0) { - processSandboxSetupMessages(); - // Only reached if the child process didn't send an exception. - throw Error("unable to start build process"); - } - - userNamespaceSync.readSide = -1; - - /* Close the write side to prevent runChild() from hanging - reading from this. */ - Finally cleanup([&]() { - userNamespaceSync.writeSide = -1; - }); - - auto ss = tokenizeString>(readLine(sendPid.readSide.get())); - assert(ss.size() == 1); - pid = string2Int(ss[0]).value(); - - if (usingUserNamespace) { - /* Set the UID/GID mapping of the builder's user namespace - such that the sandbox user maps to the build user, or to - the calling user (if build users are disabled). */ - uid_t hostUid = buildUser ? buildUser->getUID() : getuid(); - uid_t hostGid = buildUser ? buildUser->getGID() : getgid(); - uid_t nrIds = buildUser ? buildUser->getUIDCount() : 1; - - writeFile("/proc/" + std::to_string(pid) + "/uid_map", - fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); - - if (!buildUser || buildUser->getUIDCount() == 1) - writeFile("/proc/" + std::to_string(pid) + "/setgroups", "deny"); - - writeFile("/proc/" + std::to_string(pid) + "/gid_map", - fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); - } else { - debug("note: not using a user namespace"); - if (!buildUser) - throw Error("cannot perform a sandboxed build because user namespaces are not enabled; check /proc/sys/user/max_user_namespaces"); - } - - /* Now that we now the sandbox uid, we can write - /etc/passwd. */ - writeFile(chrootRootDir + "/etc/passwd", fmt( - "root:x:0:0:Nix build user:%3%:/noshell\n" - "nixbld:x:%1%:%2%:Nix build user:%3%:/noshell\n" - "nobody:x:65534:65534:Nobody:/:/noshell\n", - sandboxUid(), sandboxGid(), settings.sandboxBuildDir)); - - /* Save the mount- and user namespace of the child. We have to do this - *before* the child does a chroot. */ - sandboxMountNamespace = open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY); - if (sandboxMountNamespace.get() == -1) - throw SysError("getting sandbox mount namespace"); - - if (usingUserNamespace) { - sandboxUserNamespace = open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY); - if (sandboxUserNamespace.get() == -1) - throw SysError("getting sandbox user namespace"); - } - - /* Move the child into its own cgroup. */ - if (cgroup) - writeFile(*cgroup + "/cgroup.procs", fmt("%d", (pid_t) pid)); - - /* Signal the builder that we've updated its user namespace. */ - writeFull(userNamespaceSync.writeSide.get(), "1"); - - } else -#endif - { - pid = startProcess([&]() { - openSlave(); - runChild(); - }); - } - - /* parent */ pid.setSeparatePG(true); miscMethods->childStarted(builderOut.get()); processSandboxSetupMessages(); } +void DerivationBuilderImpl::prepareSandbox() +{ + if (drvOptions.useUidRange(drv)) + throw Error("feature 'uid-range' is not supported on this platform"); +} + +void DerivationBuilderImpl::openSlave() +{ + std::string slaveName = ptsname(builderOut.get()); + + AutoCloseFD builderOut = open(slaveName.c_str(), O_RDWR | O_NOCTTY); + if (!builderOut) + throw SysError("opening pseudoterminal slave"); + + // Put the pt into raw mode to prevent \n -> \r\n translation. + struct termios term; + if (tcgetattr(builderOut.get(), &term)) + throw SysError("getting pseudoterminal attributes"); + + cfmakeraw(&term); + + if (tcsetattr(builderOut.get(), TCSANOW, &term)) + throw SysError("putting pseudoterminal into raw mode"); + + if (dup2(builderOut.get(), STDERR_FILENO) == -1) + throw SysError("cannot pipe standard error into log file"); +} + +void DerivationBuilderImpl::startChild() +{ + pid = startProcess([&]() { + openSlave(); + runChild(); + }); +} void DerivationBuilderImpl::processSandboxSetupMessages() { @@ -1583,7 +1305,7 @@ void DerivationBuilderImpl::startDaemon() auto store = makeRestrictedStore( [&]{ - auto config = make_ref(*getLocalStore().config); + auto config = make_ref(*getLocalStore(this->store).config); config->pathInfoCacheSize = 0; config->stateDir = "/no-such-path"; config->logDir = "/no-such-path"; @@ -1683,51 +1405,6 @@ void DerivationBuilderImpl::addDependency(const StorePath & path) if (isAllowed(path)) return; addedPaths.insert(path); - - /* If we're doing a sandbox build, then we have to make the path - appear in the sandbox. */ - if (useChroot) { - - debug("materialising '%s' in the sandbox", store.printStorePath(path)); - - #ifdef __linux__ - - Path source = store.Store::toRealPath(path); - Path target = chrootRootDir + store.printStorePath(path); - - if (pathExists(target)) { - // There is a similar debug message in doBind, so only run it in this block to not have double messages. - debug("bind-mounting %s -> %s", target, source); - throw Error("store path '%s' already exists in the sandbox", store.printStorePath(path)); - } - - /* Bind-mount the path into the sandbox. This requires - entering its mount namespace, which is not possible - in multithreaded programs. So we do this in a - child process.*/ - Pid child(startProcess([&]() { - - if (usingUserNamespace && (setns(sandboxUserNamespace.get(), 0) == -1)) - throw SysError("entering sandbox user namespace"); - - if (setns(sandboxMountNamespace.get(), 0) == -1) - throw SysError("entering sandbox mount namespace"); - - doBind(source, target); - - _exit(0); - })); - - int status = child.wait(); - if (status != 0) - throw Error("could not add path '%s' to sandbox", store.printStorePath(path)); - - #else - throw Error("don't know how to make path '%s' (produced by a recursive Nix call) appear in the sandbox", - store.printStorePath(path)); - #endif - - } } void DerivationBuilderImpl::chownToBuilder(const Path & path) @@ -1843,8 +1520,6 @@ void DerivationBuilderImpl::runChild() if (buildUser) throw; } - bool setUser = true; - /* Make the contents of netrc and the CA certificate bundle available to builtin:fetchurl (which may run under a different uid and/or in a sandbox). */ @@ -1863,234 +1538,7 @@ void DerivationBuilderImpl::runChild() } catch (SystemError &) { } } -#ifdef __linux__ - if (useChroot) { - - userNamespaceSync.writeSide = -1; - - if (drainFD(userNamespaceSync.readSide.get()) != "1") - throw Error("user namespace initialisation failed"); - - userNamespaceSync.readSide = -1; - - if (derivationType.isSandboxed()) { - - /* Initialise the loopback interface. */ - AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP)); - if (!fd) throw SysError("cannot open IP socket"); - - struct ifreq ifr; - strcpy(ifr.ifr_name, "lo"); - ifr.ifr_flags = IFF_UP | IFF_LOOPBACK | IFF_RUNNING; - if (ioctl(fd.get(), SIOCSIFFLAGS, &ifr) == -1) - throw SysError("cannot set loopback interface flags"); - } - - /* Set the hostname etc. to fixed values. */ - char hostname[] = "localhost"; - if (sethostname(hostname, sizeof(hostname)) == -1) - throw SysError("cannot set host name"); - char domainname[] = "(none)"; // kernel default - if (setdomainname(domainname, sizeof(domainname)) == -1) - throw SysError("cannot set domain name"); - - /* Make all filesystems private. This is necessary - because subtrees may have been mounted as "shared" - (MS_SHARED). (Systemd does this, for instance.) Even - though we have a private mount namespace, mounting - filesystems on top of a shared subtree still propagates - outside of the namespace. Making a subtree private is - local to the namespace, though, so setting MS_PRIVATE - does not affect the outside world. */ - if (mount(0, "/", 0, MS_PRIVATE | MS_REC, 0) == -1) - throw SysError("unable to make '/' private"); - - /* Bind-mount chroot directory to itself, to treat it as a - different filesystem from /, as needed for pivot_root. */ - if (mount(chrootRootDir.c_str(), chrootRootDir.c_str(), 0, MS_BIND, 0) == -1) - throw SysError("unable to bind mount '%1%'", chrootRootDir); - - /* Bind-mount the sandbox's Nix store onto itself so that - we can mark it as a "shared" subtree, allowing bind - mounts made in *this* mount namespace to be propagated - into the child namespace created by the - unshare(CLONE_NEWNS) call below. - - Marking chrootRootDir as MS_SHARED causes pivot_root() - to fail with EINVAL. Don't know why. */ - Path chrootStoreDir = chrootRootDir + store.storeDir; - - if (mount(chrootStoreDir.c_str(), chrootStoreDir.c_str(), 0, MS_BIND, 0) == -1) - throw SysError("unable to bind mount the Nix store", chrootStoreDir); - - if (mount(0, chrootStoreDir.c_str(), 0, MS_SHARED, 0) == -1) - throw SysError("unable to make '%s' shared", chrootStoreDir); - - /* Set up a nearly empty /dev, unless the user asked to - bind-mount the host /dev. */ - Strings ss; - if (pathsInChroot.find("/dev") == pathsInChroot.end()) { - createDirs(chrootRootDir + "/dev/shm"); - createDirs(chrootRootDir + "/dev/pts"); - ss.push_back("/dev/full"); - if (store.config.systemFeatures.get().count("kvm") && pathExists("/dev/kvm")) - ss.push_back("/dev/kvm"); - ss.push_back("/dev/null"); - ss.push_back("/dev/random"); - ss.push_back("/dev/tty"); - ss.push_back("/dev/urandom"); - ss.push_back("/dev/zero"); - createSymlink("/proc/self/fd", chrootRootDir + "/dev/fd"); - createSymlink("/proc/self/fd/0", chrootRootDir + "/dev/stdin"); - createSymlink("/proc/self/fd/1", chrootRootDir + "/dev/stdout"); - createSymlink("/proc/self/fd/2", chrootRootDir + "/dev/stderr"); - } - - /* Fixed-output derivations typically need to access the - network, so give them access to /etc/resolv.conf and so - on. */ - if (!derivationType.isSandboxed()) { - // Only use nss functions to resolve hosts and - // services. Don’t use it for anything else that may - // be configured for this system. This limits the - // potential impurities introduced in fixed-outputs. - writeFile(chrootRootDir + "/etc/nsswitch.conf", "hosts: files dns\nservices: files\n"); - - /* N.B. it is realistic that these paths might not exist. It - happens when testing Nix building fixed-output derivations - within a pure derivation. */ - for (auto & path : { "/etc/resolv.conf", "/etc/services", "/etc/hosts" }) - if (pathExists(path)) - ss.push_back(path); - - if (settings.caFile != "") { - Path caFile = settings.caFile; - if (pathExists(caFile)) - pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", canonPath(caFile, true), true); - } - } - - for (auto & i : ss) { - // For backwards-compatibiliy, resolve all the symlinks in the - // chroot paths - auto canonicalPath = canonPath(i, true); - pathsInChroot.emplace(i, canonicalPath); - } - - /* Bind-mount all the directories from the "host" - filesystem that we want in the chroot - environment. */ - for (auto & i : pathsInChroot) { - if (i.second.source == "/proc") continue; // backwards compatibility - - #if HAVE_EMBEDDED_SANDBOX_SHELL - if (i.second.source == "__embedded_sandbox_shell__") { - static unsigned char sh[] = { - #include "embedded-sandbox-shell.gen.hh" - }; - auto dst = chrootRootDir + i.first; - createDirs(dirOf(dst)); - writeFile(dst, std::string_view((const char *) sh, sizeof(sh))); - chmod_(dst, 0555); - } else - #endif - doBind(i.second.source, chrootRootDir + i.first, i.second.optional); - } - - /* Bind a new instance of procfs on /proc. */ - createDirs(chrootRootDir + "/proc"); - if (mount("none", (chrootRootDir + "/proc").c_str(), "proc", 0, 0) == -1) - throw SysError("mounting /proc"); - - /* Mount sysfs on /sys. */ - if (buildUser && buildUser->getUIDCount() != 1) { - createDirs(chrootRootDir + "/sys"); - if (mount("none", (chrootRootDir + "/sys").c_str(), "sysfs", 0, 0) == -1) - throw SysError("mounting /sys"); - } - - /* Mount a new tmpfs on /dev/shm to ensure that whatever - the builder puts in /dev/shm is cleaned up automatically. */ - if (pathExists("/dev/shm") && mount("none", (chrootRootDir + "/dev/shm").c_str(), "tmpfs", 0, - fmt("size=%s", settings.sandboxShmSize).c_str()) == -1) - throw SysError("mounting /dev/shm"); - - /* Mount a new devpts on /dev/pts. Note that this - requires the kernel to be compiled with - CONFIG_DEVPTS_MULTIPLE_INSTANCES=y (which is the case - if /dev/ptx/ptmx exists). */ - if (pathExists("/dev/pts/ptmx") && - !pathExists(chrootRootDir + "/dev/ptmx") - && !pathsInChroot.count("/dev/pts")) - { - if (mount("none", (chrootRootDir + "/dev/pts").c_str(), "devpts", 0, "newinstance,mode=0620") == 0) - { - createSymlink("/dev/pts/ptmx", chrootRootDir + "/dev/ptmx"); - - /* Make sure /dev/pts/ptmx is world-writable. With some - Linux versions, it is created with permissions 0. */ - chmod_(chrootRootDir + "/dev/pts/ptmx", 0666); - } else { - if (errno != EINVAL) - throw SysError("mounting /dev/pts"); - doBind("/dev/pts", chrootRootDir + "/dev/pts"); - doBind("/dev/ptmx", chrootRootDir + "/dev/ptmx"); - } - } - - /* Make /etc unwritable */ - if (!drvOptions.useUidRange(drv)) - chmod_(chrootRootDir + "/etc", 0555); - - /* Unshare this mount namespace. This is necessary because - pivot_root() below changes the root of the mount - namespace. This means that the call to setns() in - addDependency() would hide the host's filesystem, - making it impossible to bind-mount paths from the host - Nix store into the sandbox. Therefore, we save the - pre-pivot_root namespace in - sandboxMountNamespace. Since we made /nix/store a - shared subtree above, this allows addDependency() to - make paths appear in the sandbox. */ - if (unshare(CLONE_NEWNS) == -1) - throw SysError("unsharing mount namespace"); - - /* Unshare the cgroup namespace. This means - /proc/self/cgroup will show the child's cgroup as '/' - rather than whatever it is in the parent. */ - if (cgroup && unshare(CLONE_NEWCGROUP) == -1) - throw SysError("unsharing cgroup namespace"); - - /* Do the chroot(). */ - if (chdir(chrootRootDir.c_str()) == -1) - throw SysError("cannot change directory to '%1%'", chrootRootDir); - - if (mkdir("real-root", 0500) == -1) - throw SysError("cannot create real-root directory"); - - if (pivot_root(".", "real-root") == -1) - throw SysError("cannot pivot old root directory onto '%1%'", (chrootRootDir + "/real-root")); - - if (chroot(".") == -1) - throw SysError("cannot change root directory to '%1%'", chrootRootDir); - - if (umount2("real-root", MNT_DETACH) == -1) - throw SysError("cannot unmount real root filesystem"); - - if (rmdir("real-root") == -1) - throw SysError("cannot remove real-root directory"); - - /* Switch to the sandbox uid/gid in the user namespace, - which corresponds to the build user or calling user in - the parent namespace. */ - if (setgid(sandboxGid()) == -1) - throw SysError("setgid failed"); - if (setuid(sandboxUid()) == -1) - throw SysError("setuid failed"); - - setUser = false; - } -#endif + enterChroot(); if (chdir(tmpDirInSandbox.c_str()) == -1) throw SysError("changing into '%1%'", tmpDir); @@ -2098,184 +1546,20 @@ void DerivationBuilderImpl::runChild() /* Close all other file descriptors. */ unix::closeExtraFDs(); -#ifdef __linux__ - linux::setPersonality(drv.platform); -#endif - /* Disable core dumps by default. */ struct rlimit limit = { 0, RLIM_INFINITY }; setrlimit(RLIMIT_CORE, &limit); // FIXME: set other limits to deterministic values? - /* Fill in the environment. */ - Strings envStrs; - for (auto & i : env) - envStrs.push_back(rewriteStrings(i.first + "=" + i.second, inputRewrites)); - - /* If we are running in `build-users' mode, then switch to the - user we allocated above. Make sure that we drop all root - privileges. Note that above we have closed all file - descriptors except std*, so that's safe. Also note that - setuid() when run as root sets the real, effective and - saved UIDs. */ - if (setUser && buildUser) { - /* Preserve supplementary groups of the build user, to allow - admins to specify groups such as "kvm". */ - auto gids = buildUser->getSupplementaryGIDs(); - if (setgroups(gids.size(), gids.data()) == -1) - throw SysError("cannot set supplementary groups of build user"); - - if (setgid(buildUser->getGID()) == -1 || - getgid() != buildUser->getGID() || - getegid() != buildUser->getGID()) - throw SysError("setgid failed"); - - if (setuid(buildUser->getUID()) == -1 || - getuid() != buildUser->getUID() || - geteuid() != buildUser->getUID()) - throw SysError("setuid failed"); - } - -#ifdef __APPLE__ - /* This has to appear before import statements. */ - std::string sandboxProfile = "(version 1)\n"; - - if (useChroot) { - - /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ - PathSet ancestry; - - /* We build the ancestry before adding all inputPaths to the store because we know they'll - all have the same parents (the store), and there might be lots of inputs. This isn't - particularly efficient... I doubt it'll be a bottleneck in practice */ - for (auto & i : pathsInChroot) { - Path cur = i.first; - while (cur.compare("/") != 0) { - cur = dirOf(cur); - ancestry.insert(cur); - } - } - - /* And we want the store in there regardless of how empty pathsInChroot. We include the innermost - path component this time, since it's typically /nix/store and we care about that. */ - Path cur = store.storeDir; - while (cur.compare("/") != 0) { - ancestry.insert(cur); - cur = dirOf(cur); - } - - /* Add all our input paths to the chroot */ - for (auto & i : inputPaths) { - auto p = store.printStorePath(i); - pathsInChroot[p] = p; - } - - /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */ - if (settings.darwinLogSandboxViolations) { - sandboxProfile += "(deny default)\n"; - } else { - sandboxProfile += "(deny default (with no-log))\n"; - } - - sandboxProfile += - #include "sandbox-defaults.sb" - ; - - if (!derivationType->isSandboxed()) - sandboxProfile += - #include "sandbox-network.sb" - ; - - /* Add the output paths we'll use at build-time to the chroot */ - sandboxProfile += "(allow file-read* file-write* process-exec\n"; - for (auto & [_, path] : scratchOutputs) - sandboxProfile += fmt("\t(subpath \"%s\")\n", store.printStorePath(path)); - - sandboxProfile += ")\n"; - - /* Our inputs (transitive dependencies and any impurities computed above) - - without file-write* allowed, access() incorrectly returns EPERM - */ - sandboxProfile += "(allow file-read* file-write* process-exec\n"; - - // We create multiple allow lists, to avoid exceeding a limit in the darwin sandbox interpreter. - // See https://github.com/NixOS/nix/issues/4119 - // We split our allow groups approximately at half the actual limit, 1 << 16 - const size_t breakpoint = sandboxProfile.length() + (1 << 14); - for (auto & i : pathsInChroot) { - - if (sandboxProfile.length() >= breakpoint) { - debug("Sandbox break: %d %d", sandboxProfile.length(), breakpoint); - sandboxProfile += ")\n(allow file-read* file-write* process-exec\n"; - } - - if (i.first != i.second.source) - throw Error( - "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin", - i.first, i.second.source); - - std::string path = i.first; - auto optSt = maybeLstat(path.c_str()); - if (!optSt) { - if (i.second.optional) - continue; - throw SysError("getting attributes of required path '%s", path); - } - if (S_ISDIR(optSt->st_mode)) - sandboxProfile += fmt("\t(subpath \"%s\")\n", path); - else - sandboxProfile += fmt("\t(literal \"%s\")\n", path); - } - sandboxProfile += ")\n"; - - /* Allow file-read* on full directory hierarchy to self. Allows realpath() */ - sandboxProfile += "(allow file-read*\n"; - for (auto & i : ancestry) { - sandboxProfile += fmt("\t(literal \"%s\")\n", i); - } - sandboxProfile += ")\n"; - - sandboxProfile += drvOptions.additionalSandboxProfile; - } else - sandboxProfile += - #include "sandbox-minimal.sb" - ; - - debug("Generated sandbox profile:"); - debug(sandboxProfile); - - /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms - to find temporary directories, so we want to open up a broader place for them to put their files, if needed. */ - Path globalTmpDir = canonPath(defaultTempDir(), true); - - /* They don't like trailing slashes on subpath directives */ - while (!globalTmpDir.empty() && globalTmpDir.back() == '/') - globalTmpDir.pop_back(); - - if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") { - Strings sandboxArgs; - sandboxArgs.push_back("_GLOBAL_TMP_DIR"); - sandboxArgs.push_back(globalTmpDir); - if (drvOptions.allowLocalNetworking) { - sandboxArgs.push_back("_ALLOW_LOCAL_NETWORKING"); - sandboxArgs.push_back("1"); - } - char * sandbox_errbuf = nullptr; - if (sandbox_init_with_parameters(sandboxProfile.c_str(), 0, stringsToCharPtrs(sandboxArgs).data(), &sandbox_errbuf)) { - writeFull(STDERR_FILENO, fmt("failed to configure sandbox: %s\n", sandbox_errbuf ? sandbox_errbuf : "(null)")); - _exit(1); - } - } -#endif + setUser(); /* Indicate that we managed to set up the build environment. */ writeFull(STDERR_FILENO, std::string("\2\n")); sendException = false; - /* Execute the program. This should not return. */ + /* If this is a builtin builder, call it now. This should not return. */ if (drv.isBuiltin()) { try { logger = makeJSONLogger(getStandardError()); @@ -2297,7 +1581,7 @@ void DerivationBuilderImpl::runChild() } } - // Now builder is not builtin + /* It's not a builtin builder, so execute the program. */ Strings args; args.push_back(std::string(baseNameOf(drv.builder))); @@ -2305,31 +1589,11 @@ void DerivationBuilderImpl::runChild() for (auto & i : drv.args) args.push_back(rewriteStrings(i, inputRewrites)); -#ifdef __APPLE__ - posix_spawnattr_t attrp; + Strings envStrs; + for (auto & i : env) + envStrs.push_back(rewriteStrings(i.first + "=" + i.second, inputRewrites)); - if (posix_spawnattr_init(&attrp)) - throw SysError("failed to initialize builder"); - - if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) - throw SysError("failed to initialize builder"); - - if (drv.platform == "aarch64-darwin") { - // Unset kern.curproc_arch_affinity so we can escape Rosetta - int affinity = 0; - sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); - - cpu_type_t cpu = CPU_TYPE_ARM64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } else if (drv.platform == "x86_64-darwin") { - cpu_type_t cpu = CPU_TYPE_X86_64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } - - posix_spawn(NULL, drv.builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); -#else - execve(drv.builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); -#endif + execBuilder(args, envStrs); throw SysError("executing '%1%'", drv.builder); @@ -2339,6 +1603,37 @@ void DerivationBuilderImpl::runChild() } } +void DerivationBuilderImpl::setUser() +{ + /* If we are running in `build-users' mode, then switch to the + user we allocated above. Make sure that we drop all root + privileges. Note that above we have closed all file + descriptors except std*, so that's safe. Also note that + setuid() when run as root sets the real, effective and + saved UIDs. */ + if (buildUser) { + /* Preserve supplementary groups of the build user, to allow + admins to specify groups such as "kvm". */ + auto gids = buildUser->getSupplementaryGIDs(); + if (setgroups(gids.size(), gids.data()) == -1) + throw SysError("cannot set supplementary groups of build user"); + + if (setgid(buildUser->getGID()) == -1 || + getgid() != buildUser->getGID() || + getegid() != buildUser->getGID()) + throw SysError("setgid failed"); + + if (setuid(buildUser->getUID()) == -1 || + getuid() != buildUser->getUID() || + geteuid() != buildUser->getUID()) + throw SysError("setuid failed"); + } +} + +void DerivationBuilderImpl::execBuilder(const Strings & args, const Strings & envStrs) +{ + execve(drv.builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); +} SingleDrvOutputs DerivationBuilderImpl::registerOutputs() { @@ -2777,7 +2072,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() } } - auto & localStore = getLocalStore(); + auto & localStore = getLocalStore(store); if (buildMode == bmCheck) { @@ -2854,7 +2149,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() paths referenced by each of them. If there are cycles in the outputs, this will fail. */ { - auto & localStore = getLocalStore(); + auto & localStore = getLocalStore(store); ValidPathInfos infos2; for (auto & [outputName, newInfo] : infos) { @@ -3075,5 +2370,70 @@ StorePath DerivationBuilderImpl::makeFallbackPath(const StorePath & path) Hash(HashAlgorithm::SHA256), path.name()); } +// FIXME: do this properly +#include "linux-derivation-builder.cc" +#include "darwin-derivation-builder.cc" + +std::unique_ptr makeDerivationBuilder( + Store & store, + std::unique_ptr miscMethods, + DerivationBuilderParams params) +{ + bool useSandbox = false; + + /* Are we doing a sandboxed build? */ + { + if (settings.sandboxMode == smEnabled) { + if (params.drvOptions.noChroot) + throw Error("derivation '%s' has '__noChroot' set, " + "but that's not allowed when 'sandbox' is 'true'", store.printStorePath(params.drvPath)); +#ifdef __APPLE__ + if (drvOptions.additionalSandboxProfile != "") + throw Error("derivation '%s' specifies a sandbox profile, " + "but this is only allowed when 'sandbox' is 'relaxed'", store.printStorePath(params.drvPath)); +#endif + useSandbox = true; + } + else if (settings.sandboxMode == smDisabled) + useSandbox = false; + else if (settings.sandboxMode == smRelaxed) + // FIXME: cache derivationType + useSandbox = params.drv.type().isSandboxed() && !params.drvOptions.noChroot; + } + + auto & localStore = getLocalStore(store); + if (localStore.storeDir != localStore.config->realStoreDir.get()) { + #ifdef __linux__ + useSandbox = true; + #else + throw Error("building using a diverted store is not supported on this platform"); + #endif + } + + #ifdef __linux__ + if (useSandbox) { + if (!mountAndPidNamespacesSupported()) { + if (!settings.sandboxFallback) + throw Error("this system does not support the kernel namespaces that are required for sandboxing; use '--no-sandbox' to disable sandboxing"); + debug("auto-disabling sandboxing because the prerequisite namespaces are not available"); + useSandbox = false; + } + } + + if (useSandbox) + return std::make_unique( + store, + std::move(miscMethods), + std::move(params)); + #endif + + if (useSandbox) + throw Error("sandboxing builds is not supported on this platform"); + + return std::make_unique( + store, + std::move(miscMethods), + std::move(params)); +} } diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc new file mode 100644 index 000000000..59c554119 --- /dev/null +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -0,0 +1,568 @@ +#ifdef __linux__ + +struct LinuxDerivationBuilder : DerivationBuilderImpl +{ + /** + * Pipe for synchronising updates to the builder namespaces. + */ + Pipe userNamespaceSync; + + /** + * The mount namespace and user namespace of the builder, used to add additional + * paths to the sandbox as a result of recursive Nix calls. + */ + AutoCloseFD sandboxMountNamespace; + AutoCloseFD sandboxUserNamespace; + + /** + * On Linux, whether we're doing the build in its own user + * namespace. + */ + bool usingUserNamespace = true; + + LinuxDerivationBuilder( + Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) + : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) + { + useChroot = true; + } + + uid_t sandboxUid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 1000 : 0) : buildUser->getUID(); } + gid_t sandboxGid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 100 : 0) : buildUser->getGID(); } + + bool needsHashRewrite() override + { + return false; + } + + void prepareSandbox() override + { + /* Create a temporary directory in which we set up the chroot + environment using bind-mounts. We put it in the Nix store + so that the build outputs can be moved efficiently from the + chroot to their final location. */ + auto chrootParentDir = store.Store::toRealPath(drvPath) + ".chroot"; + deletePath(chrootParentDir); + + /* Clean up the chroot directory automatically. */ + autoDelChroot = std::make_shared(chrootParentDir); + + printMsg(lvlChatty, "setting up chroot environment in '%1%'", chrootParentDir); + + if (mkdir(chrootParentDir.c_str(), 0700) == -1) + throw SysError("cannot create '%s'", chrootRootDir); + + chrootRootDir = chrootParentDir + "/root"; + + if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1) + throw SysError("cannot create '%1%'", chrootRootDir); + + if (buildUser && chown(chrootRootDir.c_str(), buildUser->getUIDCount() != 1 ? buildUser->getUID() : 0, buildUser->getGID()) == -1) + throw SysError("cannot change ownership of '%1%'", chrootRootDir); + + /* Create a writable /tmp in the chroot. Many builders need + this. (Of course they should really respect $TMPDIR + instead.) */ + Path chrootTmpDir = chrootRootDir + "/tmp"; + createDirs(chrootTmpDir); + chmod_(chrootTmpDir, 01777); + + /* Create a /etc/passwd with entries for the build user and the + nobody account. The latter is kind of a hack to support + Samba-in-QEMU. */ + createDirs(chrootRootDir + "/etc"); + if (drvOptions.useUidRange(drv)) + chownToBuilder(chrootRootDir + "/etc"); + + if (drvOptions.useUidRange(drv) && (!buildUser || buildUser->getUIDCount() < 65536)) + throw Error("feature 'uid-range' requires the setting '%s' to be enabled", settings.autoAllocateUids.name); + + /* Declare the build user's group so that programs get a consistent + view of the system (e.g., "id -gn"). */ + writeFile(chrootRootDir + "/etc/group", + fmt("root:x:0:\n" + "nixbld:!:%1%:\n" + "nogroup:x:65534:\n", sandboxGid())); + + /* Create /etc/hosts with localhost entry. */ + if (derivationType.isSandboxed()) + writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); + + /* Make the closure of the inputs available in the chroot, + rather than the whole Nix store. This prevents any access + to undeclared dependencies. Directories are bind-mounted, + while other inputs are hard-linked (since only directories + can be bind-mounted). !!! As an extra security + precaution, make the fake Nix store only writable by the + build user. */ + Path chrootStoreDir = chrootRootDir + store.storeDir; + createDirs(chrootStoreDir); + chmod_(chrootStoreDir, 01775); + + if (buildUser && chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) + throw SysError("cannot change ownership of '%1%'", chrootStoreDir); + + for (auto & i : inputPaths) { + auto p = store.printStorePath(i); + Path r = store.toRealPath(p); + pathsInChroot.insert_or_assign(p, r); + } + + /* If we're repairing, checking or rebuilding part of a + multiple-outputs derivation, it's possible that we're + rebuilding a path that is in settings.sandbox-paths + (typically the dependencies of /bin/sh). Throw them + out. */ + for (auto & i : drv.outputsAndOptPaths(store)) { + /* If the name isn't known a priori (i.e. floating + content-addressing derivation), the temporary location we use + should be fresh. Freshness means it is impossible that the path + is already in the sandbox, so we don't need to worry about + removing it. */ + if (i.second.second) + pathsInChroot.erase(store.printStorePath(*i.second.second)); + } + + if (cgroup) { + if (mkdir(cgroup->c_str(), 0755) != 0) + throw SysError("creating cgroup '%s'", *cgroup); + chownToBuilder(*cgroup); + chownToBuilder(*cgroup + "/cgroup.procs"); + chownToBuilder(*cgroup + "/cgroup.threads"); + //chownToBuilder(*cgroup + "/cgroup.subtree_control"); + } + } + + void startChild() override + { + /* Set up private namespaces for the build: + + - The PID namespace causes the build to start as PID 1. + Processes outside of the chroot are not visible to those + on the inside, but processes inside the chroot are + visible from the outside (though with different PIDs). + + - The private mount namespace ensures that all the bind + mounts we do will only show up in this process and its + children, and will disappear automatically when we're + done. + + - The private network namespace ensures that the builder + cannot talk to the outside world (or vice versa). It + only has a private loopback interface. (Fixed-output + derivations are not run in a private network namespace + to allow functions like fetchurl to work.) + + - The IPC namespace prevents the builder from communicating + with outside processes using SysV IPC mechanisms (shared + memory, message queues, semaphores). It also ensures + that all IPC objects are destroyed when the builder + exits. + + - The UTS namespace ensures that builders see a hostname of + localhost rather than the actual hostname. + + We use a helper process to do the clone() to work around + clone() being broken in multi-threaded programs due to + at-fork handlers not being run. Note that we use + CLONE_PARENT to ensure that the real builder is parented to + us. + */ + + userNamespaceSync.create(); + + usingUserNamespace = userNamespacesSupported(); + + Pipe sendPid; + sendPid.create(); + + Pid helper = startProcess([&]() { + sendPid.readSide.close(); + + /* We need to open the slave early, before + CLONE_NEWUSER. Otherwise we get EPERM when running as + root. */ + openSlave(); + + try { + /* Drop additional groups here because we can't do it + after we've created the new user namespace. */ + if (setgroups(0, 0) == -1) { + if (errno != EPERM) + throw SysError("setgroups failed"); + if (settings.requireDropSupplementaryGroups) + throw Error("setgroups failed. Set the require-drop-supplementary-groups option to false to skip this step."); + } + + ProcessOptions options; + options.cloneFlags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | CLONE_PARENT | SIGCHLD; + if (derivationType.isSandboxed()) + options.cloneFlags |= CLONE_NEWNET; + if (usingUserNamespace) + options.cloneFlags |= CLONE_NEWUSER; + + pid_t child = startProcess([&]() { runChild(); }, options); + + writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); + _exit(0); + } catch (...) { + handleChildException(true); + _exit(1); + } + }); + + sendPid.writeSide.close(); + + if (helper.wait() != 0) { + processSandboxSetupMessages(); + // Only reached if the child process didn't send an exception. + throw Error("unable to start build process"); + } + + userNamespaceSync.readSide = -1; + + /* Close the write side to prevent runChild() from hanging + reading from this. */ + Finally cleanup([&]() { + userNamespaceSync.writeSide = -1; + }); + + auto ss = tokenizeString>(readLine(sendPid.readSide.get())); + assert(ss.size() == 1); + pid = string2Int(ss[0]).value(); + + if (usingUserNamespace) { + /* Set the UID/GID mapping of the builder's user namespace + such that the sandbox user maps to the build user, or to + the calling user (if build users are disabled). */ + uid_t hostUid = buildUser ? buildUser->getUID() : getuid(); + uid_t hostGid = buildUser ? buildUser->getGID() : getgid(); + uid_t nrIds = buildUser ? buildUser->getUIDCount() : 1; + + writeFile("/proc/" + std::to_string(pid) + "/uid_map", + fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); + + if (!buildUser || buildUser->getUIDCount() == 1) + writeFile("/proc/" + std::to_string(pid) + "/setgroups", "deny"); + + writeFile("/proc/" + std::to_string(pid) + "/gid_map", + fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); + } else { + debug("note: not using a user namespace"); + if (!buildUser) + throw Error("cannot perform a sandboxed build because user namespaces are not enabled; check /proc/sys/user/max_user_namespaces"); + } + + /* Now that we now the sandbox uid, we can write + /etc/passwd. */ + writeFile(chrootRootDir + "/etc/passwd", fmt( + "root:x:0:0:Nix build user:%3%:/noshell\n" + "nixbld:x:%1%:%2%:Nix build user:%3%:/noshell\n" + "nobody:x:65534:65534:Nobody:/:/noshell\n", + sandboxUid(), sandboxGid(), settings.sandboxBuildDir)); + + /* Save the mount- and user namespace of the child. We have to do this + *before* the child does a chroot. */ + sandboxMountNamespace = open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY); + if (sandboxMountNamespace.get() == -1) + throw SysError("getting sandbox mount namespace"); + + if (usingUserNamespace) { + sandboxUserNamespace = open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY); + if (sandboxUserNamespace.get() == -1) + throw SysError("getting sandbox user namespace"); + } + + /* Move the child into its own cgroup. */ + if (cgroup) + writeFile(*cgroup + "/cgroup.procs", fmt("%d", (pid_t) pid)); + + /* Signal the builder that we've updated its user namespace. */ + writeFull(userNamespaceSync.writeSide.get(), "1"); + } + + void enterChroot() override + { + userNamespaceSync.writeSide = -1; + + if (drainFD(userNamespaceSync.readSide.get()) != "1") + throw Error("user namespace initialisation failed"); + + userNamespaceSync.readSide = -1; + + if (derivationType.isSandboxed()) { + + /* Initialise the loopback interface. */ + AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP)); + if (!fd) + throw SysError("cannot open IP socket"); + + struct ifreq ifr; + strcpy(ifr.ifr_name, "lo"); + ifr.ifr_flags = IFF_UP | IFF_LOOPBACK | IFF_RUNNING; + if (ioctl(fd.get(), SIOCSIFFLAGS, &ifr) == -1) + throw SysError("cannot set loopback interface flags"); + } + + /* Set the hostname etc. to fixed values. */ + char hostname[] = "localhost"; + if (sethostname(hostname, sizeof(hostname)) == -1) + throw SysError("cannot set host name"); + char domainname[] = "(none)"; // kernel default + if (setdomainname(domainname, sizeof(domainname)) == -1) + throw SysError("cannot set domain name"); + + /* Make all filesystems private. This is necessary + because subtrees may have been mounted as "shared" + (MS_SHARED). (Systemd does this, for instance.) Even + though we have a private mount namespace, mounting + filesystems on top of a shared subtree still propagates + outside of the namespace. Making a subtree private is + local to the namespace, though, so setting MS_PRIVATE + does not affect the outside world. */ + if (mount(0, "/", 0, MS_PRIVATE | MS_REC, 0) == -1) + throw SysError("unable to make '/' private"); + + /* Bind-mount chroot directory to itself, to treat it as a + different filesystem from /, as needed for pivot_root. */ + if (mount(chrootRootDir.c_str(), chrootRootDir.c_str(), 0, MS_BIND, 0) == -1) + throw SysError("unable to bind mount '%1%'", chrootRootDir); + + /* Bind-mount the sandbox's Nix store onto itself so that + we can mark it as a "shared" subtree, allowing bind + mounts made in *this* mount namespace to be propagated + into the child namespace created by the + unshare(CLONE_NEWNS) call below. + + Marking chrootRootDir as MS_SHARED causes pivot_root() + to fail with EINVAL. Don't know why. */ + Path chrootStoreDir = chrootRootDir + store.storeDir; + + if (mount(chrootStoreDir.c_str(), chrootStoreDir.c_str(), 0, MS_BIND, 0) == -1) + throw SysError("unable to bind mount the Nix store", chrootStoreDir); + + if (mount(0, chrootStoreDir.c_str(), 0, MS_SHARED, 0) == -1) + throw SysError("unable to make '%s' shared", chrootStoreDir); + + /* Set up a nearly empty /dev, unless the user asked to + bind-mount the host /dev. */ + Strings ss; + if (pathsInChroot.find("/dev") == pathsInChroot.end()) { + createDirs(chrootRootDir + "/dev/shm"); + createDirs(chrootRootDir + "/dev/pts"); + ss.push_back("/dev/full"); + if (store.config.systemFeatures.get().count("kvm") && pathExists("/dev/kvm")) + ss.push_back("/dev/kvm"); + ss.push_back("/dev/null"); + ss.push_back("/dev/random"); + ss.push_back("/dev/tty"); + ss.push_back("/dev/urandom"); + ss.push_back("/dev/zero"); + createSymlink("/proc/self/fd", chrootRootDir + "/dev/fd"); + createSymlink("/proc/self/fd/0", chrootRootDir + "/dev/stdin"); + createSymlink("/proc/self/fd/1", chrootRootDir + "/dev/stdout"); + createSymlink("/proc/self/fd/2", chrootRootDir + "/dev/stderr"); + } + + /* Fixed-output derivations typically need to access the + network, so give them access to /etc/resolv.conf and so + on. */ + if (!derivationType.isSandboxed()) { + // Only use nss functions to resolve hosts and + // services. Don’t use it for anything else that may + // be configured for this system. This limits the + // potential impurities introduced in fixed-outputs. + writeFile(chrootRootDir + "/etc/nsswitch.conf", "hosts: files dns\nservices: files\n"); + + /* N.B. it is realistic that these paths might not exist. It + happens when testing Nix building fixed-output derivations + within a pure derivation. */ + for (auto & path : {"/etc/resolv.conf", "/etc/services", "/etc/hosts"}) + if (pathExists(path)) + ss.push_back(path); + + if (settings.caFile != "") { + Path caFile = settings.caFile; + if (pathExists(caFile)) + pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", canonPath(caFile, true), true); + } + } + + for (auto & i : ss) { + // For backwards-compatibiliy, resolve all the symlinks in the + // chroot paths + auto canonicalPath = canonPath(i, true); + pathsInChroot.emplace(i, canonicalPath); + } + + /* Bind-mount all the directories from the "host" + filesystem that we want in the chroot + environment. */ + for (auto & i : pathsInChroot) { + if (i.second.source == "/proc") + continue; // backwards compatibility + +# if HAVE_EMBEDDED_SANDBOX_SHELL + if (i.second.source == "__embedded_sandbox_shell__") { + static unsigned char sh[] = { +# include "embedded-sandbox-shell.gen.hh" + }; + auto dst = chrootRootDir + i.first; + createDirs(dirOf(dst)); + writeFile(dst, std::string_view((const char *) sh, sizeof(sh))); + chmod_(dst, 0555); + } else +# endif + doBind(i.second.source, chrootRootDir + i.first, i.second.optional); + } + + /* Bind a new instance of procfs on /proc. */ + createDirs(chrootRootDir + "/proc"); + if (mount("none", (chrootRootDir + "/proc").c_str(), "proc", 0, 0) == -1) + throw SysError("mounting /proc"); + + /* Mount sysfs on /sys. */ + if (buildUser && buildUser->getUIDCount() != 1) { + createDirs(chrootRootDir + "/sys"); + if (mount("none", (chrootRootDir + "/sys").c_str(), "sysfs", 0, 0) == -1) + throw SysError("mounting /sys"); + } + + /* Mount a new tmpfs on /dev/shm to ensure that whatever + the builder puts in /dev/shm is cleaned up automatically. */ + if (pathExists("/dev/shm") + && mount( + "none", + (chrootRootDir + "/dev/shm").c_str(), + "tmpfs", + 0, + fmt("size=%s", settings.sandboxShmSize).c_str()) + == -1) + throw SysError("mounting /dev/shm"); + + /* Mount a new devpts on /dev/pts. Note that this + requires the kernel to be compiled with + CONFIG_DEVPTS_MULTIPLE_INSTANCES=y (which is the case + if /dev/ptx/ptmx exists). */ + if (pathExists("/dev/pts/ptmx") && !pathExists(chrootRootDir + "/dev/ptmx") + && !pathsInChroot.count("/dev/pts")) { + if (mount("none", (chrootRootDir + "/dev/pts").c_str(), "devpts", 0, "newinstance,mode=0620") == 0) { + createSymlink("/dev/pts/ptmx", chrootRootDir + "/dev/ptmx"); + + /* Make sure /dev/pts/ptmx is world-writable. With some + Linux versions, it is created with permissions 0. */ + chmod_(chrootRootDir + "/dev/pts/ptmx", 0666); + } else { + if (errno != EINVAL) + throw SysError("mounting /dev/pts"); + doBind("/dev/pts", chrootRootDir + "/dev/pts"); + doBind("/dev/ptmx", chrootRootDir + "/dev/ptmx"); + } + } + + /* Make /etc unwritable */ + if (!drvOptions.useUidRange(drv)) + chmod_(chrootRootDir + "/etc", 0555); + + /* Unshare this mount namespace. This is necessary because + pivot_root() below changes the root of the mount + namespace. This means that the call to setns() in + addDependency() would hide the host's filesystem, + making it impossible to bind-mount paths from the host + Nix store into the sandbox. Therefore, we save the + pre-pivot_root namespace in + sandboxMountNamespace. Since we made /nix/store a + shared subtree above, this allows addDependency() to + make paths appear in the sandbox. */ + if (unshare(CLONE_NEWNS) == -1) + throw SysError("unsharing mount namespace"); + + /* Unshare the cgroup namespace. This means + /proc/self/cgroup will show the child's cgroup as '/' + rather than whatever it is in the parent. */ + if (cgroup && unshare(CLONE_NEWCGROUP) == -1) + throw SysError("unsharing cgroup namespace"); + + /* Do the chroot(). */ + if (chdir(chrootRootDir.c_str()) == -1) + throw SysError("cannot change directory to '%1%'", chrootRootDir); + + if (mkdir("real-root", 0500) == -1) + throw SysError("cannot create real-root directory"); + + if (pivot_root(".", "real-root") == -1) + throw SysError("cannot pivot old root directory onto '%1%'", (chrootRootDir + "/real-root")); + + if (chroot(".") == -1) + throw SysError("cannot change root directory to '%1%'", chrootRootDir); + + if (umount2("real-root", MNT_DETACH) == -1) + throw SysError("cannot unmount real root filesystem"); + + if (rmdir("real-root") == -1) + throw SysError("cannot remove real-root directory"); + + // FIXME: move to LinuxDerivationBuilder + linux::setPersonality(drv.platform); + } + + void setUser() override + { + /* Switch to the sandbox uid/gid in the user namespace, + which corresponds to the build user or calling user in + the parent namespace. */ + if (setgid(sandboxGid()) == -1) + throw SysError("setgid failed"); + if (setuid(sandboxUid()) == -1) + throw SysError("setuid failed"); + } + + std::variant, SingleDrvOutputs> unprepareBuild() override + { + sandboxMountNamespace = -1; + sandboxUserNamespace = -1; + + return DerivationBuilderImpl::unprepareBuild(); + } + + void addDependency(const StorePath & path) override + { + if (isAllowed(path)) + return; + + addedPaths.insert(path); + + debug("materialising '%s' in the sandbox", store.printStorePath(path)); + + Path source = store.Store::toRealPath(path); + Path target = chrootRootDir + store.printStorePath(path); + + if (pathExists(target)) { + // There is a similar debug message in doBind, so only run it in this block to not have double messages. + debug("bind-mounting %s -> %s", target, source); + throw Error("store path '%s' already exists in the sandbox", store.printStorePath(path)); + } + + /* Bind-mount the path into the sandbox. This requires + entering its mount namespace, which is not possible + in multithreaded programs. So we do this in a + child process.*/ + Pid child(startProcess([&]() { + if (usingUserNamespace && (setns(sandboxUserNamespace.get(), 0) == -1)) + throw SysError("entering sandbox user namespace"); + + if (setns(sandboxMountNamespace.get(), 0) == -1) + throw SysError("entering sandbox mount namespace"); + + doBind(source, target); + + _exit(0); + })); + + int status = child.wait(); + if (status != 0) + throw Error("could not add path '%s' to sandbox", store.printStorePath(path)); + } +}; + +#endif From 2d5d3e44ddf843ec57b03d425a6617af95a9b34b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 20:30:36 +0200 Subject: [PATCH 05/28] Move pathsInChroot --- .../unix/build/darwin-derivation-builder.cc | 9 +- src/libstore/unix/build/derivation-builder.cc | 255 +++++++++--------- .../unix/build/linux-derivation-builder.cc | 15 +- 3 files changed, 151 insertions(+), 128 deletions(-) diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc index 3366403a7..cc2364390 100644 --- a/src/libstore/unix/build/darwin-derivation-builder.cc +++ b/src/libstore/unix/build/darwin-derivation-builder.cc @@ -2,6 +2,8 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl { + PathsInChroot pathsInChroot; + DarwinDerivationBuilder( Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) @@ -9,6 +11,11 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl useChroot = true; } + void prepareSandbox() override + { + pathsInChroot = getPathsInSandbox(); + } + void execBuilder(const Strings & args, const Strings & envStrs) override { posix_spawnattr_t attrp; @@ -69,7 +76,7 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl /* Add all our input paths to the chroot */ for (auto & i : inputPaths) { auto p = store.printStorePath(i); - pathsInChroot[p] = p; + pathsInChroot.insert_or_assign(p, p); } /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 8b1a2e0ff..a2bca3a59 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -156,6 +156,7 @@ protected: /** * RAII object to delete the chroot directory. */ + // FIXME: move std::shared_ptr autoDelChroot; /** @@ -176,7 +177,6 @@ protected: { } }; typedef std::map PathsInChroot; // maps target path to source path - PathsInChroot pathsInChroot; typedef std::map Environment; Environment env; @@ -257,6 +257,17 @@ public: protected: + /** + * Return the paths that should be made available in the sandbox. + * This includes: + * + * * The paths specified by the `sandbox-paths` setting, and their closure in the Nix store. + * * The contents of the `__impureHostDeps` derivation attribute, if the sandbox is in relaxed mode. + * * The paths returned by the `pre-build-hook`. + * * The paths in the input closure of the derivation. + */ + PathsInChroot getPathsInSandbox(); + /** * Called by prepareBuild() to do any setup in the parent to * prepare for a sandboxed build. @@ -918,131 +929,11 @@ void DerivationBuilderImpl::startBuilder() } } - if (useChroot) { - - /* Allow a user-configurable set of directories from the - host file system. */ - pathsInChroot.clear(); - - for (auto i : settings.sandboxPaths.get()) { - if (i.empty()) continue; - bool optional = false; - if (i[i.size() - 1] == '?') { - optional = true; - i.pop_back(); - } - size_t p = i.find('='); - - std::string inside, outside; - if (p == std::string::npos) { - inside = i; - outside = i; - } else { - inside = i.substr(0, p); - outside = i.substr(p + 1); - } - - if (!optional && !maybeLstat(outside)) { - throw SysError("path '%s' is configured as part of the `sandbox-paths` option, but is inaccessible", outside); - } - - pathsInChroot[inside] = {outside, optional}; - } - if (hasPrefix(store.storeDir, tmpDirInSandbox)) - { - throw Error("`sandbox-build-dir` must not contain the storeDir"); - } - pathsInChroot[tmpDirInSandbox] = tmpDir; - - /* Add the closure of store paths to the chroot. */ - StorePathSet closure; - for (auto & i : pathsInChroot) - try { - if (store.isInStore(i.second.source)) - store.computeFSClosure(store.toStorePath(i.second.source).first, closure); - } catch (InvalidPath & e) { - } catch (Error & e) { - e.addTrace({}, "while processing 'sandbox-paths'"); - throw; - } - for (auto & i : closure) { - auto p = store.printStorePath(i); - pathsInChroot.insert_or_assign(p, p); - } - - PathSet allowedPaths = settings.allowedImpureHostPrefixes; - - /* This works like the above, except on a per-derivation level */ - auto impurePaths = drvOptions.impureHostDeps; - - for (auto & i : impurePaths) { - bool found = false; - /* Note: we're not resolving symlinks here to prevent - giving a non-root user info about inaccessible - files. */ - Path canonI = canonPath(i); - /* If only we had a trie to do this more efficiently :) luckily, these are generally going to be pretty small */ - for (auto & a : allowedPaths) { - Path canonA = canonPath(a); - if (isDirOrInDir(canonI, canonA)) { - found = true; - break; - } - } - if (!found) - throw Error("derivation '%s' requested impure path '%s', but it was not in allowed-impure-host-deps", - store.printStorePath(drvPath), i); - - /* Allow files in drvOptions.impureHostDeps to be missing; e.g. - macOS 11+ has no /usr/lib/libSystem*.dylib */ - pathsInChroot[i] = {i, true}; - } - } else { - if (drvOptions.useUidRange(drv)) - throw Error("feature 'uid-range' is only supported in sandboxed builds"); - } - prepareSandbox(); if (needsHashRewrite() && pathExists(homeDir)) throw Error("home directory '%1%' exists; please remove it to assure purity of builds without sandboxing", homeDir); - if (useChroot && settings.preBuildHook != "") { - printMsg(lvlChatty, "executing pre-build hook '%1%'", settings.preBuildHook); - auto args = useChroot ? Strings({store.printStorePath(drvPath), chrootRootDir}) : - Strings({ store.printStorePath(drvPath) }); - enum BuildHookState { - stBegin, - stExtraChrootDirs - }; - auto state = stBegin; - auto lines = runProgram(settings.preBuildHook, false, args); - auto lastPos = std::string::size_type{0}; - for (auto nlPos = lines.find('\n'); nlPos != std::string::npos; - nlPos = lines.find('\n', lastPos)) - { - auto line = lines.substr(lastPos, nlPos - lastPos); - lastPos = nlPos + 1; - if (state == stBegin) { - if (line == "extra-sandbox-paths" || line == "extra-chroot-dirs") { - state = stExtraChrootDirs; - } else { - throw Error("unknown pre-build hook command '%1%'", line); - } - } else if (state == stExtraChrootDirs) { - if (line == "") { - state = stBegin; - } else { - auto p = line.find('='); - if (p == std::string::npos) - pathsInChroot[line] = line; - else - pathsInChroot[line.substr(0, p)] = line.substr(p + 1); - } - } - } - } - /* Fire up a Nix daemon to process recursive Nix calls from the builder. */ if (drvOptions.getRequiredSystemFeatures(drv).count("recursive-nix")) @@ -1093,6 +984,125 @@ void DerivationBuilderImpl::startBuilder() processSandboxSetupMessages(); } +DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox() +{ + PathsInChroot pathsInChroot; + + /* Allow a user-configurable set of directories from the + host file system. */ + for (auto i : settings.sandboxPaths.get()) { + if (i.empty()) continue; + bool optional = false; + if (i[i.size() - 1] == '?') { + optional = true; + i.pop_back(); + } + size_t p = i.find('='); + + std::string inside, outside; + if (p == std::string::npos) { + inside = i; + outside = i; + } else { + inside = i.substr(0, p); + outside = i.substr(p + 1); + } + + if (!optional && !maybeLstat(outside)) { + throw SysError("path '%s' is configured as part of the `sandbox-paths` option, but is inaccessible", outside); + } + + pathsInChroot[inside] = {outside, optional}; + } + if (hasPrefix(store.storeDir, tmpDirInSandbox)) + { + throw Error("`sandbox-build-dir` must not contain the storeDir"); + } + pathsInChroot[tmpDirInSandbox] = tmpDir; + + /* Add the closure of store paths to the chroot. */ + StorePathSet closure; + for (auto & i : pathsInChroot) + try { + if (store.isInStore(i.second.source)) + store.computeFSClosure(store.toStorePath(i.second.source).first, closure); + } catch (InvalidPath & e) { + } catch (Error & e) { + e.addTrace({}, "while processing 'sandbox-paths'"); + throw; + } + for (auto & i : closure) { + auto p = store.printStorePath(i); + pathsInChroot.insert_or_assign(p, p); + } + + PathSet allowedPaths = settings.allowedImpureHostPrefixes; + + /* This works like the above, except on a per-derivation level */ + auto impurePaths = drvOptions.impureHostDeps; + + for (auto & i : impurePaths) { + bool found = false; + /* Note: we're not resolving symlinks here to prevent + giving a non-root user info about inaccessible + files. */ + Path canonI = canonPath(i); + /* If only we had a trie to do this more efficiently :) luckily, these are generally going to be pretty small */ + for (auto & a : allowedPaths) { + Path canonA = canonPath(a); + if (isDirOrInDir(canonI, canonA)) { + found = true; + break; + } + } + if (!found) + throw Error("derivation '%s' requested impure path '%s', but it was not in allowed-impure-host-deps", + store.printStorePath(drvPath), i); + + /* Allow files in drvOptions.impureHostDeps to be missing; e.g. + macOS 11+ has no /usr/lib/libSystem*.dylib */ + pathsInChroot[i] = {i, true}; + } + + if (settings.preBuildHook != "") { + printMsg(lvlChatty, "executing pre-build hook '%1%'", settings.preBuildHook); + auto args = useChroot ? Strings({store.printStorePath(drvPath), chrootRootDir}) : + Strings({ store.printStorePath(drvPath) }); + enum BuildHookState { + stBegin, + stExtraChrootDirs + }; + auto state = stBegin; + auto lines = runProgram(settings.preBuildHook, false, args); + auto lastPos = std::string::size_type{0}; + for (auto nlPos = lines.find('\n'); nlPos != std::string::npos; + nlPos = lines.find('\n', lastPos)) + { + auto line = lines.substr(lastPos, nlPos - lastPos); + lastPos = nlPos + 1; + if (state == stBegin) { + if (line == "extra-sandbox-paths" || line == "extra-chroot-dirs") { + state = stExtraChrootDirs; + } else { + throw Error("unknown pre-build hook command '%1%'", line); + } + } else if (state == stExtraChrootDirs) { + if (line == "") { + state = stBegin; + } else { + auto p = line.find('='); + if (p == std::string::npos) + pathsInChroot[line] = line; + else + pathsInChroot[line.substr(0, p)] = line.substr(p + 1); + } + } + } + } + + return pathsInChroot; +} + void DerivationBuilderImpl::prepareSandbox() { if (drvOptions.useUidRange(drv)) @@ -2430,6 +2440,9 @@ std::unique_ptr makeDerivationBuilder( if (useSandbox) throw Error("sandboxing builds is not supported on this platform"); + if (params.drvOptions.useUidRange(params.drv)) + throw Error("feature 'uid-range' is only supported in sandboxed builds"); + return std::make_unique( store, std::move(miscMethods), diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 59c554119..1e33056ea 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -20,6 +20,8 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl */ bool usingUserNamespace = true; + PathsInChroot pathsInChroot; + LinuxDerivationBuilder( Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) @@ -102,12 +104,6 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl if (buildUser && chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) throw SysError("cannot change ownership of '%1%'", chrootStoreDir); - for (auto & i : inputPaths) { - auto p = store.printStorePath(i); - Path r = store.toRealPath(p); - pathsInChroot.insert_or_assign(p, r); - } - /* If we're repairing, checking or rebuilding part of a multiple-outputs derivation, it's possible that we're rebuilding a path that is in settings.sandbox-paths @@ -131,6 +127,13 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl chownToBuilder(*cgroup + "/cgroup.threads"); //chownToBuilder(*cgroup + "/cgroup.subtree_control"); } + + pathsInChroot = getPathsInSandbox(); + + for (auto & i : inputPaths) { + auto p = store.printStorePath(i); + pathsInChroot.insert_or_assign(p, store.toRealPath(p)); + } } void startChild() override From 305a9680e419bcf1858c4c886bf959dd1841e72b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 21:25:56 +0200 Subject: [PATCH 06/28] Eliminate useChroot --- .../unix/build/darwin-derivation-builder.cc | 12 ++ src/libstore/unix/build/derivation-builder.cc | 131 +++++++----------- .../unix/build/linux-derivation-builder.cc | 113 ++++++++++++--- 3 files changed, 155 insertions(+), 101 deletions(-) diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc index cc2364390..2ba54ad97 100644 --- a/src/libstore/unix/build/darwin-derivation-builder.cc +++ b/src/libstore/unix/build/darwin-derivation-builder.cc @@ -1,5 +1,15 @@ #ifdef __APPLE__ +# include +# include +# include + +/* This definition is undocumented but depended upon by all major browsers. */ +extern "C" int +sandbox_init_with_parameters(const char * profile, uint64_t flags, const char * const parameters[], char ** errorbuf); + +namespace nix { + struct DarwinDerivationBuilder : DerivationBuilderImpl { PathsInChroot pathsInChroot; @@ -185,4 +195,6 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl } } +} + #endif diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index a2bca3a59..65e4799e7 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -59,15 +59,6 @@ # include "nix/store/personality.hh" #endif -#ifdef __APPLE__ -# include -# include -# include - -/* This definition is undocumented but depended upon by all major browsers. */ -extern "C" int sandbox_init_with_parameters(const char *profile, uint64_t flags, const char *const parameters[], char **errorbuf); -#endif - #include #include #include @@ -123,6 +114,7 @@ protected: /** * The cgroup of the builder, if any. */ + // FIXME: move std::optional cgroup; /** @@ -141,18 +133,6 @@ protected: */ Path tmpDirInSandbox; - /** - * Whether we're currently doing a chroot build. - */ - // FIXME: remove - bool useChroot = false; - - /** - * The root of the chroot environment. - */ - // FIXME: move - Path chrootRootDir; - /** * RAII object to delete the chroot directory. */ @@ -257,6 +237,14 @@ public: protected: + /** + * Acquire a build user lock. Return nullptr if no lock is available. + */ + virtual std::unique_ptr getBuildUser() + { + return acquireUserLock(1, false); + } + /** * Return the paths that should be made available in the sandbox. * This includes: @@ -268,12 +256,28 @@ protected: */ PathsInChroot getPathsInSandbox(); + virtual void setBuildTmpDir() + { + tmpDir = topTmpDir; + tmpDirInSandbox = topTmpDir; + } + /** * Called by prepareBuild() to do any setup in the parent to * prepare for a sandboxed build. */ virtual void prepareSandbox(); + virtual Strings getPreBuildHookArgs() + { + return Strings({store.printStorePath(drvPath)}); + } + + virtual Path realPathInSandbox(const Path & p) + { + return store.toRealPath(p); + } + /** * Open the slave side of the pseudoterminal and use it as stderr. */ @@ -377,9 +381,13 @@ public: void killSandbox(bool getStats) override; +protected: + + virtual void cleanupBuild(); + private: - bool cleanupDecideWhetherDiskFull(); + bool decideWhetherDiskFull(); /** * Create alternative path calculated from but distinct from the @@ -469,11 +477,10 @@ bool DerivationBuilderImpl::prepareBuild() { if (useBuildUsers()) { if (!buildUser) - buildUser = acquireUserLock(drvOptions.useUidRange(drv) ? 65536 : 1, useChroot); + buildUser = getBuildUser(); - if (!buildUser) { + if (!buildUser) return false; - } } return true; @@ -535,7 +542,9 @@ std::variant, SingleDrvOutputs> Derivation /* Check the exit status. */ if (!statusOk(status)) { - diskFull |= cleanupDecideWhetherDiskFull(); + diskFull |= decideWhetherDiskFull(); + + cleanupBuild(); auto msg = fmt( "Cannot build '%s'.\n" @@ -589,6 +598,10 @@ std::variant, SingleDrvOutputs> Derivation } } +void DerivationBuilderImpl::cleanupBuild() +{ + deleteTmpDir(false); +} static void chmod_(const Path & path, mode_t mode) { @@ -641,10 +654,7 @@ static void replaceValidPath(const Path & storePath, const Path & tmpPath) deletePath(oldPath); } - - - -bool DerivationBuilderImpl::cleanupDecideWhetherDiskFull() +bool DerivationBuilderImpl::decideWhetherDiskFull() { bool diskFull = false; @@ -667,19 +677,6 @@ bool DerivationBuilderImpl::cleanupDecideWhetherDiskFull() } #endif - deleteTmpDir(false); - - /* Move paths out of the chroot for easier debugging of - build failures. */ - if (useChroot && buildMode == bmNormal) - for (auto & [_, status] : initialOutputs) { - if (!status.known) continue; - if (buildMode != bmCheck && status.known->isValid()) continue; - auto p = store.toRealPath(status.known->path); - if (pathExists(chrootRootDir + p)) - std::filesystem::rename((chrootRootDir + p), p); - } - return diskFull; } @@ -834,23 +831,9 @@ void DerivationBuilderImpl::startBuilder() /* Create a temporary directory where the build will take place. */ topTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), false, false, 0700); -#ifdef __APPLE__ - if (false) { -#else - if (useChroot) { -#endif - /* If sandboxing is enabled, put the actual TMPDIR underneath - an inaccessible root-owned directory, to prevent outside - access. - - On macOS, we don't use an actual chroot, so this isn't - possible. Any mitigation along these lines would have to be - done directly in the sandbox profile. */ - tmpDir = topTmpDir + "/build"; - createDir(tmpDir, 0700); - } else { - tmpDir = topTmpDir; - } + setBuildTmpDir(); + assert(!tmpDir.empty()); + assert(!tmpDirInSandbox.empty()); chownToBuilder(tmpDir); for (auto & [outputName, status] : initialOutputs) { @@ -1066,14 +1049,12 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox() if (settings.preBuildHook != "") { printMsg(lvlChatty, "executing pre-build hook '%1%'", settings.preBuildHook); - auto args = useChroot ? Strings({store.printStorePath(drvPath), chrootRootDir}) : - Strings({ store.printStorePath(drvPath) }); enum BuildHookState { stBegin, stExtraChrootDirs }; auto state = stBegin; - auto lines = runProgram(settings.preBuildHook, false, args); + auto lines = runProgram(settings.preBuildHook, false, getPreBuildHookArgs()); auto lastPos = std::string::size_type{0}; for (auto nlPos = lines.find('\n'); nlPos != std::string::npos; nlPos = lines.find('\n', lastPos)) @@ -1170,14 +1151,6 @@ void DerivationBuilderImpl::processSandboxSetupMessages() void DerivationBuilderImpl::initTmpDir() { - /* In a sandbox, for determinism, always use the same temporary - directory. */ -#ifdef __linux__ - tmpDirInSandbox = useChroot ? settings.sandboxBuildDir : tmpDir; -#else - tmpDirInSandbox = tmpDir; -#endif - /* In non-structured mode, set all bindings either directory in the environment or via a file, as specified by `DerivationOptions::passAsFile`. */ @@ -1666,14 +1639,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() for (auto & i : scratchOutputs) referenceablePaths.insert(i.second); for (auto & p : addedPaths) referenceablePaths.insert(p); - /* FIXME `needsHashRewrite` should probably be removed and we get to the - real reason why we aren't using the chroot dir */ - auto toRealPathChroot = [&](const Path & p) -> Path { - return useChroot && !needsHashRewrite() - ? chrootRootDir + p - : store.toRealPath(p); - }; - /* Check whether the output paths were created, and make all output paths read-only. Then get the references of each output (that we might need to register), so we can topologically sort them. For the ones @@ -1690,7 +1655,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() throw BuildError( "builder for '%s' has no scratch output for '%s'", store.printStorePath(drvPath), outputName); - auto actualPath = toRealPathChroot(store.printStorePath(*scratchOutput)); + auto actualPath = realPathInSandbox(store.printStorePath(*scratchOutput)); outputsToSort.insert(outputName); @@ -1799,7 +1764,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() auto output = get(drv.outputs, outputName); auto scratchPath = get(scratchOutputs, outputName); assert(output && scratchPath); - auto actualPath = toRealPathChroot(store.printStorePath(*scratchPath)); + auto actualPath = realPathInSandbox(store.printStorePath(*scratchPath)); auto finish = [&](StorePath finalStorePath) { /* Store the final path */ @@ -2380,10 +2345,14 @@ StorePath DerivationBuilderImpl::makeFallbackPath(const StorePath & path) Hash(HashAlgorithm::SHA256), path.name()); } +} + // FIXME: do this properly #include "linux-derivation-builder.cc" #include "darwin-derivation-builder.cc" +namespace nix { + std::unique_ptr makeDerivationBuilder( Store & store, std::unique_ptr miscMethods, diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 1e33056ea..c52831166 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -1,5 +1,7 @@ #ifdef __linux__ +namespace nix { + struct LinuxDerivationBuilder : DerivationBuilderImpl { /** @@ -20,23 +22,56 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl */ bool usingUserNamespace = true; + /** + * The root of the chroot environment. + */ + Path chrootRootDir; + PathsInChroot pathsInChroot; LinuxDerivationBuilder( Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) { - useChroot = true; } - uid_t sandboxUid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 1000 : 0) : buildUser->getUID(); } - gid_t sandboxGid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 100 : 0) : buildUser->getGID(); } + uid_t sandboxUid() + { + return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 1000 : 0) : buildUser->getUID(); + } + + gid_t sandboxGid() + { + return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 100 : 0) : buildUser->getGID(); + } bool needsHashRewrite() override { return false; } + std::unique_ptr getBuildUser() override + { + return acquireUserLock(drvOptions.useUidRange(drv) ? 65536 : 1, true); + } + + void setBuildTmpDir() override + { + /* If sandboxing is enabled, put the actual TMPDIR underneath + an inaccessible root-owned directory, to prevent outside + access. + + On macOS, we don't use an actual chroot, so this isn't + possible. Any mitigation along these lines would have to be + done directly in the sandbox profile. */ + tmpDir = topTmpDir + "/build"; + createDir(tmpDir, 0700); + + /* In a sandbox, for determinism, always use the same temporary + directory. */ + tmpDirInSandbox = settings.sandboxBuildDir; + } + void prepareSandbox() override { /* Create a temporary directory in which we set up the chroot @@ -59,7 +94,10 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1) throw SysError("cannot create '%1%'", chrootRootDir); - if (buildUser && chown(chrootRootDir.c_str(), buildUser->getUIDCount() != 1 ? buildUser->getUID() : 0, buildUser->getGID()) == -1) + if (buildUser + && chown( + chrootRootDir.c_str(), buildUser->getUIDCount() != 1 ? buildUser->getUID() : 0, buildUser->getGID()) + == -1) throw SysError("cannot change ownership of '%1%'", chrootRootDir); /* Create a writable /tmp in the chroot. Many builders need @@ -81,10 +119,12 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl /* Declare the build user's group so that programs get a consistent view of the system (e.g., "id -gn"). */ - writeFile(chrootRootDir + "/etc/group", + writeFile( + chrootRootDir + "/etc/group", fmt("root:x:0:\n" "nixbld:!:%1%:\n" - "nogroup:x:65534:\n", sandboxGid())); + "nogroup:x:65534:\n", + sandboxGid())); /* Create /etc/hosts with localhost entry. */ if (derivationType.isSandboxed()) @@ -125,7 +165,7 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl chownToBuilder(*cgroup); chownToBuilder(*cgroup + "/cgroup.procs"); chownToBuilder(*cgroup + "/cgroup.threads"); - //chownToBuilder(*cgroup + "/cgroup.subtree_control"); + // chownToBuilder(*cgroup + "/cgroup.subtree_control"); } pathsInChroot = getPathsInSandbox(); @@ -136,6 +176,18 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl } } + Strings getPreBuildHookArgs() override + { + assert(!chrootRootDir.empty()); + return Strings({store.printStorePath(drvPath), chrootRootDir}); + } + + Path realPathInSandbox(const Path & p) override + { + // FIXME: why the needsHashRewrite() conditional? + return !needsHashRewrite() ? chrootRootDir + p : store.toRealPath(p); + } + void startChild() override { /* Set up private namespaces for the build: @@ -194,7 +246,8 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl if (errno != EPERM) throw SysError("setgroups failed"); if (settings.requireDropSupplementaryGroups) - throw Error("setgroups failed. Set the require-drop-supplementary-groups option to false to skip this step."); + throw Error( + "setgroups failed. Set the require-drop-supplementary-groups option to false to skip this step."); } ProcessOptions options; @@ -226,9 +279,7 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl /* Close the write side to prevent runChild() from hanging reading from this. */ - Finally cleanup([&]() { - userNamespaceSync.writeSide = -1; - }); + Finally cleanup([&]() { userNamespaceSync.writeSide = -1; }); auto ss = tokenizeString>(readLine(sendPid.readSide.get())); assert(ss.size() == 1); @@ -242,30 +293,32 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl uid_t hostGid = buildUser ? buildUser->getGID() : getgid(); uid_t nrIds = buildUser ? buildUser->getUIDCount() : 1; - writeFile("/proc/" + std::to_string(pid) + "/uid_map", - fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); + writeFile("/proc/" + std::to_string(pid) + "/uid_map", fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); if (!buildUser || buildUser->getUIDCount() == 1) writeFile("/proc/" + std::to_string(pid) + "/setgroups", "deny"); - writeFile("/proc/" + std::to_string(pid) + "/gid_map", - fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); + writeFile("/proc/" + std::to_string(pid) + "/gid_map", fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); } else { debug("note: not using a user namespace"); if (!buildUser) - throw Error("cannot perform a sandboxed build because user namespaces are not enabled; check /proc/sys/user/max_user_namespaces"); + throw Error( + "cannot perform a sandboxed build because user namespaces are not enabled; check /proc/sys/user/max_user_namespaces"); } /* Now that we now the sandbox uid, we can write /etc/passwd. */ - writeFile(chrootRootDir + "/etc/passwd", fmt( - "root:x:0:0:Nix build user:%3%:/noshell\n" + writeFile( + chrootRootDir + "/etc/passwd", + fmt("root:x:0:0:Nix build user:%3%:/noshell\n" "nixbld:x:%1%:%2%:Nix build user:%3%:/noshell\n" "nobody:x:65534:65534:Nobody:/:/noshell\n", - sandboxUid(), sandboxGid(), settings.sandboxBuildDir)); + sandboxUid(), + sandboxGid(), + settings.sandboxBuildDir)); /* Save the mount- and user namespace of the child. We have to do this - *before* the child does a chroot. */ + *before* the child does a chroot. */ sandboxMountNamespace = open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY); if (sandboxMountNamespace.get() == -1) throw SysError("getting sandbox mount namespace"); @@ -528,6 +581,24 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl return DerivationBuilderImpl::unprepareBuild(); } + void cleanupBuild() override + { + DerivationBuilderImpl::cleanupBuild(); + + /* Move paths out of the chroot for easier debugging of + build failures. */ + if (buildMode == bmNormal) + for (auto & [_, status] : initialOutputs) { + if (!status.known) + continue; + if (buildMode != bmCheck && status.known->isValid()) + continue; + auto p = store.toRealPath(status.known->path); + if (pathExists(chrootRootDir + p)) + std::filesystem::rename((chrootRootDir + p), p); + } + } + void addDependency(const StorePath & path) override { if (isAllowed(path)) @@ -568,4 +639,6 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl } }; +} + #endif From 1acdb9168d6295599f2974467608c7d3f635f004 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 21:36:13 +0200 Subject: [PATCH 07/28] Move doBind() --- src/libstore/unix/build/derivation-builder.cc | 47 ----------------- .../unix/build/linux-derivation-builder.cc | 50 +++++++++++++++++++ 2 files changed, 50 insertions(+), 47 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 65e4799e7..ee9c89390 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -39,24 +39,13 @@ # include #endif -/* Includes required for chroot support. */ #ifdef __linux__ # include "linux/fchmodat2-compat.hh" -# include -# include -# include -# include -# include -# include -# include # include -# include "nix/util/namespaces.hh" # if HAVE_SECCOMP # include # endif -# define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, put_old)) # include "nix/util/cgroup.hh" -# include "nix/store/personality.hh" #endif #include @@ -680,42 +669,6 @@ bool DerivationBuilderImpl::decideWhetherDiskFull() return diskFull; } - -#ifdef __linux__ -static void doBind(const Path & source, const Path & target, bool optional = false) { - debug("bind mounting '%1%' to '%2%'", source, target); - - auto bindMount = [&]() { - if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1) - throw SysError("bind mount from '%1%' to '%2%' failed", source, target); - }; - - auto maybeSt = maybeLstat(source); - if (!maybeSt) { - if (optional) - return; - else - throw SysError("getting attributes of path '%1%'", source); - } - auto st = *maybeSt; - - if (S_ISDIR(st.st_mode)) { - createDirs(target); - bindMount(); - } else if (S_ISLNK(st.st_mode)) { - // Symlinks can (apparently) not be bind-mounted, so just copy it - createDirs(dirOf(target)); - copyFile( - std::filesystem::path(source), - std::filesystem::path(target), false); - } else { - createDirs(dirOf(target)); - writeFile(target, ""); - bindMount(); - } -}; -#endif - /** * Rethrow the current exception as a subclass of `Error`. */ diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index c52831166..7e2aed1c8 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -1,7 +1,57 @@ #ifdef __linux__ +# ifdef __linux__ +# include +# include +# include +# include +# include +# include +# include +# include +# include "nix/util/namespaces.hh" +# if HAVE_SECCOMP +# include +# endif +# define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, put_old)) +# include "nix/util/cgroup.hh" +# include "nix/store/personality.hh" +# endif + namespace nix { +static void doBind(const Path & source, const Path & target, bool optional = false) +{ + debug("bind mounting '%1%' to '%2%'", source, target); + + auto bindMount = [&]() { + if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1) + throw SysError("bind mount from '%1%' to '%2%' failed", source, target); + }; + + auto maybeSt = maybeLstat(source); + if (!maybeSt) { + if (optional) + return; + else + throw SysError("getting attributes of path '%1%'", source); + } + auto st = *maybeSt; + + if (S_ISDIR(st.st_mode)) { + createDirs(target); + bindMount(); + } else if (S_ISLNK(st.st_mode)) { + // Symlinks can (apparently) not be bind-mounted, so just copy it + createDirs(dirOf(target)); + copyFile(std::filesystem::path(source), std::filesystem::path(target), false); + } else { + createDirs(dirOf(target)); + writeFile(target, ""); + bindMount(); + } +} + struct LinuxDerivationBuilder : DerivationBuilderImpl { /** From 5d96e55e91bd2ccf586f0757952b52f2b19f7186 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 21:44:51 +0200 Subject: [PATCH 08/28] Move seccomp code --- src/libstore/unix/build/derivation-builder.cc | 100 ------------- .../unix/build/linux-derivation-builder.cc | 133 +++++++++++++++--- 2 files changed, 117 insertions(+), 116 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index ee9c89390..b40511111 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -40,11 +40,6 @@ #endif #ifdef __linux__ -# include "linux/fchmodat2-compat.hh" -# include -# if HAVE_SECCOMP -# include -# endif # include "nix/util/cgroup.hh" #endif @@ -1350,95 +1345,6 @@ void DerivationBuilderImpl::chownToBuilder(const Path & path) throw SysError("cannot change ownership of '%1%'", path); } - -void setupSeccomp() -{ -#ifdef __linux__ - if (!settings.filterSyscalls) return; -#if HAVE_SECCOMP - scmp_filter_ctx ctx; - - if (!(ctx = seccomp_init(SCMP_ACT_ALLOW))) - throw SysError("unable to initialize seccomp mode 2"); - - Finally cleanup([&]() { - seccomp_release(ctx); - }); - - constexpr std::string_view nativeSystem = NIX_LOCAL_SYSTEM; - - if (nativeSystem == "x86_64-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_X86) != 0) - throw SysError("unable to add 32-bit seccomp architecture"); - - if (nativeSystem == "x86_64-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_X32) != 0) - throw SysError("unable to add X32 seccomp architecture"); - - if (nativeSystem == "aarch64-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_ARM) != 0) - printError("unable to add ARM seccomp architecture; this may result in spurious build failures if running 32-bit ARM processes"); - - if (nativeSystem == "mips64-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_MIPS) != 0) - printError("unable to add mips seccomp architecture"); - - if (nativeSystem == "mips64-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_MIPS64N32) != 0) - printError("unable to add mips64-*abin32 seccomp architecture"); - - if (nativeSystem == "mips64el-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_MIPSEL) != 0) - printError("unable to add mipsel seccomp architecture"); - - if (nativeSystem == "mips64el-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_MIPSEL64N32) != 0) - printError("unable to add mips64el-*abin32 seccomp architecture"); - - /* Prevent builders from creating setuid/setgid binaries. */ - for (int perm : { S_ISUID, S_ISGID }) { - if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), SCMP_SYS(chmod), 1, - SCMP_A1(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) != 0) - throw SysError("unable to add seccomp rule"); - - if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), SCMP_SYS(fchmod), 1, - SCMP_A1(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) != 0) - throw SysError("unable to add seccomp rule"); - - if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), SCMP_SYS(fchmodat), 1, - SCMP_A2(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) != 0) - throw SysError("unable to add seccomp rule"); - - if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), NIX_SYSCALL_FCHMODAT2, 1, - SCMP_A2(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) != 0) - throw SysError("unable to add seccomp rule"); - } - - /* Prevent builders from using EAs or ACLs. Not all filesystems - support these, and they're not allowed in the Nix store because - they're not representable in the NAR serialisation. */ - if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(getxattr), 0) != 0 || - seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(lgetxattr), 0) != 0 || - seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(fgetxattr), 0) != 0 || - seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(setxattr), 0) != 0 || - seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(lsetxattr), 0) != 0 || - seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(fsetxattr), 0) != 0) - throw SysError("unable to add seccomp rule"); - - if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_NNP, settings.allowNewPrivileges ? 0 : 1) != 0) - throw SysError("unable to set 'no new privileges' seccomp attribute"); - - if (seccomp_load(ctx) != 0) - throw SysError("unable to load seccomp BPF program"); -#else - throw Error( - "seccomp is not supported on this platform; " - "you can bypass this error by setting the option 'filter-syscalls' to false, but note that untrusted builds can then create setuid binaries!"); -#endif -#endif -} - - void DerivationBuilderImpl::runChild() { /* Warning: in the child we should absolutely not make any SQLite @@ -1450,12 +1356,6 @@ void DerivationBuilderImpl::runChild() commonChildInit(); - try { - setupSeccomp(); - } catch (...) { - if (buildUser) throw; - } - /* Make the contents of netrc and the CA certificate bundle available to builtin:fetchurl (which may run under a different uid and/or in a sandbox). */ diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 7e2aed1c8..bfda1e33a 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -1,25 +1,123 @@ #ifdef __linux__ -# ifdef __linux__ -# include -# include -# include -# include -# include -# include -# include -# include -# include "nix/util/namespaces.hh" -# if HAVE_SECCOMP -# include -# endif -# define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, put_old)) -# include "nix/util/cgroup.hh" -# include "nix/store/personality.hh" +# include "linux/fchmodat2-compat.hh" +# include +# include +# include +# include +# include +# include +# include +# include +# include "nix/util/namespaces.hh" +# if HAVE_SECCOMP +# include # endif +# define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, put_old)) +# include "nix/util/cgroup.hh" +# include "nix/store/personality.hh" namespace nix { +static void setupSeccomp() +{ + if (!settings.filterSyscalls) + return; + +# if HAVE_SECCOMP + scmp_filter_ctx ctx; + + if (!(ctx = seccomp_init(SCMP_ACT_ALLOW))) + throw SysError("unable to initialize seccomp mode 2"); + + Finally cleanup([&]() { seccomp_release(ctx); }); + + constexpr std::string_view nativeSystem = NIX_LOCAL_SYSTEM; + + if (nativeSystem == "x86_64-linux" && seccomp_arch_add(ctx, SCMP_ARCH_X86) != 0) + throw SysError("unable to add 32-bit seccomp architecture"); + + if (nativeSystem == "x86_64-linux" && seccomp_arch_add(ctx, SCMP_ARCH_X32) != 0) + throw SysError("unable to add X32 seccomp architecture"); + + if (nativeSystem == "aarch64-linux" && seccomp_arch_add(ctx, SCMP_ARCH_ARM) != 0) + printError( + "unable to add ARM seccomp architecture; this may result in spurious build failures if running 32-bit ARM processes"); + + if (nativeSystem == "mips64-linux" && seccomp_arch_add(ctx, SCMP_ARCH_MIPS) != 0) + printError("unable to add mips seccomp architecture"); + + if (nativeSystem == "mips64-linux" && seccomp_arch_add(ctx, SCMP_ARCH_MIPS64N32) != 0) + printError("unable to add mips64-*abin32 seccomp architecture"); + + if (nativeSystem == "mips64el-linux" && seccomp_arch_add(ctx, SCMP_ARCH_MIPSEL) != 0) + printError("unable to add mipsel seccomp architecture"); + + if (nativeSystem == "mips64el-linux" && seccomp_arch_add(ctx, SCMP_ARCH_MIPSEL64N32) != 0) + printError("unable to add mips64el-*abin32 seccomp architecture"); + + /* Prevent builders from creating setuid/setgid binaries. */ + for (int perm : {S_ISUID, S_ISGID}) { + if (seccomp_rule_add( + ctx, + SCMP_ACT_ERRNO(EPERM), + SCMP_SYS(chmod), + 1, + SCMP_A1(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) + != 0) + throw SysError("unable to add seccomp rule"); + + if (seccomp_rule_add( + ctx, + SCMP_ACT_ERRNO(EPERM), + SCMP_SYS(fchmod), + 1, + SCMP_A1(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) + != 0) + throw SysError("unable to add seccomp rule"); + + if (seccomp_rule_add( + ctx, + SCMP_ACT_ERRNO(EPERM), + SCMP_SYS(fchmodat), + 1, + SCMP_A2(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) + != 0) + throw SysError("unable to add seccomp rule"); + + if (seccomp_rule_add( + ctx, + SCMP_ACT_ERRNO(EPERM), + NIX_SYSCALL_FCHMODAT2, + 1, + SCMP_A2(SCMP_CMP_MASKED_EQ, (scmp_datum_t) perm, (scmp_datum_t) perm)) + != 0) + throw SysError("unable to add seccomp rule"); + } + + /* Prevent builders from using EAs or ACLs. Not all filesystems + support these, and they're not allowed in the Nix store because + they're not representable in the NAR serialisation. */ + if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(getxattr), 0) != 0 + || seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(lgetxattr), 0) != 0 + || seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(fgetxattr), 0) != 0 + || seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(setxattr), 0) != 0 + || seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(lsetxattr), 0) != 0 + || seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(fsetxattr), 0) != 0) + throw SysError("unable to add seccomp rule"); + + if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_NNP, settings.allowNewPrivileges ? 0 : 1) != 0) + throw SysError("unable to set 'no new privileges' seccomp attribute"); + + if (seccomp_load(ctx) != 0) + throw SysError("unable to load seccomp BPF program"); +# else + throw Error( + "seccomp is not supported on this platform; " + "you can bypass this error by setting the option 'filter-syscalls' to false, but note that untrusted builds can then create setuid binaries!"); +# endif +} + static void doBind(const Path & source, const Path & target, bool optional = false) { debug("bind mounting '%1%' to '%2%'", source, target); @@ -608,6 +706,9 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl if (rmdir("real-root") == -1) throw SysError("cannot remove real-root directory"); + // FIXME: move to LinuxDerivationBuilder + setupSeccomp(); + // FIXME: move to LinuxDerivationBuilder linux::setPersonality(drv.platform); } From f5176500be9644fe771528ad780b8245fb8aa0fe Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 22:23:59 +0200 Subject: [PATCH 09/28] Move autoDelChroot --- src/libstore/unix/build/derivation-builder.cc | 9 --------- src/libstore/unix/build/linux-derivation-builder.cc | 12 ++++++++++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index b40511111..459b294f5 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -117,12 +117,6 @@ protected: */ Path tmpDirInSandbox; - /** - * RAII object to delete the chroot directory. - */ - // FIXME: move - std::shared_ptr autoDelChroot; - /** * The sort of derivation we are building. * @@ -564,9 +558,6 @@ std::variant, SingleDrvOutputs> Derivation for (auto & i : redirectedOutputs) deletePath(store.Store::toRealPath(i.second)); - /* Delete the chroot (if we were using one). */ - autoDelChroot.reset(); /* this runs the destructor */ - deleteTmpDir(true); return std::move(builtOutputs); diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index bfda1e33a..48c605ca3 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -175,6 +175,11 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl */ Path chrootRootDir; + /** + * RAII object to delete the chroot directory. + */ + std::shared_ptr autoDelChroot; + PathsInChroot pathsInChroot; LinuxDerivationBuilder( @@ -183,6 +188,13 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl { } + void deleteTmpDir(bool force) override + { + autoDelChroot.reset(); /* this runs the destructor */ + + DerivationBuilderImpl::deleteTmpDir(force); + } + uid_t sandboxUid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 1000 : 0) : buildUser->getUID(); From 492b684b9ecd08259703f30596e28ece975db191 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 22:29:08 +0200 Subject: [PATCH 10/28] Get rid of tmpDirInSandbox variable --- src/libstore/unix/build/derivation-builder.cc | 38 ++++++++++--------- .../unix/build/linux-derivation-builder.cc | 5 ++- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 459b294f5..b8fc9b178 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -112,11 +112,6 @@ protected: */ Path topTmpDir; - /** - * The path of the temporary directory in the sandbox. - */ - Path tmpDirInSandbox; - /** * The sort of derivation we are building. * @@ -237,7 +232,15 @@ protected: virtual void setBuildTmpDir() { tmpDir = topTmpDir; - tmpDirInSandbox = topTmpDir; + } + + /** + * Return the path of the temporary directory in the sandbox. + */ + virtual Path tmpDirInSandbox() + { + assert(!topTmpDir.empty()); + return topTmpDir; } /** @@ -772,7 +775,6 @@ void DerivationBuilderImpl::startBuilder() topTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), false, false, 0700); setBuildTmpDir(); assert(!tmpDir.empty()); - assert(!tmpDirInSandbox.empty()); chownToBuilder(tmpDir); for (auto & [outputName, status] : initialOutputs) { @@ -936,11 +938,11 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox() pathsInChroot[inside] = {outside, optional}; } - if (hasPrefix(store.storeDir, tmpDirInSandbox)) + if (hasPrefix(store.storeDir, tmpDirInSandbox())) { throw Error("`sandbox-build-dir` must not contain the storeDir"); } - pathsInChroot[tmpDirInSandbox] = tmpDir; + pathsInChroot[tmpDirInSandbox()] = tmpDir; /* Add the closure of store paths to the chroot. */ StorePathSet closure; @@ -1103,7 +1105,7 @@ void DerivationBuilderImpl::initTmpDir() Path p = tmpDir + "/" + fn; writeFile(p, rewriteStrings(i.second, inputRewrites)); chownToBuilder(p); - env[i.first + "Path"] = tmpDirInSandbox + "/" + fn; + env[i.first + "Path"] = tmpDirInSandbox() + "/" + fn; } } @@ -1111,16 +1113,16 @@ void DerivationBuilderImpl::initTmpDir() /* For convenience, set an environment pointing to the top build directory. */ - env["NIX_BUILD_TOP"] = tmpDirInSandbox; + env["NIX_BUILD_TOP"] = tmpDirInSandbox(); /* Also set TMPDIR and variants to point to this directory. */ - env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmpDirInSandbox; + env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmpDirInSandbox(); /* Explicitly set PWD to prevent problems with chroot builds. In particular, dietlibc cannot figure out the cwd because the inode of the current directory doesn't appear in .. (because getdents returns the inode of the mount point). */ - env["PWD"] = tmpDirInSandbox; + env["PWD"] = tmpDirInSandbox(); } @@ -1213,10 +1215,10 @@ void DerivationBuilderImpl::writeStructuredAttrs() writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); chownToBuilder(tmpDir + "/.attrs.sh"); - env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox + "/.attrs.sh"; + env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox() + "/.attrs.sh"; writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); chownToBuilder(tmpDir + "/.attrs.json"); - env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox + "/.attrs.json"; + env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox() + "/.attrs.json"; } } @@ -1240,7 +1242,7 @@ void DerivationBuilderImpl::startDaemon() auto socketName = ".nix-socket"; Path socketPath = tmpDir + "/" + socketName; - env["NIX_REMOTE"] = "unix://" + tmpDirInSandbox + "/" + socketName; + env["NIX_REMOTE"] = "unix://" + tmpDirInSandbox() + "/" + socketName; daemonSocket = createUnixDomainSocket(socketPath, 0600); @@ -1352,7 +1354,7 @@ void DerivationBuilderImpl::runChild() different uid and/or in a sandbox). */ BuiltinBuilderContext ctx{ .drv = drv, - .tmpDirInSandbox = tmpDirInSandbox, + .tmpDirInSandbox = tmpDirInSandbox(), }; if (drv.isBuiltin() && drv.builder == "builtin:fetchurl") { @@ -1367,7 +1369,7 @@ void DerivationBuilderImpl::runChild() enterChroot(); - if (chdir(tmpDirInSandbox.c_str()) == -1) + if (chdir(tmpDirInSandbox().c_str()) == -1) throw SysError("changing into '%1%'", tmpDir); /* Close all other file descriptors. */ diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 48c605ca3..57298c91f 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -226,10 +226,13 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl done directly in the sandbox profile. */ tmpDir = topTmpDir + "/build"; createDir(tmpDir, 0700); + } + Path tmpDirInSandbox() override + { /* In a sandbox, for determinism, always use the same temporary directory. */ - tmpDirInSandbox = settings.sandboxBuildDir; + return settings.sandboxBuildDir; } void prepareSandbox() override From c9bb16a7410d621f988344321fb1000ddc83a47e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 22:35:47 +0200 Subject: [PATCH 11/28] Inline initTmpDir() --- src/libstore/unix/build/derivation-builder.cc | 62 ++++++++----------- 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index b8fc9b178..fc0e4d7eb 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -286,11 +286,6 @@ protected: private: - /** - * Setup tmp dir location. - */ - void initTmpDir(); - /** * Write a JSON file containing the derivation attributes. */ @@ -1089,9 +1084,32 @@ void DerivationBuilderImpl::processSandboxSetupMessages() } } - -void DerivationBuilderImpl::initTmpDir() +void DerivationBuilderImpl::initEnv() { + env.clear(); + + /* Most shells initialise PATH to some default (/bin:/usr/bin:...) when + PATH is not set. We don't want this, so we fill it in with some dummy + value. */ + env["PATH"] = "/path-not-set"; + + /* Set HOME to a non-existing path to prevent certain programs from using + /etc/passwd (or NIS, or whatever) to locate the home directory (for + example, wget looks for ~/.wgetrc). I.e., these tools use /etc/passwd + if HOME is not set, but they will just assume that the settings file + they are looking for does not exist if HOME is set but points to some + non-existing path. */ + env["HOME"] = homeDir; + + /* Tell the builder where the Nix store is. Usually they + shouldn't care, but this is useful for purity checking (e.g., + the compiler or linker might only want to accept paths to files + in the store or in the build directory). */ + env["NIX_STORE"] = store.storeDir; + + /* The maximum number of cores to utilize for parallel building. */ + env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores); + /* In non-structured mode, set all bindings either directory in the environment or via a file, as specified by `DerivationOptions::passAsFile`. */ @@ -1123,36 +1141,6 @@ void DerivationBuilderImpl::initTmpDir() inode of the current directory doesn't appear in .. (because getdents returns the inode of the mount point). */ env["PWD"] = tmpDirInSandbox(); -} - - -void DerivationBuilderImpl::initEnv() -{ - env.clear(); - - /* Most shells initialise PATH to some default (/bin:/usr/bin:...) when - PATH is not set. We don't want this, so we fill it in with some dummy - value. */ - env["PATH"] = "/path-not-set"; - - /* Set HOME to a non-existing path to prevent certain programs from using - /etc/passwd (or NIS, or whatever) to locate the home directory (for - example, wget looks for ~/.wgetrc). I.e., these tools use /etc/passwd - if HOME is not set, but they will just assume that the settings file - they are looking for does not exist if HOME is set but points to some - non-existing path. */ - env["HOME"] = homeDir; - - /* Tell the builder where the Nix store is. Usually they - shouldn't care, but this is useful for purity checking (e.g., - the compiler or linker might only want to accept paths to files - in the store or in the build directory). */ - env["NIX_STORE"] = store.storeDir; - - /* The maximum number of cores to utilize for parallel building. */ - env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores); - - initTmpDir(); /* Compatibility hack with Nix <= 0.7: if this is a fixed-output derivation, tell the builder, so that for instance `fetchurl' From ab18d8ca5fe90391a12d0f26ed301ff52068dbce Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 23:51:24 +0200 Subject: [PATCH 12/28] Move cgroup support --- src/libstore/build/derivation-goal.cc | 2 + src/libstore/unix/build/derivation-builder.cc | 85 +++---------------- .../unix/build/linux-derivation-builder.cc | 73 +++++++++++++++- 3 files changed, 83 insertions(+), 77 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 02f80b65e..fb06670fb 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -99,6 +99,8 @@ void DerivationGoal::killChild() if (builder && builder->pid != -1) { worker.childTerminated(this); + // FIXME: move this into DerivationBuilder. + /* If we're using a build user, then there is a tricky race condition: if we kill the build user before the child has done its setuid() to the build user uid, then it won't be diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index fc0e4d7eb..9c63e3cbb 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -39,10 +39,6 @@ # include #endif -#ifdef __linux__ -# include "nix/util/cgroup.hh" -#endif - #include #include #include @@ -95,12 +91,6 @@ protected: */ std::unique_ptr buildUser; - /** - * The cgroup of the builder, if any. - */ - // FIXME: move - std::optional cgroup; - /** * The temporary directory used for the build. */ @@ -243,6 +233,15 @@ protected: return topTmpDir; } + /** + * Ensure that there are no processes running that conflict with + * `buildUser`. + */ + virtual void prepareUser() + { + killSandbox(false); + } + /** * Called by prepareBuild() to do any setup in the parent to * prepare for a sandboxed build. @@ -429,19 +428,7 @@ static LocalStore & getLocalStore(Store & store) void DerivationBuilderImpl::killSandbox(bool getStats) { - if (cgroup) { - #ifdef __linux__ - auto stats = destroyCgroup(*cgroup); - if (getStats) { - buildResult.cpuUser = stats.cpuUser; - buildResult.cpuSystem = stats.cpuSystem; - } - #else - unreachable(); - #endif - } - - else if (buildUser) { + if (buildUser) { auto uid = buildUser->getUID(); assert(uid != 0); killUser(uid); @@ -690,60 +677,10 @@ static void handleChildException(bool sendException) void DerivationBuilderImpl::startBuilder() { - if ((buildUser && buildUser->getUIDCount() != 1) - #ifdef __linux__ - || settings.useCgroups - #endif - ) - { - #ifdef __linux__ - experimentalFeatureSettings.require(Xp::Cgroups); - - /* If we're running from the daemon, then this will return the - root cgroup of the service. Otherwise, it will return the - current cgroup. */ - auto rootCgroup = getRootCgroup(); - auto cgroupFS = getCgroupFS(); - if (!cgroupFS) - throw Error("cannot determine the cgroups file system"); - auto rootCgroupPath = canonPath(*cgroupFS + "/" + rootCgroup); - if (!pathExists(rootCgroupPath)) - throw Error("expected cgroup directory '%s'", rootCgroupPath); - - static std::atomic counter{0}; - - cgroup = buildUser - ? fmt("%s/nix-build-uid-%d", rootCgroupPath, buildUser->getUID()) - : fmt("%s/nix-build-pid-%d-%d", rootCgroupPath, getpid(), counter++); - - debug("using cgroup '%s'", *cgroup); - - /* When using a build user, record the cgroup we used for that - user so that if we got interrupted previously, we can kill - any left-over cgroup first. */ - if (buildUser) { - auto cgroupsDir = settings.nixStateDir + "/cgroups"; - createDirs(cgroupsDir); - - auto cgroupFile = fmt("%s/%d", cgroupsDir, buildUser->getUID()); - - if (pathExists(cgroupFile)) { - auto prevCgroup = readFile(cgroupFile); - destroyCgroup(prevCgroup); - } - - writeFile(cgroupFile, *cgroup); - } - - #else - throw Error("cgroups are not supported on this platform"); - #endif - } - /* Make sure that no other processes are executing under the sandbox uids. This must be done before any chownToBuilder() calls. */ - killSandbox(false); + prepareUser(); /* Right platform? */ if (!drvOptions.canBuildLocally(store, drv)) { diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 57298c91f..5dfd468a3 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -1,6 +1,10 @@ #ifdef __linux__ +# include "nix/store/personality.hh" +# include "nix/util/cgroup.hh" +# include "nix/util/namespaces.hh" # include "linux/fchmodat2-compat.hh" + # include # include # include @@ -9,13 +13,12 @@ # include # include # include -# include "nix/util/namespaces.hh" + # if HAVE_SECCOMP # include # endif + # define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, put_old)) -# include "nix/util/cgroup.hh" -# include "nix/store/personality.hh" namespace nix { @@ -182,6 +185,11 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl PathsInChroot pathsInChroot; + /** + * The cgroup of the builder, if any. + */ + std::optional cgroup; + LinuxDerivationBuilder( Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) @@ -235,6 +243,51 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl return settings.sandboxBuildDir; } + void prepareUser() override + { + if ((buildUser && buildUser->getUIDCount() != 1) || settings.useCgroups) { + experimentalFeatureSettings.require(Xp::Cgroups); + + /* If we're running from the daemon, then this will return the + root cgroup of the service. Otherwise, it will return the + current cgroup. */ + auto rootCgroup = getRootCgroup(); + auto cgroupFS = getCgroupFS(); + if (!cgroupFS) + throw Error("cannot determine the cgroups file system"); + auto rootCgroupPath = canonPath(*cgroupFS + "/" + rootCgroup); + if (!pathExists(rootCgroupPath)) + throw Error("expected cgroup directory '%s'", rootCgroupPath); + + static std::atomic counter{0}; + + cgroup = buildUser ? fmt("%s/nix-build-uid-%d", rootCgroupPath, buildUser->getUID()) + : fmt("%s/nix-build-pid-%d-%d", rootCgroupPath, getpid(), counter++); + + debug("using cgroup '%s'", *cgroup); + + /* When using a build user, record the cgroup we used for that + user so that if we got interrupted previously, we can kill + any left-over cgroup first. */ + if (buildUser) { + auto cgroupsDir = settings.nixStateDir + "/cgroups"; + createDirs(cgroupsDir); + + auto cgroupFile = fmt("%s/%d", cgroupsDir, buildUser->getUID()); + + if (pathExists(cgroupFile)) { + auto prevCgroup = readFile(cgroupFile); + destroyCgroup(prevCgroup); + } + + writeFile(cgroupFile, *cgroup); + } + } + + // Kill any processes left in the cgroup or build user. + DerivationBuilderImpl::prepareUser(); + } + void prepareSandbox() override { /* Create a temporary directory in which we set up the chroot @@ -747,6 +800,20 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl return DerivationBuilderImpl::unprepareBuild(); } + void killSandbox(bool getStats) override + { + if (cgroup) { + auto stats = destroyCgroup(*cgroup); + if (getStats) { + buildResult.cpuUser = stats.cpuUser; + buildResult.cpuSystem = stats.cpuSystem; + } + return; + } + + DerivationBuilderImpl::killSandbox(getStats); + } + void cleanupBuild() override { DerivationBuilderImpl::cleanupBuild(); From 21fd15227917b795154cfe5f2858659da5fe9119 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 27 May 2025 15:25:51 +0200 Subject: [PATCH 13/28] Fix macOS build --- .../unix/build/darwin-derivation-builder.cc | 71 +++++++++++-------- src/libstore/unix/build/derivation-builder.cc | 20 ++++-- 2 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc index 2ba54ad97..5e06dbe55 100644 --- a/src/libstore/unix/build/darwin-derivation-builder.cc +++ b/src/libstore/unix/build/darwin-derivation-builder.cc @@ -14,11 +14,20 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl { PathsInChroot pathsInChroot; + /** + * Whether full sandboxing is enabled. Note that macOS builds + * always have *some* sandboxing (see sandbox-minimal.sb). + */ + bool useSandbox; + DarwinDerivationBuilder( - Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) + Store & store, + std::unique_ptr miscMethods, + DerivationBuilderParams params, + bool useSandbox) : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) + , useSandbox(useSandbox) { - useChroot = true; } void prepareSandbox() override @@ -26,32 +35,6 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl pathsInChroot = getPathsInSandbox(); } - void execBuilder(const Strings & args, const Strings & envStrs) override - { - posix_spawnattr_t attrp; - - if (posix_spawnattr_init(&attrp)) - throw SysError("failed to initialize builder"); - - if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) - throw SysError("failed to initialize builder"); - - if (drv.platform == "aarch64-darwin") { - // Unset kern.curproc_arch_affinity so we can escape Rosetta - int affinity = 0; - sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); - - cpu_type_t cpu = CPU_TYPE_ARM64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } else if (drv.platform == "x86_64-darwin") { - cpu_type_t cpu = CPU_TYPE_X86_64; - posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); - } - - posix_spawn( - NULL, drv.builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); - } - void setUser() override { DerivationBuilderImpl::setUser(); @@ -59,7 +42,7 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl /* This has to appear before import statements. */ std::string sandboxProfile = "(version 1)\n"; - if (useChroot) { + if (useSandbox) { /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ PathSet ancestry; @@ -101,7 +84,7 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl # include "sandbox-defaults.sb" ; - if (!derivationType->isSandboxed()) + if (!derivationType.isSandboxed()) sandboxProfile += # include "sandbox-network.sb" ; @@ -193,7 +176,33 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl } } } -} + + void execBuilder(const Strings & args, const Strings & envStrs) override + { + posix_spawnattr_t attrp; + + if (posix_spawnattr_init(&attrp)) + throw SysError("failed to initialize builder"); + + if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC)) + throw SysError("failed to initialize builder"); + + if (drv.platform == "aarch64-darwin") { + // Unset kern.curproc_arch_affinity so we can escape Rosetta + int affinity = 0; + sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity)); + + cpu_type_t cpu = CPU_TYPE_ARM64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } else if (drv.platform == "x86_64-darwin") { + cpu_type_t cpu = CPU_TYPE_X86_64; + posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); + } + + posix_spawn( + NULL, drv.builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); + } +}; } diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 9c63e3cbb..8c64d31e8 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -315,8 +315,6 @@ protected: */ void runChild(); -private: - /** * Move the current process into the chroot, if any. Called early * by runChild(). @@ -337,6 +335,8 @@ private: */ virtual void execBuilder(const Strings & args, const Strings & envStrs); +private: + /** * Check that the derivation outputs all exist and register them * as valid. @@ -2138,7 +2138,7 @@ std::unique_ptr makeDerivationBuilder( throw Error("derivation '%s' has '__noChroot' set, " "but that's not allowed when 'sandbox' is 'true'", store.printStorePath(params.drvPath)); #ifdef __APPLE__ - if (drvOptions.additionalSandboxProfile != "") + if (params.drvOptions.additionalSandboxProfile != "") throw Error("derivation '%s' specifies a sandbox profile, " "but this is only allowed when 'sandbox' is 'relaxed'", store.printStorePath(params.drvPath)); #endif @@ -2177,16 +2177,24 @@ std::unique_ptr makeDerivationBuilder( std::move(params)); #endif - if (useSandbox) - throw Error("sandboxing builds is not supported on this platform"); - if (params.drvOptions.useUidRange(params.drv)) throw Error("feature 'uid-range' is only supported in sandboxed builds"); + #ifdef __APPLE__ + return std::make_unique( + store, + std::move(miscMethods), + std::move(params), + useSandbox); + #else + if (useSandbox) + throw Error("sandboxing builds is not supported on this platform"); + return std::make_unique( store, std::move(miscMethods), std::move(params)); + #endif } } From d0a263711aab8fa54afc3cb166374ab0a6853448 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 27 May 2025 17:53:56 +0200 Subject: [PATCH 14/28] Remove unused variable --- src/libstore/unix/build/derivation-builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 8c64d31e8..daa19c380 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -802,7 +802,7 @@ void DerivationBuilderImpl::startBuilder() printMsg(lvlVomit, "setting builder env variable '%1%'='%2%'", i.first, i.second); /* Create the log file. */ - [[maybe_unused]] Path logFile = miscMethods->openLogFile(); + miscMethods->openLogFile(); /* Create a pseudoterminal to get the output of the builder. */ builderOut = posix_openpt(O_RDWR | O_NOCTTY); From 95f87abf66f658b628e56b871f33de52798ee978 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 28 May 2025 13:04:09 +0200 Subject: [PATCH 15/28] Cleanup --- src/libstore/unix/build/linux-derivation-builder.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 5dfd468a3..0d7d94b87 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -190,11 +190,7 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl */ std::optional cgroup; - LinuxDerivationBuilder( - Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) - : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) - { - } + using DerivationBuilderImpl::DerivationBuilderImpl; void deleteTmpDir(bool force) override { From 803d461e956b64187a079805352380b286a0c788 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 28 May 2025 19:02:38 +0200 Subject: [PATCH 16/28] Add external builders These are helper programs that execute derivations for specified system types (e.g. using QEMU to emulate another system type). To use, set `external-builders`: external-builders = [{"systems": ["aarch64-linux"], "program": "/path/to/external-builder.py"}] The external builder gets one command line argument, the path to a JSON file containing all necessary information about the derivation: { "args": [...], "builder": "/nix/store/kwcyvgdg98n98hqapaz8sw92pc2s78x6-bash-5.2p37/bin/bash", "env": { "HOME": "/homeless-shelter", ... }, "realStoreDir": "/tmp/nix/nix/store", "storeDir": "/nix/store", "tmpDir": "/tmp/nix-shell.dzQ2hE/nix-build-patchelf-0.14.3.drv-46/build", "tmpDirInSandbox": "/build" } --- src/libstore/globals.cc | 11 ++ src/libstore/include/nix/store/globals.hh | 17 +++ src/libstore/unix/build/derivation-builder.cc | 27 ++++- .../unix/build/external-derivation-builder.cc | 107 ++++++++++++++++++ 4 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 src/libstore/unix/build/external-derivation-builder.cc diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index e4c1f8819..89f2ee7d0 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -309,6 +309,17 @@ unsigned int MaxBuildJobsSetting::parse(const std::string & str) const } } +NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Settings::ExternalBuilder, systems, program); + +template<> Settings::ExternalBuilders BaseSetting::parse(const std::string & str) const +{ + return nlohmann::json::parse(str).template get(); +} + +template<> std::string BaseSetting::to_string() const +{ + return nlohmann::json(value).dump(); +} static void preloadNSS() { diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 00d7dcd6b..7f3c9f388 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -1236,6 +1236,23 @@ public: Set it to 1 to warn on all paths. )" }; + + struct ExternalBuilder + { + std::vector systems; + Path program; + }; + + using ExternalBuilders = std::vector; + + Setting externalBuilders{ + this, + {}, + "external-builders", + R"( + Helper programs that execute derivations. + )" + }; }; diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index daa19c380..ff06acfbb 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -208,6 +208,12 @@ protected: return acquireUserLock(1, false); } + /** + * Throw an exception if we can't do this derivation because of + * missing system features. + */ + virtual void checkSystem(); + /** * Return the paths that should be made available in the sandbox. * This includes: @@ -675,13 +681,8 @@ static void handleChildException(bool sendException) } } -void DerivationBuilderImpl::startBuilder() +void DerivationBuilderImpl::checkSystem() { - /* Make sure that no other processes are executing under the - sandbox uids. This must be done before any chownToBuilder() - calls. */ - prepareUser(); - /* Right platform? */ if (!drvOptions.canBuildLocally(store, drv)) { auto msg = fmt( @@ -701,6 +702,16 @@ void DerivationBuilderImpl::startBuilder() throw BuildError(msg); } +} + +void DerivationBuilderImpl::startBuilder() +{ + checkSystem(); + + /* Make sure that no other processes are executing under the + sandbox uids. This must be done before any chownToBuilder() + calls. */ + prepareUser(); /* Create a temporary directory where the build will take place. */ @@ -2121,6 +2132,7 @@ StorePath DerivationBuilderImpl::makeFallbackPath(const StorePath & path) // FIXME: do this properly #include "linux-derivation-builder.cc" #include "darwin-derivation-builder.cc" +#include "external-derivation-builder.cc" namespace nix { @@ -2129,6 +2141,9 @@ std::unique_ptr makeDerivationBuilder( std::unique_ptr miscMethods, DerivationBuilderParams params) { + if (auto builder = ExternalDerivationBuilder::newIfSupported(store, miscMethods, params)) + return builder; + bool useSandbox = false; /* Are we doing a sandboxed build? */ diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc new file mode 100644 index 000000000..0f32392a5 --- /dev/null +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -0,0 +1,107 @@ +namespace nix { + +struct ExternalDerivationBuilder : DerivationBuilderImpl +{ + Settings::ExternalBuilder externalBuilder; + + ExternalDerivationBuilder( + Store & store, + std::unique_ptr miscMethods, + DerivationBuilderParams params, + Settings::ExternalBuilder externalBuilder) + : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) + , externalBuilder(std::move(externalBuilder)) + { + } + + static std::unique_ptr newIfSupported( + Store & store, std::unique_ptr & miscMethods, DerivationBuilderParams & params) + { + for (auto & handler : settings.externalBuilders.get()) { + for (auto & system : handler.systems) + if (params.drv.platform == system) + return std::make_unique( + store, std::move(miscMethods), std::move(params), std::move(handler)); + } + return {}; + } + + bool prepareBuild() override + { + // External builds don't use build users, so this always + // succeeds. + return true; + } + + Path tmpDirInSandbox() override + { + /* In a sandbox, for determinism, always use the same temporary + directory. */ + return "/build"; + } + + void setBuildTmpDir() override + { + tmpDir = topTmpDir + "/build"; + createDir(tmpDir, 0700); + } + + void prepareUser() override + { + // Nothing to do here since we don't have a build user. + } + + void checkSystem() override + { + // FIXME: should check system features. + } + + void startChild() override + { + if (drvOptions.getRequiredSystemFeatures(drv).count("recursive-nix")) + throw Error("'recursive-nix' is not supported yet by external derivation builders"); + + auto json = nlohmann::json::object(); + + json.emplace("builder", drv.builder); + { + auto l = nlohmann::json::array(); + for (auto & i : drv.args) + l.push_back(rewriteStrings(i, inputRewrites)); + json.emplace("args", std::move(l)); + } + { + auto j = nlohmann::json::object(); + for (auto & [name, value] : env) + j.emplace(name, rewriteStrings(value, inputRewrites)); + json.emplace("env", std::move(j)); + } + json.emplace("topTmpDir", topTmpDir); + json.emplace("tmpDir", tmpDir); + json.emplace("tmpDirInSandbox", tmpDirInSandbox()); + json.emplace("storeDir", store.storeDir); + json.emplace("realStoreDir", getLocalStore(store).config->realStoreDir.get()); + json.emplace("system", drv.platform); + + auto jsonFile = topTmpDir + "/build.json"; + writeFile(jsonFile, json.dump()); + + pid = startProcess([&]() { + openSlave(); + try { + commonChildInit(); + + Strings args = {externalBuilder.program, jsonFile}; + + execv(externalBuilder.program.c_str(), stringsToCharPtrs(args).data()); + + throw SysError("executing '%s'", externalBuilder.program); + } catch (...) { + handleChildException(true); + _exit(1); + } + }); + } +}; + +} From a0fb93f09bac64ea21888034a0ef619b1fabcb86 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 30 May 2025 20:56:51 +0200 Subject: [PATCH 17/28] Make sandbox error messages more readable --- src/libstore/unix/build/derivation-builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index ff06acfbb..6baf61125 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1016,7 +1016,7 @@ void DerivationBuilderImpl::processSandboxSetupMessages() e.addTrace({}, "while waiting for the build environment for '%s' to initialize (%s, previous messages: %s)", store.printStorePath(drvPath), statusToString(status), - concatStringsSep("|", msgs)); + concatStringsSep("\n", msgs)); throw; } }(); From 5842d54ceea46542763a1466e018360e3a71545b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 11 Jun 2025 10:17:58 +0200 Subject: [PATCH 18/28] Drop bad std::move Co-authored-by: Cole Helbling --- src/libstore/unix/build/external-derivation-builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 0f32392a5..8efdf8ff9 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -21,7 +21,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl for (auto & system : handler.systems) if (params.drv.platform == system) return std::make_unique( - store, std::move(miscMethods), std::move(params), std::move(handler)); + store, std::move(miscMethods), std::move(params), handler); } return {}; } From a20a7fa1eae3d65cbf3e1fca866028bedf6e17e0 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 17 Jun 2025 12:59:48 -0700 Subject: [PATCH 19/28] Allow specifying args to external builder program --- src/libstore/globals.cc | 2 +- src/libstore/include/nix/store/globals.hh | 63 +++++++++++++++++++ .../unix/build/external-derivation-builder.cc | 10 ++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 89f2ee7d0..997d72b99 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -309,7 +309,7 @@ unsigned int MaxBuildJobsSetting::parse(const std::string & str) const } } -NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Settings::ExternalBuilder, systems, program); +NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Settings::ExternalBuilder, systems, program, args); template<> Settings::ExternalBuilders BaseSetting::parse(const std::string & str) const { diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 7f3c9f388..2976ee57a 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -1241,6 +1241,7 @@ public: { std::vector systems; Path program; + std::optional> args; }; using ExternalBuilders = std::vector; @@ -1251,6 +1252,68 @@ public: "external-builders", R"( Helper programs that execute derivations. + + The program is passed a JSON document that describes the build environment as the final argument. + The JSON document looks like this: + + { + "args": [ + "-e", + "/nix/store/vj1c3wf9c11a0qs6p3ymfvrnsdgsdcbq-source-stdenv.sh", + "/nix/store/shkw4qm9qcw5sc5n1k5jznc83ny02r39-default-builder.sh" + ], + "builder": "/nix/store/s1qkj0ph0ma64a6743mvkwnabrbw1hsc-bash-5.2p37/bin/bash", + "env": { + "HOME": "/homeless-shelter", + "NIX_BUILD_CORES": "14", + "NIX_BUILD_TOP": "/build", + "NIX_LOG_FD": "2", + "NIX_STORE": "/nix/store", + "PATH": "/path-not-set", + "PWD": "/build", + "TEMP": "/build", + "TEMPDIR": "/build", + "TERM": "xterm-256color", + "TMP": "/build", + "TMPDIR": "/build", + "__structuredAttrs": "", + "buildInputs": "", + "builder": "/nix/store/s1qkj0ph0ma64a6743mvkwnabrbw1hsc-bash-5.2p37/bin/bash", + "cmakeFlags": "", + "configureFlags": "", + "depsBuildBuild": "", + "depsBuildBuildPropagated": "", + "depsBuildTarget": "", + "depsBuildTargetPropagated": "", + "depsHostHost": "", + "depsHostHostPropagated": "", + "depsTargetTarget": "", + "depsTargetTargetPropagated": "", + "doCheck": "1", + "doInstallCheck": "1", + "mesonFlags": "", + "name": "hello-2.12.2", + "nativeBuildInputs": "/nix/store/l31j72f1h33hsa4nq4iyhsmsqjyndq9f-version-check-hook", + "out": "/nix/store/2yx2prgxmzbkrnbb4liy6n4zkzb1cqai-hello-2.12.2", + "outputs": "out", + "patches": "", + "pname": "hello", + "postInstallCheck": "stat \"${!outputBin}/bin/hello\"\n", + "propagatedBuildInputs": "", + "propagatedNativeBuildInputs": "", + "src": "/nix/store/dw402azxjrgrzrk6j0p66wkqrab5mwgw-hello-2.12.2.tar.gz", + "stdenv": "/nix/store/i8bw5nqg1225m281zr6lgsz42bw04z7g-stdenv-linux", + "strictDeps": "", + "system": "aarch64-linux", + "version": "2.12.2" + }, + "realStoreDir": "/nix/store", + "storeDir": "/nix/store", + "system": "aarch64-linux", + "tmpDir": "/private/tmp/nix-build-hello-2.12.2.drv-0/build", + "tmpDirInSandbox": "/build", + "topTmpDir": "/private/tmp/nix-build-hello-2.12.2.drv-0" + } )" }; }; diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 8efdf8ff9..0757ed51f 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -83,6 +83,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl json.emplace("realStoreDir", getLocalStore(store).config->realStoreDir.get()); json.emplace("system", drv.platform); + // FIXME: maybe write this JSON into the builder's stdin instead....? auto jsonFile = topTmpDir + "/build.json"; writeFile(jsonFile, json.dump()); @@ -91,8 +92,15 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl try { commonChildInit(); - Strings args = {externalBuilder.program, jsonFile}; + Strings args = {externalBuilder.program}; + if (externalBuilder.args) { + args.insert(args.end(), externalBuilder.args->begin(), externalBuilder.args->end()); + } + + args.insert(args.end(), jsonFile); + + debug("executing external builder: %s", concatStringsSep(" ", args)); execv(externalBuilder.program.c_str(), stringsToCharPtrs(args).data()); throw SysError("executing '%s'", externalBuilder.program); From b64a310eb261118bcf5196761f685af3e44c2561 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 28 May 2025 12:49:13 -0400 Subject: [PATCH 20/28] Fix warning when `HAVE_EMBEDDED_SANDBOX_SHELL` is not set Clang doesn't like the double indent that is needed for the `if...else` that is CPP'd away. Adding braces is fine in the `if...else...` case, and fine as a naked block in the CPP'd away case, and properly-indented both ways. --- src/libstore/unix/build/linux-derivation-builder.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 0d7d94b87..dbd98ab3d 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -681,7 +681,9 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl chmod_(dst, 0555); } else # endif + { doBind(i.second.source, chrootRootDir + i.first, i.second.optional); + } } /* Bind a new instance of procfs on /proc. */ From 1521a819b75810e9c0f0450745d66b4620fff3da Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 30 Jun 2025 10:18:10 -0700 Subject: [PATCH 21/28] external-derivation-builder: `args` must always be specified I don't want to figure out how to make nlohmann treat std::optional<> the same way Rust's serde_json treats Option<> (i.e. skip it if it's not there). --- src/libstore/include/nix/store/globals.hh | 2 +- src/libstore/unix/build/external-derivation-builder.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 2976ee57a..f7c714777 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -1241,7 +1241,7 @@ public: { std::vector systems; Path program; - std::optional> args; + std::vector args; }; using ExternalBuilders = std::vector; diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 0757ed51f..1906ddd70 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -94,8 +94,8 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl Strings args = {externalBuilder.program}; - if (externalBuilder.args) { - args.insert(args.end(), externalBuilder.args->begin(), externalBuilder.args->end()); + if (!externalBuilder.args.empty()) { + args.insert(args.end(), externalBuilder.args.begin(), externalBuilder.args.end()); } args.insert(args.end(), jsonFile); From d1f57c5dae43468d331a7fdb4c5a5e44eff28f1c Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 30 Jun 2025 13:56:04 -0700 Subject: [PATCH 22/28] external-derivation-builder: write the json doc into builder's stdin --- src/libstore/include/nix/store/globals.hh | 2 +- .../unix/build/external-derivation-builder.cc | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index fcfc2e94a..041300bed 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -1248,7 +1248,7 @@ public: R"( Helper programs that execute derivations. - The program is passed a JSON document that describes the build environment as the final argument. + The program is passed a JSON document that describes the build environment on standard input. The JSON document looks like this: { diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 1906ddd70..9fe0eb19f 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -4,6 +4,11 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl { Settings::ExternalBuilder externalBuilder; + /** + * Pipe for talking to the spawned builder. + */ + Pipe toBuilder; + ExternalDerivationBuilder( Store & store, std::unique_ptr miscMethods, @@ -83,23 +88,22 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl json.emplace("realStoreDir", getLocalStore(store).config->realStoreDir.get()); json.emplace("system", drv.platform); - // FIXME: maybe write this JSON into the builder's stdin instead....? - auto jsonFile = topTmpDir + "/build.json"; - writeFile(jsonFile, json.dump()); + toBuilder.create(); pid = startProcess([&]() { openSlave(); try { commonChildInit(); + if (dup2(toBuilder.readSide.get(), STDIN_FILENO) == -1) + throw SysError("duping to-builder read side to builder's stdin"); + Strings args = {externalBuilder.program}; if (!externalBuilder.args.empty()) { args.insert(args.end(), externalBuilder.args.begin(), externalBuilder.args.end()); } - args.insert(args.end(), jsonFile); - debug("executing external builder: %s", concatStringsSep(" ", args)); execv(externalBuilder.program.c_str(), stringsToCharPtrs(args).data()); @@ -109,6 +113,9 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl _exit(1); } }); + + writeFull(toBuilder.writeSide.get(), json.dump()); + toBuilder.close(); } }; From efa239875b772544e6650aee57452d108d29acbe Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 14 Jul 2025 07:32:11 -0700 Subject: [PATCH 23/28] Add an `external-builders` experimental feature --- src/libstore/include/nix/store/globals.hh | 20 ++++++++++++++++++- .../unix/build/external-derivation-builder.cc | 1 + src/libutil/experimental-features.cc | 8 ++++++++ .../include/nix/util/experimental-features.hh | 1 + 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 041300bed..2dfd187c1 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -1309,7 +1309,25 @@ public: "tmpDirInSandbox": "/build", "topTmpDir": "/private/tmp/nix-build-hello-2.12.2.drv-0" } - )" + )", + {}, // aliases + true, // document default + // NOTE(cole-h): even though we can make the experimental feature required here, the errors + // are not as good (it just becomes a warning if you try to use this setting without the + // experimental feature) + // + // With this commented out: + // + // error: experimental Nix feature 'external-builders' is disabled; add '--extra-experimental-features external-builders' to enable it + // + // With this uncommented: + // + // warning: Ignoring setting 'external-builders' because experimental feature 'external-builders' is not enabled + // error: Cannot build '/nix/store/vwsp4qd8a62jqa36p26d15hin4xnj949-opentofu-1.10.2.drv'. + // Reason: required system or feature not available + // Required system: 'aarch64-linux' with features {} + // Current system: 'aarch64-darwin' with features {apple-virt, benchmark, big-parallel, nixos-test} + // Xp::ExternalBuilders }; }; diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 9fe0eb19f..20919187c 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -17,6 +17,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) , externalBuilder(std::move(externalBuilder)) { + experimentalFeatureSettings.require(Xp::ExternalBuilders); } static std::unique_ptr newIfSupported( diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 04e8705e5..075b90ec5 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -288,6 +288,14 @@ constexpr std::array xpFeatureDetails )", .trackingUrl = "https://github.com/NixOS/nix/milestone/55", }, + { + .tag = Xp::ExternalBuilders, + .name = "external-builders", + .description = R"( + Enables support for external builders / sandbox providers. + )", + .trackingUrl = "", + }, { .tag = Xp::BLAKE3Hashes, .name = "blake3-hashes", diff --git a/src/libutil/include/nix/util/experimental-features.hh b/src/libutil/include/nix/util/experimental-features.hh index d7bc56f27..5a01d960c 100644 --- a/src/libutil/include/nix/util/experimental-features.hh +++ b/src/libutil/include/nix/util/experimental-features.hh @@ -35,6 +35,7 @@ enum struct ExperimentalFeature MountedSSHStore, VerifiedFetches, PipeOperators, + ExternalBuilders, BLAKE3Hashes, }; From 5b27325bc23472862ece37cd5883ebb65f206959 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 14 Jul 2025 11:00:13 -0700 Subject: [PATCH 24/28] Revert "external-derivation-builder: write the json doc into builder's stdin" This reverts commit d1f57c5dae43468d331a7fdb4c5a5e44eff28f1c. --- src/libstore/include/nix/store/globals.hh | 2 +- .../unix/build/external-derivation-builder.cc | 17 +++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 2dfd187c1..fdc0c0827 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -1248,7 +1248,7 @@ public: R"( Helper programs that execute derivations. - The program is passed a JSON document that describes the build environment on standard input. + The program is passed a JSON document that describes the build environment as the final argument. The JSON document looks like this: { diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 20919187c..e71cd7119 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -4,11 +4,6 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl { Settings::ExternalBuilder externalBuilder; - /** - * Pipe for talking to the spawned builder. - */ - Pipe toBuilder; - ExternalDerivationBuilder( Store & store, std::unique_ptr miscMethods, @@ -89,22 +84,23 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl json.emplace("realStoreDir", getLocalStore(store).config->realStoreDir.get()); json.emplace("system", drv.platform); - toBuilder.create(); + // FIXME: maybe write this JSON into the builder's stdin instead....? + auto jsonFile = topTmpDir + "/build.json"; + writeFile(jsonFile, json.dump()); pid = startProcess([&]() { openSlave(); try { commonChildInit(); - if (dup2(toBuilder.readSide.get(), STDIN_FILENO) == -1) - throw SysError("duping to-builder read side to builder's stdin"); - Strings args = {externalBuilder.program}; if (!externalBuilder.args.empty()) { args.insert(args.end(), externalBuilder.args.begin(), externalBuilder.args.end()); } + args.insert(args.end(), jsonFile); + debug("executing external builder: %s", concatStringsSep(" ", args)); execv(externalBuilder.program.c_str(), stringsToCharPtrs(args).data()); @@ -114,9 +110,6 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl _exit(1); } }); - - writeFull(toBuilder.writeSide.get(), json.dump()); - toBuilder.close(); } }; From de158c335c97b4728856311d6cdacb2eaac920dd Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 14 Jul 2025 11:01:46 -0700 Subject: [PATCH 25/28] fixup: document why we're not writing through stdin right now --- src/libstore/unix/build/external-derivation-builder.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index e71cd7119..508ad45a3 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -84,8 +84,10 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl json.emplace("realStoreDir", getLocalStore(store).config->realStoreDir.get()); json.emplace("system", drv.platform); - // FIXME: maybe write this JSON into the builder's stdin instead....? - auto jsonFile = topTmpDir + "/build.json"; + // TODO(cole-h): writing this to stdin is too much effort right now, if we want to revisit + // that, see this comment by Eelco about how to make it not suck: + // https://github.com/DeterminateSystems/nix-src/pull/141#discussion_r2205493257 + auto jsonFile = std::filesystem::path{topTmpDir} / "build.json"; writeFile(jsonFile, json.dump()); pid = startProcess([&]() { From 51449d7a5197ee66a647d2e0cf4374aa6e850c4b Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 15 Jul 2025 09:56:07 -0700 Subject: [PATCH 26/28] external-derivation-builder: run under build user, chown topTmpDir to builder The chown to builder is necessary for granting the builder the ability to access its entire ancestry (which is required on macOS for things like mounting the build directory into a VM to work) while running under a build user. Eelco mentioned that the reason topTmpDir is generally 700 is because of how the Linux chroot is setup, but since we do not use a chroot on macOS, it's fine to make the build dir readable to the build user. --- .../unix/build/external-derivation-builder.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 508ad45a3..79ce0ba45 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -29,9 +29,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl bool prepareBuild() override { - // External builds don't use build users, so this always - // succeeds. - return true; + return DerivationBuilderImpl::prepareBuild(); } Path tmpDirInSandbox() override @@ -49,7 +47,12 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl void prepareUser() override { - // Nothing to do here since we don't have a build user. + DerivationBuilderImpl::prepareUser(); + } + + void setUser() override + { + DerivationBuilderImpl::setUser(); } void checkSystem() override @@ -103,6 +106,10 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl args.insert(args.end(), jsonFile); + chownToBuilder(topTmpDir); + + setUser(); + debug("executing external builder: %s", concatStringsSep(" ", args)); execv(externalBuilder.program.c_str(), stringsToCharPtrs(args).data()); From d3dc64b81138417290ac31f6fb9171d3778f1ad3 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 15 Jul 2025 09:56:07 -0700 Subject: [PATCH 27/28] external-derivation-builder: chdir into tmpdir --- src/libstore/unix/build/external-derivation-builder.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 79ce0ba45..a393d75d9 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -106,6 +106,9 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl args.insert(args.end(), jsonFile); + if (chdir(tmpDir.c_str()) == -1) + throw SysError("changing into '%1%'", tmpDir); + chownToBuilder(topTmpDir); setUser(); From 3cabd4ff2ee5fd8caa098cad87e0f6764cd22bf9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 17 Jul 2025 17:35:15 +0200 Subject: [PATCH 28/28] Improve error message parsing external-builders setting --- src/libstore/globals.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 23c844e3f..9f51d90d9 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -313,7 +313,11 @@ NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Settings::ExternalBuilder, systems, program, template<> Settings::ExternalBuilders BaseSetting::parse(const std::string & str) const { - return nlohmann::json::parse(str).template get(); + try { + return nlohmann::json::parse(str).template get(); + } catch (std::exception & e) { + throw UsageError("parsing setting '%s': %s", name, e.what()); + } } template<> std::string BaseSetting::to_string() const