From e786a701a83c3b824e8aa950a20a8fcde328d0ef Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 9 Sep 2024 17:37:44 +0200 Subject: [PATCH 1/5] installerScriptForGHA: aarch64-darwin Backport of df3e92ff964ba7e87c426f4bbd621032811ab5e1 https://github.com/NixOS/nix/pull/11009 GitHub Actions seems to have magically switched architectures without changing their identifiers. See https://github.com/actions/runner-images/blob/2813ee66cbe85b31a8322ff8967148548b1a5db9/README.md#available-images Maybe they have more complete documentation elsewhere, but it seems to be incapable of selecting a runner based on architecture. --- build/hydra.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/hydra.nix b/build/hydra.nix index 595aad324..4ac134a11 100644 --- a/build/hydra.nix +++ b/build/hydra.nix @@ -98,7 +98,7 @@ in installerScriptForGHA = installScriptFor [ # Native self.hydraJobs.binaryTarball."x86_64-linux" - self.hydraJobs.binaryTarball."x86_64-darwin" + self.hydraJobs.binaryTarball."aarch64-darwin" # Cross self.hydraJobs.binaryTarballCross."x86_64-linux"."armv6l-unknown-linux-gnueabihf" self.hydraJobs.binaryTarballCross."x86_64-linux"."armv7l-unknown-linux-gnueabihf" From 05994033d58e358ddafe51d1d04626eb76b8a192 Mon Sep 17 00:00:00 2001 From: Puck Meerburg Date: Fri, 1 Mar 2024 11:42:24 -0500 Subject: [PATCH 2/5] fix: Run all derivation builders inside the sandbox on macOS --- configure.ac | 6 +- package.nix | 2 + .../unix/build/local-derivation-goal.cc | 237 +++++++++--------- 3 files changed, 123 insertions(+), 122 deletions(-) diff --git a/configure.ac b/configure.ac index 90a6d45d5..f98a0a5ea 100644 --- a/configure.ac +++ b/configure.ac @@ -62,12 +62,16 @@ AC_CHECK_TOOL([AR], [ar]) AC_SYS_LARGEFILE -# Solaris-specific stuff. +# OS-specific stuff. case "$host_os" in solaris*) # Solaris requires -lsocket -lnsl for network functions LDFLAGS="-lsocket -lnsl $LDFLAGS" ;; + darwin*) + # Need to link to libsandbox. + LDFLAGS="-lsandbox $LDFLAGS" + ;; esac diff --git a/package.nix b/package.nix index cf1654c6a..1dfe7ab31 100644 --- a/package.nix +++ b/package.nix @@ -27,6 +27,7 @@ , libseccomp , libsodium , man +, darwin , lowdown , mdbook , mdbook-linkcheck @@ -250,6 +251,7 @@ in { gtest rapidcheck ] ++ lib.optional stdenv.isLinux libseccomp + ++ lib.optional stdenv.hostPlatform.isDarwin darwin.apple_sdk.libs.sandbox ++ lib.optional stdenv.hostPlatform.isx86_64 libcpuid # There have been issues building these dependencies ++ lib.optional (stdenv.hostPlatform == stdenv.buildPlatform && (stdenv.isLinux || stdenv.isDarwin)) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index ae9c715d6..878644fa5 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -58,6 +58,10 @@ #if __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 @@ -2017,141 +2021,132 @@ void LocalDerivationGoal::runChild() std::string builder = "invalid"; - if (drv->isBuiltin()) { - ; - } #if __APPLE__ - else { - /* This has to appear before import statements. */ - std::string sandboxProfile = "(version 1)\n"; + /* This has to appear before import statements. */ + std::string sandboxProfile = "(version 1)\n"; - if (useChroot) { + if (useChroot) { - /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ - PathSet ancestry; + /* 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 = worker.store.storeDir; + /* 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) { - ancestry.insert(cur); cur = dirOf(cur); + ancestry.insert(cur); } + } - /* Add all our input paths to the chroot */ - for (auto & i : inputPaths) { - auto p = worker.store.printStorePath(i); - pathsInChroot[p] = p; - } + /* 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 = worker.store.storeDir; + while (cur.compare("/") != 0) { + ancestry.insert(cur); + cur = dirOf(cur); + } - /* 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"; - } + /* Add all our input paths to the chroot */ + for (auto & i : inputPaths) { + auto p = worker.store.printStorePath(i); + pathsInChroot[p] = p; + } - 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", worker.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"; - for (auto & i : pathsInChroot) { - 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 += additionalSandboxProfile; - } else - sandboxProfile += - #include "sandbox-minimal.sb" - ; - - debug("Generated sandbox profile:"); - debug(sandboxProfile); - - Path sandboxFile = tmpDir + "/.sandbox.sb"; - - writeFile(sandboxFile, sandboxProfile); - - bool allowLocalNetworking = parsedDrv->getBoolAttr("__darwinAllowLocalNetworking"); - - /* 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") { - builder = "/usr/bin/sandbox-exec"; - args.push_back("sandbox-exec"); - args.push_back("-f"); - args.push_back(sandboxFile); - args.push_back("-D"); - args.push_back("_GLOBAL_TMP_DIR=" + globalTmpDir); - if (allowLocalNetworking) { - args.push_back("-D"); - args.push_back(std::string("_ALLOW_LOCAL_NETWORKING=1")); - } - args.push_back(drv->builder); + /* 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 { - builder = drv->builder; - args.push_back(std::string(baseNameOf(drv->builder))); + 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", worker.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"; + for (auto & i : pathsInChroot) { + 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 += additionalSandboxProfile; + } else + sandboxProfile += + #include "sandbox-minimal.sb" + ; + + debug("Generated sandbox profile:"); + debug(sandboxProfile); + + bool allowLocalNetworking = parsedDrv->getBoolAttr("__darwinAllowLocalNetworking"); + + /* 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 (allowLocalNetworking) { + sandboxArgs.push_back("_ALLOW_LOCAL_NETWORKING"); + sandboxArgs.push_back("1"); + } + if (sandbox_init_with_parameters(sandboxProfile.c_str(), 0, stringsToCharPtrs(sandboxArgs).data(), NULL)) { + writeFull(STDERR_FILENO, "failed to configure sandbox\n"); + _exit(1); } } + + builder = drv->builder; + args.push_back(std::string(baseNameOf(drv->builder))); #else - else { + if (!drv->isBuiltin()) { builder = drv->builder; args.push_back(std::string(baseNameOf(drv->builder))); } From c43954ffac356b4168cbcfe2a67b4bad3f0dff5d Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 3 Oct 2024 12:44:12 +0200 Subject: [PATCH 3/5] local-derivation-goal: Print sandbox error detail on darwin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Théophane Hufschmitt --- src/libstore/unix/build/local-derivation-goal.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 878644fa5..0df1f0683 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2137,8 +2137,9 @@ void LocalDerivationGoal::runChild() sandboxArgs.push_back("_ALLOW_LOCAL_NETWORKING"); sandboxArgs.push_back("1"); } - if (sandbox_init_with_parameters(sandboxProfile.c_str(), 0, stringsToCharPtrs(sandboxArgs).data(), NULL)) { - writeFull(STDERR_FILENO, "failed to configure sandbox\n"); + 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); } } From 53b4bdcb8b0f114bea978cffbea325fd73f779b5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 3 Oct 2024 12:50:27 +0200 Subject: [PATCH 4/5] local-derivation-goal: Refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This works because the `builder` and `args` variables are only used in the non-builtin code path. Co-Authored-By: Théophane Hufschmitt --- src/libstore/unix/build/local-derivation-goal.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 0df1f0683..9e67283c9 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2143,15 +2143,12 @@ void LocalDerivationGoal::runChild() _exit(1); } } +#endif - builder = drv->builder; - args.push_back(std::string(baseNameOf(drv->builder))); -#else if (!drv->isBuiltin()) { builder = drv->builder; args.push_back(std::string(baseNameOf(drv->builder))); } -#endif for (auto & i : drv->args) args.push_back(rewriteStrings(i, inputRewrites)); From 67b5c7004302cbd344f63ccd306673a9adec4520 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 3 Oct 2024 12:57:00 +0200 Subject: [PATCH 5/5] local-derivation-goal: Move builder preparation to non-builtin code path --- .../unix/build/local-derivation-goal.cc | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 9e67283c9..1f4bafb56 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2016,11 +2016,6 @@ void LocalDerivationGoal::runChild() throw SysError("setuid failed"); } - /* Fill in the arguments. */ - Strings args; - - std::string builder = "invalid"; - #if __APPLE__ /* This has to appear before import statements. */ std::string sandboxProfile = "(version 1)\n"; @@ -2145,14 +2140,6 @@ void LocalDerivationGoal::runChild() } #endif - if (!drv->isBuiltin()) { - builder = drv->builder; - args.push_back(std::string(baseNameOf(drv->builder))); - } - - for (auto & i : drv->args) - args.push_back(rewriteStrings(i, inputRewrites)); - /* Indicate that we managed to set up the build environment. */ writeFull(STDERR_FILENO, std::string("\2\n")); @@ -2183,6 +2170,14 @@ void LocalDerivationGoal::runChild() } } + // Now builder is not builtin + + Strings args; + args.push_back(std::string(baseNameOf(drv->builder))); + + for (auto & i : drv->args) + args.push_back(rewriteStrings(i, inputRewrites)); + #if __APPLE__ posix_spawnattr_t attrp; @@ -2204,9 +2199,9 @@ void LocalDerivationGoal::runChild() posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL); } - posix_spawn(NULL, builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); + posix_spawn(NULL, drv->builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); #else - execve(builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); + execve(drv->builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); #endif throw SysError("executing '%1%'", drv->builder);