From 8c20f0fc335410627e4bfbafc8b29058b39de5c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 2 Apr 2024 17:06:48 +0200 Subject: [PATCH] Run the builds in a daemon-controled directory Instead of running the builds under `$TMPDIR/{unique-build-directory-owned-by-the-build-user}`, run them under `$TMPDIR/{unique-build-directory-owned-by-the-daemon}/{subdir-owned-by-the-build-user}` where the build directory is only readable and traversable by the daemon user. This achieves two things: 1. It prevents builders from making their build directory world-readable (or even writeable), which would allow the outside world to interact with them. 2. It prevents external processes running as the build user (either because that somehow leaked, maybe as a consequence of 1., or because `build-users` isn't in use) from gaining access to the build directory. (cherry picked from commit 1d3696f0fb88d610abc234a60e0d6d424feafdf1) --- .../unix/build/local-derivation-goal.cc | 5 +++-- src/libutil/file-system.cc | 5 +++++ src/libutil/file-system.hh | 5 +++++ tests/functional/check.sh | 2 +- tests/nixos/user-sandboxing/default.nix | 20 ++++++++++--------- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 72125cb82..3544759bb 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -501,8 +501,9 @@ void LocalDerivationGoal::startBuilder() /* Create a temporary directory where the build will take place. */ - tmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), false, false, 0700); - + auto parentTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), false, false, 0700); + tmpDir = parentTmpDir + "/build"; + createDir(tmpDir, 0700); chownToBuilder(tmpDir); for (auto & [outputName, status] : initialOutputs) { diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index b03bb767b..989f6a4c8 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -469,6 +469,11 @@ void deletePath(const Path & path) deletePath(path, dummy); } +void createDir(const Path &path, mode_t mode) +{ + if (mkdir(path.c_str(), mode) == -1) + throw SysError("creating directory '%1%'", path); +} Paths createDirs(const Path & path) { diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 0c4e7cfdd..c393b8573 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -184,6 +184,11 @@ inline Paths createDirs(PathView path) return createDirs(Path(path)); } +/** + * Create a single directory. + */ +void createDir(const Path & path, mode_t mode = 0755); + /** * Create a symlink. */ diff --git a/tests/functional/check.sh b/tests/functional/check.sh index 38883c5d7..95e5a8b8c 100644 --- a/tests/functional/check.sh +++ b/tests/functional/check.sh @@ -44,7 +44,7 @@ test_custom_build_dir() { --no-out-link --keep-failed --option build-dir "$TEST_ROOT/custom-build-dir" 2> $TEST_ROOT/log || status=$? [ "$status" = "100" ] [[ 1 == "$(count "$customBuildDir/nix-build-"*)" ]] - local buildDir="$customBuildDir/nix-build-"* + local buildDir="$customBuildDir/nix-build-"*"/build" grep $checkBuildId $buildDir/checkBuildId } test_custom_build_dir diff --git a/tests/nixos/user-sandboxing/default.nix b/tests/nixos/user-sandboxing/default.nix index cdb0c7eb6..8a16f44e8 100644 --- a/tests/nixos/user-sandboxing/default.nix +++ b/tests/nixos/user-sandboxing/default.nix @@ -16,6 +16,8 @@ let set -x chmod 700 . + # Shouldn't be able to open the root build directory + (! chmod 700 ..) touch foo @@ -85,15 +87,15 @@ in # Wait for the build to be ready # This is OK because it runs as root, so we can access everything - machine.wait_for_file("/tmp/nix-build-open-build-dir.drv-0/syncPoint") + machine.wait_for_file("/tmp/nix-build-open-build-dir.drv-0/build/syncPoint") # But Alice shouldn't be able to access the build directory - machine.fail("su alice -c 'ls /tmp/nix-build-open-build-dir.drv-0'") - machine.fail("su alice -c 'touch /tmp/nix-build-open-build-dir.drv-0/bar'") - machine.fail("su alice -c 'cat /tmp/nix-build-open-build-dir.drv-0/foo'") + machine.fail("su alice -c 'ls /tmp/nix-build-open-build-dir.drv-0/build'") + machine.fail("su alice -c 'touch /tmp/nix-build-open-build-dir.drv-0/build/bar'") + machine.fail("su alice -c 'cat /tmp/nix-build-open-build-dir.drv-0/build/foo'") # Tell the user to finish the build - machine.succeed("echo foo > /tmp/nix-build-open-build-dir.drv-0/syncPoint") + machine.succeed("echo foo > /tmp/nix-build-open-build-dir.drv-0/build/syncPoint") with subtest("Being able to execute stuff as the build user doesn't give access to the build dir"): machine.succeed(r""" @@ -105,16 +107,16 @@ in args = [ (builtins.storePath "${create-hello-world}") ]; }' >&2 & """.strip()) - machine.wait_for_file("/tmp/nix-build-innocent.drv-0/syncPoint") + machine.wait_for_file("/tmp/nix-build-innocent.drv-0/build/syncPoint") # The build ran as `nixbld1` (which is the only build user on the # machine), but a process running as `nixbld1` outside the sandbox # shouldn't be able to touch the build directory regardless - machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'ls /tmp/nix-build-innocent.drv-0'") - machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'echo pwned > /tmp/nix-build-innocent.drv-0/result'") + machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'ls /tmp/nix-build-innocent.drv-0/build'") + machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'echo pwned > /tmp/nix-build-innocent.drv-0/build/result'") # Finish the build - machine.succeed("echo foo > /tmp/nix-build-innocent.drv-0/syncPoint") + machine.succeed("echo foo > /tmp/nix-build-innocent.drv-0/build/syncPoint") # Check that the build was not affected machine.succeed(r"""