From 88b7db1ba455926868dd61f5ea39e454e5fb0433 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 30 Mar 2025 16:45:34 +0200 Subject: [PATCH] libstore: Don't default build-dir to temp-dir, store setting If a build directory is accessible to other users it is possible to smuggle data in and out of build directories. Usually this is only a build purity problem, but in combination with other issues it can be used to break out of a build sandbox. to prevent this we default to using a subdirectory of nixStateDir (which is more restrictive). (cherry picked from pennae Lix commit 55b416f6897fb0d8a9315a530a9b7f0914458ded) (store setting done by roberth) --- doc/manual/rl-next/build-dir-mandatory.md | 9 +++++ misc/systemd/nix-daemon.conf.in | 3 +- src/libstore/globals.cc | 1 + src/libstore/include/nix/store/globals.hh | 9 +---- src/libstore/include/nix/store/local-store.hh | 33 ++++++++++++++++++- src/libstore/local-store.cc | 12 +++++++ src/libstore/unix/build/derivation-builder.cc | 6 +++- .../build-remote-trustless-should-fail-0.sh | 1 - tests/functional/build-remote-trustless.sh | 1 - tests/functional/build-remote.sh | 1 - tests/functional/supplementary-groups.sh | 1 - 11 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 doc/manual/rl-next/build-dir-mandatory.md diff --git a/doc/manual/rl-next/build-dir-mandatory.md b/doc/manual/rl-next/build-dir-mandatory.md new file mode 100644 index 000000000..cb45a4315 --- /dev/null +++ b/doc/manual/rl-next/build-dir-mandatory.md @@ -0,0 +1,9 @@ +--- +synopsis: "`build-dir` no longer defaults to `$TMPDIR`" +--- + +The directory in which temporary build directories are created no longer defaults +to `TMPDIR` or `/tmp`, to avoid builders making their directories +world-accessible. This behavior allowed escaping the build sandbox and can +cause build impurities even when not used maliciously. We now default to `builds` +in `NIX_STATE_DIR` (which is `/nix/var/nix/builds` in the default configuration). diff --git a/misc/systemd/nix-daemon.conf.in b/misc/systemd/nix-daemon.conf.in index e7b264234..a0ddc4019 100644 --- a/misc/systemd/nix-daemon.conf.in +++ b/misc/systemd/nix-daemon.conf.in @@ -1 +1,2 @@ -d @localstatedir@/nix/daemon-socket 0755 root root - - +d @localstatedir@/nix/daemon-socket 0755 root root - - +d @localstatedir@/nix/builds 0755 root root 7d - diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index de5128347..721491def 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -6,6 +6,7 @@ #include "nix/util/abstract-setting-to-json.hh" #include "nix/util/compute-levels.hh" #include "nix/util/signals.hh" +#include "nix/util/types.hh" #include #include diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index b5157b4f4..0ac689b55 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -697,14 +697,7 @@ public: Setting> buildDir{this, std::nullopt, "build-dir", R"( - The directory on the host, in which derivations' temporary build directories are created. - - If not set, Nix uses the system temporary directory indicated by the `TMPDIR` environment variable. - Note that builds are often performed by the Nix daemon, so its `TMPDIR` is used, and not that of the Nix command line interface. - - This is also the location where [`--keep-failed`](@docroot@/command-ref/opt-common.md#opt-keep-failed) leaves its files. - - If Nix runs without sandbox, or if the platform does not support sandboxing with bind mounts (e.g. macOS), then the [`builder`](@docroot@/language/derivations.md#attr-builder)'s environment contains this directory instead of the virtual location [`sandbox-build-dir`](#conf-sandbox-build-dir). + Override the `build-dir` store setting for all stores that have this setting. )"}; Setting allowedImpureHostPrefixes{this, {}, "allowed-impure-host-deps", diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index 9a118fcc5..e52d51f75 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -34,7 +34,38 @@ struct OptimiseStats uint64_t bytesFreed = 0; }; -struct LocalStoreConfig : std::enable_shared_from_this, virtual LocalFSStoreConfig +struct LocalBuildStoreConfig : virtual LocalFSStoreConfig { + +private: + /** + Input for computing the build directory. See `getBuildDir()`. + */ + Setting> buildDir{this, std::nullopt, "build-dir", + R"( + The directory on the host, in which derivations' temporary build directories are created. + + If not set, Nix will use the `builds` subdirectory of its configured state directory. + + Note that builds are often performed by the Nix daemon, so its `build-dir` applies. + + Nix will create this directory automatically with suitable permissions if it does not exist. + Otherwise its permissions must allow all users to traverse the directory (i.e. it must have `o+x` set, in unix parlance) for non-sandboxed builds to work correctly. + + This is also the location where [`--keep-failed`](@docroot@/command-ref/opt-common.md#opt-keep-failed) leaves its files. + + If Nix runs without sandbox, or if the platform does not support sandboxing with bind mounts (e.g. macOS), then the [`builder`](@docroot@/language/derivations.md#attr-builder)'s environment will contain this directory, instead of the virtual location [`sandbox-build-dir`](#conf-sandbox-build-dir). + + > **Warning** + > + > `build-dir` must not be set to a world-writable directory. + > Placing temporary build directories in a world-writable place allows other users to access or modify build data that is currently in use. + > This alone is merely an impurity, but combined with another factor this has allowed malicious derivations to escape the build sandbox. + )"}; +public: + Path getBuildDir() const; +}; + +struct LocalStoreConfig : std::enable_shared_from_this, virtual LocalFSStoreConfig, virtual LocalBuildStoreConfig { using LocalFSStoreConfig::LocalFSStoreConfig; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9e1324262..e25a802ec 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -77,6 +77,18 @@ std::string LocalStoreConfig::doc() ; } +Path LocalBuildStoreConfig::getBuildDir() const +{ + if (settings.buildDir.get().has_value()) { + return *settings.buildDir.get(); + } + if (buildDir.get().has_value()) { + return *buildDir.get(); + } + + return stateDir.get() + "/builds"; +} + ref LocalStore::Config::openStore() const { return make_ref(ref{shared_from_this()}); diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index e2a148aeb..a036c95f1 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -725,9 +725,13 @@ void DerivationBuilderImpl::startBuilder() throw BuildError(msg); } + auto buildDir = getLocalStore(store).config->getBuildDir(); + + createDirs(buildDir); + /* Create a temporary directory where the build will take place. */ - topTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), 0700); + topTmpDir = createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), 0700); setBuildTmpDir(); assert(!tmpDir.empty()); diff --git a/tests/functional/build-remote-trustless-should-fail-0.sh b/tests/functional/build-remote-trustless-should-fail-0.sh index 3401de1b0..e79527d72 100755 --- a/tests/functional/build-remote-trustless-should-fail-0.sh +++ b/tests/functional/build-remote-trustless-should-fail-0.sh @@ -12,7 +12,6 @@ requiresUnprivilegedUserNamespaces [[ $busybox =~ busybox ]] || skipTest "no busybox" unset NIX_STORE_DIR -unset NIX_STATE_DIR # We first build a dependency of the derivation we eventually want to # build. diff --git a/tests/functional/build-remote-trustless.sh b/tests/functional/build-remote-trustless.sh index 9f91a91a9..6014b57bb 100644 --- a/tests/functional/build-remote-trustless.sh +++ b/tests/functional/build-remote-trustless.sh @@ -9,7 +9,6 @@ requiresUnprivilegedUserNamespaces [[ "$busybox" =~ busybox ]] || skipTest "no busybox" unset NIX_STORE_DIR -unset NIX_STATE_DIR remoteDir=$TEST_ROOT/remote diff --git a/tests/functional/build-remote.sh b/tests/functional/build-remote.sh index 765cd71b4..f396bc72e 100644 --- a/tests/functional/build-remote.sh +++ b/tests/functional/build-remote.sh @@ -8,7 +8,6 @@ requiresUnprivilegedUserNamespaces # Avoid store dir being inside sandbox build-dir unset NIX_STORE_DIR -unset NIX_STATE_DIR function join_by { local d=$1; shift; echo -n "$1"; shift; printf "%s" "${@/#/$d}"; } diff --git a/tests/functional/supplementary-groups.sh b/tests/functional/supplementary-groups.sh index 400333f7d..a667d3e99 100755 --- a/tests/functional/supplementary-groups.sh +++ b/tests/functional/supplementary-groups.sh @@ -14,7 +14,6 @@ execUnshare <