diff --git a/.circleci/config.yml b/.circleci/config.yml index 4e07834bf9..188da5f951 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -316,6 +316,143 @@ jobs: at: /tmp/workspace - run: ./.circleci/release.sh + # The sandbox tests (//tools/sandbox/test:sandbox_test) create user, mount, + # and network namespaces and mount within them. The docker executor denies + # mount inside the job container and doesn't expose any configuration to + # allow it. Instead, these workflows run the sandbox tests on machine + # executors with rootless podman. + # + # podman's default seccomp profile allows unprivileged namespace and mount + # syscalls, and the kernel gates them on CAP_SYS_ADMIN in the caller's user + # namespace, which the sandbox creates for itself, so no custom seccomp + # profile and no CAP_SYS_ADMIN are needed. The container only needs + # SETUID/SETGID (new*idmap writing multi-range maps) and SETFCAP (mapping + # parent uid 0 into a user namespace requires it, see user_namespaces(7)). + # unmask=ALL is required so a /proc can be mounted in the nested mount + # namespace (the kernel refuses while runtime /proc masks are in place). + sandbox-test-linux: + working_directory: ~/please + machine: + image: ubuntu-2404:current + resource_class: medium + steps: + - checkout + - run: + name: Set up rootless podman + command: | + sudo apt-get update + sudo apt-get install -y podman catatonit uidmap passt slirp4netns + grep -q "^$(id -un):" /etc/subuid || sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 "$(id -un)" + # The job shell has no systemd user session or D-Bus, so podman's + # default systemd cgroup manager fails to create transient scopes + # ("Interactive authentication required"); use cgroupfs instead. + mkdir -p ~/.config/containers + printf '[engine]\ncgroup_manager = "cgroupfs"\n' > ~/.config/containers/containers.conf + podman system migrate || true + - run: + name: Run sandbox tests + command: | + podman run --rm --init \ + --security-opt unmask=ALL \ + --cap-add SETUID --cap-add SETGID --cap-add SETFCAP \ + -v "$PWD:/please" -w /please \ + -e PLZ_ARGS="--profile ci" \ + ghcr.io/thought-machine/please_ubuntu:20260318 \ + sh -ec ' + # Mirrors the tools/images Dockerfile changes; drop once new + # image tags containing them are published and referenced. + apt-get update && apt-get install -y uidmap + printf "root:0:65536\n" >> /etc/subuid + printf "root:0:65536\n" >> /etc/subgid + ./pleasew -p -v2 test --rerun --test_results_file plz-out/results/sandbox/test_results.xml --log_file plz-out/log/sandbox_test.log //tools/sandbox/test:sandbox_test + ' + - store_test_results: + path: plz-out/results + - store_artifacts: + path: plz-out/log + + sandbox-test-linux-alt: + working_directory: ~/please + machine: + image: ubuntu-2404:current + resource_class: medium + steps: + - checkout + - run: + name: Set up rootless podman + command: | + sudo apt-get update + sudo apt-get install -y podman catatonit uidmap passt slirp4netns + grep -q "^$(id -un):" /etc/subuid || sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 "$(id -un)" + # The job shell has no systemd user session or D-Bus, so podman's + # default systemd cgroup manager fails to create transient scopes + # ("Interactive authentication required"); use cgroupfs instead. + mkdir -p ~/.config/containers + printf '[engine]\ncgroup_manager = "cgroupfs"\n' > ~/.config/containers/containers.conf + podman system migrate || true + - run: + name: Run sandbox tests + command: | + podman run --rm --init \ + --security-opt unmask=ALL \ + --cap-add SETUID --cap-add SETGID --cap-add SETFCAP \ + -v "$PWD:/please" -w /please \ + -e PLZ_ARGS="--profile ci-alt" \ + ghcr.io/thought-machine/please_ubuntu_alt:20260318 \ + sh -ec ' + # Mirrors the tools/images Dockerfile changes; drop once new + # image tags containing them are published and referenced. + apt-get update && apt-get install -y uidmap + printf "root:0:65536\n" >> /etc/subuid + printf "root:0:65536\n" >> /etc/subgid + ./pleasew -p -v2 test --rerun --test_results_file plz-out/results/sandbox/test_results.xml --log_file plz-out/log/sandbox_test.log //tools/sandbox/test:sandbox_test + ' + - store_test_results: + path: plz-out/results + - store_artifacts: + path: plz-out/log + + sandbox-test-alpine: + working_directory: ~/please + machine: + image: ubuntu-2404:current + resource_class: medium + steps: + - checkout + - run: + name: Set up rootless podman + command: | + sudo apt-get update + sudo apt-get install -y podman catatonit uidmap passt slirp4netns + grep -q "^$(id -un):" /etc/subuid || sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 "$(id -un)" + # The job shell has no systemd user session or D-Bus, so podman's + # default systemd cgroup manager fails to create transient scopes + # ("Interactive authentication required"); use cgroupfs instead. + mkdir -p ~/.config/containers + printf '[engine]\ncgroup_manager = "cgroupfs"\n' > ~/.config/containers/containers.conf + podman system migrate || true + - run: + name: Run sandbox tests + command: | + podman run --rm --init \ + --security-opt unmask=ALL \ + --cap-add SETUID --cap-add SETGID --cap-add SETFCAP \ + -v "$PWD:/please" -w /please \ + -e PLZ_ARGS="--profile ci --profile alpine --exclude no-musl" \ + ghcr.io/thought-machine/please_alpine:20260318 \ + sh -ec ' + # Mirrors the tools/images Dockerfile changes; drop once new + # image tags containing them are published and referenced. + apk add --no-cache shadow-uidmap curl + printf "root:0:65536\n" >> /etc/subuid + printf "root:0:65536\n" >> /etc/subgid + ./pleasew -p -v2 test --rerun --test_results_file plz-out/results/sandbox/test_results.xml --log_file plz-out/log/sandbox_test.log //tools/sandbox/test:sandbox_test + ' + - store_test_results: + path: plz-out/results + - store_artifacts: + path: plz-out/log + # Runs a benchmarking test and records some performance results. perf-test: docker: @@ -337,6 +474,9 @@ workflows: - build-alpine - build-linux - build-linux-alt + - sandbox-test-linux + - sandbox-test-linux-alt + - sandbox-test-alpine - build-darwin: requires: - build-alpine @@ -366,6 +506,9 @@ workflows: - build-darwin - test-rex - test-http-cache + - sandbox-test-linux + - sandbox-test-linux-alt + - sandbox-test-alpine filters: branches: only: master diff --git a/.mailmap b/.mailmap new file mode 100644 index 0000000000..c19cac55e7 --- /dev/null +++ b/.mailmap @@ -0,0 +1,2 @@ +Grant Zvolsky +Grant Zvolsky diff --git a/tools/images/alpine/Dockerfile b/tools/images/alpine/Dockerfile index bbb7889242..d865af371f 100644 --- a/tools/images/alpine/Dockerfile +++ b/tools/images/alpine/Dockerfile @@ -2,7 +2,11 @@ FROM golang:1.26-alpine LABEL org.opencontainers.image.authors="please thoughtmachine net" LABEL org.opencontainers.image.source=https://github.com/thought-machine/please -RUN apk add --no-cache git patch gcc g++ libc-dev bash libgcc xz protoc protobuf-dev perl-utils +RUN apk add --no-cache git patch gcc g++ libc-dev bash libgcc xz protoc protobuf-dev perl-utils shadow-uidmap curl + +# Subordinate id delegation for sandbox tests +RUN printf 'root:0:65536\n' >> /etc/subuid && \ + printf 'root:0:65536\n' >> /etc/subgid # Ensure this is where we expect on the PATH RUN ln -s /usr/local/go/bin/go /usr/local/bin/go diff --git a/tools/images/ubuntu/Dockerfile b/tools/images/ubuntu/Dockerfile index 57f6d22ca7..cf05c3dfe9 100644 --- a/tools/images/ubuntu/Dockerfile +++ b/tools/images/ubuntu/Dockerfile @@ -12,9 +12,13 @@ RUN truncate -s0 /tmp/preseed.cfg; \ apt-get update && \ apt-get install -y python3 python3-dev python3-pip time \ curl unzip git locales pkg-config zlib1g-dev psmisc \ - openssh-client ca-certificates && \ + openssh-client ca-certificates uidmap && \ apt-get clean +# Subordinate id delegation for sandbox tests +RUN printf 'root:0:65536\n' >> /etc/subuid && \ + printf 'root:0:65536\n' >> /etc/subgid + # Go - we want a specific package version here. RUN curl -fsSL https://dl.google.com/go/go1.26.1.linux-amd64.tar.gz | tar -xzC /usr/local RUN ln -s /usr/local/go/bin/go /usr/local/bin/go && ln -s /usr/local/go/bin/gofmt /usr/local/bin/gofmt diff --git a/tools/images/ubuntu_alt/Dockerfile b/tools/images/ubuntu_alt/Dockerfile index d11e01bb06..7551a1bf18 100644 --- a/tools/images/ubuntu_alt/Dockerfile +++ b/tools/images/ubuntu_alt/Dockerfile @@ -7,9 +7,13 @@ ENV DEBIAN_FRONTEND noninteractive # Most dependencies; Python, Clang RUN apt-get update && \ apt-get install -y python3 python3-dev python3-pip \ - curl unzip git locales pkg-config zlib1g-dev clang && \ + curl unzip git locales pkg-config zlib1g-dev clang uidmap && \ apt-get clean +# Subordinate id delegation for sandbox tests +RUN printf 'root:0:65536\n' >> /etc/subuid && \ + printf 'root:0:65536\n' >> /etc/subgid + # Go RUN curl -fsSL https://dl.google.com/go/go1.26.1.linux-amd64.tar.gz | tar -xzC /usr/local RUN ln -s /usr/local/go/bin/go /usr/local/bin/go && ln -s /usr/local/go/bin/gofmt /usr/local/bin/gofmt diff --git a/tools/sandbox/BUILD b/tools/sandbox/BUILD index e65f547892..df91bb1506 100644 --- a/tools/sandbox/BUILD +++ b/tools/sandbox/BUILD @@ -14,14 +14,6 @@ c_binary( deps = [":sandbox"], ) -c_binary( - name = "nonet_sandbox", - srcs = ["nonet_main.c"], - static = (CONFIG.get("STATIC_SANDBOX") is not None), - visibility = ["PUBLIC"], - deps = [":sandbox"], -) - cc_test( name = "sandbox_test", srcs = ["sandbox_test.cc"], diff --git a/tools/sandbox/README.md b/tools/sandbox/README.md new file mode 100644 index 0000000000..c0a6d73c4b --- /dev/null +++ b/tools/sandbox/README.md @@ -0,0 +1,92 @@ +# please_sandbox + +> [!CAUTION] +> The Please Sandbox is not a security boundary. It is not designed to run untrusted or malicious +> code. + +`please_sandbox` is a wrapper that allows running a given binary in Linux namespaces. By default, it +creates PID, IPC, UTS, user, mount and network namespaces. It also does a bunch of things on the +filesystem: + +- if `TMP_DIR` is not set or not under `/tmp`, a tmpfs is mounted over `/tmp` and `TMPDIR` is set to + `/tmp`. If it is set, `$TMP_DIR` is bind mounted onto `/tmp/plz_sandbox`, which becomes the + working directory, and the root filesystem is remounted read-only; +- if `SANDBOX_DIRS` is set, we expect a comma-separated list of path that will be hidden with a + tmpfs; +- if `SANDBOX_FILE_MOUNTS` is set, we expect it to be set to a comma-separated list of key-value + pairs in the following format: `key:value`. Keys must point to existing paths and will be bind + mounted to the path given as value; +- if `SANDBOX_UID_MAP` and `SANDBOX_GID_MAP` are set (both are required if either is), we pass + these arguments to newuidmap/newgidmap to configure uid/gid mappings. The format is 1..n + space-delimited triples of [id lowerid count], see `man newuidmap`; + +Mount and network namespaces can be disabled setting the `SHARE_MOUNT` and `SHARE_NETWORK` +environment variables to `1`. Remounting of `/proc` can be disabled by setting `MOUNT_PROC=0`, +e.g. for systems that don't allow mounting a full `/proc` from a new user namespace (see the +mount namespace notes below). + +When the network namespace is used, the loopback interface is brought up with an additional IP +address, which defaults to 10.1.1.1. The `SANDBOX_LOCAL_IP` environment variable can be used +to change this IP, and setting it to the empty string disables the extra address entirely. + +## Using the knobs with Please + +Please invokes the tool configured in `[sandbox] tool` with the command to run as its arguments, +and sets `SHARE_NETWORK` and `SHARE_MOUNT` itself according to the rule being run. Remote +execution workers invoke the sandbox tool themselves and should set these variables explicitly, +since the binary's defaults apply when they are absent. To set the other knobs, or to override +Please's per-rule choices, point `tool` at a thin wrapper script: + +```sh +#!/bin/sh +# please_sandbox, but without remounting /proc. +export MOUNT_PROC=0 +exec /path/to/please_sandbox "$@" +``` + +```ini +[sandbox] +tool = /path/to/noproc_sandbox_wrapper +``` + +## UID/GID mapping in user namespace + +By default, the sandbox only maps the real UID/GID of the user running the sandbox into the namespace. +However, `SANDBOX_UID_MAP` and `SANDBOX_GID_MAP` may be used to define arguments that are passed to +`new*idmap`. + +The example below will map the real UID of the user running the sandbox to root and UIDs +from range [100000;165536) to [1;65536) in the child namespace (assuming your UID is 1000): + +```bash +$ TMP_DIR=/tmp SANDBOX_UID_MAP="0 $UID 1 1 100000 65536" SANDBOX_GID_MAP="0 $(id -g) 1" please_sandbox cat /proc/self/uid_map + 0 1000 1 + 1 100000 65536 +``` + +## Capabilities and other requirements + +### Namespaces + +Historically, creating Mount, PID, IPC, UTS, and Network namespaces required the heavily overloaded +`CAP_SYS_ADMIN` capability on the host system. However, since Linux 3.8, unprivileged processes can +use User Namespaces to obtain local `CAP_SYS_ADMIN` privileges. This allows processes to create +Mount, PID, IPC, UTS, and Network namespaces without needing host-level root or `CAP_SYS_ADMIN` +privileges. + +#### Mount namespace + +In addition to the above, when enabling the mount namespace, the sandbox will remount /proc, so any +process that use it will have access to accurate information of the PID namespace (otherwise they'd +still have access to /proc from the parent namespace). There's however a specific edge case in the +Linux kernel, that prevents /proc from being remounted in a mount namespace, when the parent /proc +is not fully visible, [see this commit](https://github.com/torvalds/linux/commit/1b852bceb0d1). + +This is an issue with most container runtimes (and therefore Kubernetes), as by default, they will +hide some part of /proc in a container to reduce attack surface, eg +[see this docker PR](https://github.com/docker/cli/pull/1808). + +Users of the sandbox have different options: +- unmask /proc (k8s pod `procMount: Unmasked` securityContext option combined with `hostUsers: false`), +- disable the /proc isolation (`MOUNT_PROC=0`), in which case the sandboxed process will be able to see (but not send signals to) processes outside the sandbox, +- disable the mount namespace entirely with `SHARE_MOUNT=1`. diff --git a/tools/sandbox/main.c b/tools/sandbox/main.c index 42abd96724..05283897b1 100644 --- a/tools/sandbox/main.c +++ b/tools/sandbox/main.c @@ -1,5 +1,5 @@ // please_sandbox is a very small binary to implement sandboxing -// of tests (and possibly other build actions) via cgroups. +// of tests (and possibly other build actions) via namespaces. // Essentially this is a very lightweight replacement for Docker // where we would use it for tests to avoid port clashes etc. // @@ -27,5 +27,9 @@ int main(int argc, char* argv[]) { const char* share_mount_env = getenv("SHARE_MOUNT"); const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); - return contain(&argv[1], unshare_network, unshare_mount); + // /proc is remounted by default but it can be opted out if `MOUNT_PROC=0` env is set + const char* mount_proc_env = getenv("MOUNT_PROC"); + const bool mount_proc = mount_proc_env == NULL || strcmp(mount_proc_env, "0"); + + return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, mount_proc); } diff --git a/tools/sandbox/nonet_main.c b/tools/sandbox/nonet_main.c deleted file mode 100644 index efedbfcecd..0000000000 --- a/tools/sandbox/nonet_main.c +++ /dev/null @@ -1,17 +0,0 @@ -// nonet_sandbox is a slightly modified version of please_sandbox that does all the same -// things except it leaves the network unscathed. -// It is currently not used, but is conceptually useful to sandbox rules that request sandbox -// disabling in order to gain network access (which is by far the most common case for that), -// but it's still useful to contain the other namespaces. -#include -#include "tools/sandbox/sandbox.h" - -int main(int argc, char* argv[]) { - if (argc < 2) { - fputs("nonet_sandbox implements limited sandboxing via Linux namespaces.\n", stderr); - fputs("It takes no flags, it simply executes the command given as arguments.\n", stderr); - fputs("Usage: nonet_sandbox command args...\n", stderr); - return 1; - } - return contain(&argv[1], false, true); -} diff --git a/tools/sandbox/sandbox.c b/tools/sandbox/sandbox.c index 0866d64c92..52c8f421a2 100644 --- a/tools/sandbox/sandbox.c +++ b/tools/sandbox/sandbox.c @@ -1,6 +1,8 @@ #include "tools/sandbox/sandbox.h" #define _GNU_SOURCE +#include +#include #include #include #include @@ -20,8 +22,12 @@ #include #include #include -#include +#include +#include #include +#include + +static int cloned_pid; int perror_sock(char *errmsg, const int sock) { close(sock); @@ -44,15 +50,14 @@ int lo_up() { memset(&req, 0, sizeof(req)); strncpy(req.ifr_name, "lo", IFNAMSIZ); if (ioctl(sock, SIOCGIFFLAGS, &req) < 0) { - perror_sock("SIOCGIFFLAGS", sock); - return 1; + return perror_sock("SIOCGIFFLAGS", sock); } req.ifr_flags |= IFF_UP; if (ioctl(sock, SIOCSIFFLAGS, &req) < 0) { - perror_sock("SIOCSIFFLAGS", sock); - return 1; + return perror_sock("SIOCSIFFLAGS", sock); } + close(sock); return 0; } @@ -99,6 +104,14 @@ int default_gateway() { // 127.0.0.0/8 range int add_local_ip() { + // SANDBOX_LOCAL_IP overrides the default address; defined empty string disables it. + const char* local_ip = getenv("SANDBOX_LOCAL_IP"); + if (local_ip == NULL) { + local_ip = "10.1.1.1"; + } else if (local_ip[0] == '\0') { + return 0; + } + const int sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE); if (sock < 0) { perror("socket"); @@ -131,7 +144,12 @@ int add_local_ip() req.rta.rta_type = IFA_LOCAL; req.rta.rta_len = RTA_LENGTH(sizeof(req.addr)); - req.addr = inet_addr("10.1.1.1"); + in_addr_t ip_addr = inet_addr(local_ip); + if (ip_addr == INADDR_NONE) { + fprintf(stderr, "Invalid IP address provided for SANDBOX_LOCAL_IP: %s\n", local_ip); + return 1; + } + req.addr = ip_addr; if (send(sock, &req, req.nh.nlmsg_len, 0) < 0) { return perror_sock("send", sock); @@ -141,35 +159,116 @@ int add_local_ip() return 0; } -// deny_groups disables the ability to call setgroups(2). This is required -// before we can successfully write to gid_map in map_ids. -int deny_groups() { - FILE* f = fopen("/proc/self/setgroups", "w"); +// map_ids maps root inside the namespace to the user and group ID of the user +// running the sandbox. It also maps user-defined id ranges via `SANDBOX_*ID_MAP`. +// Without this we fail to create directories in the tmpfs with an EOVERFLOW. +int map_ids(const pid_t child, const char *path, const char *map) { + const size_t child_size = snprintf(NULL, 0, "%d", child); + char child_pid[child_size+1]; + + snprintf(child_pid, child_size+1, "%d", child); + + char *argv[256]; + int argc = 0; + argv[argc++] = (char*)path; + argv[argc++] = child_pid; + + // strtok_r mutates its argument, so the input to this function should be copied + char *map_copy = NULL; + if (map != NULL) { + map_copy = strdup(map); + if (!map_copy) { + perror("strdup"); + return 1; + } + char *saveptr; + char *token = strtok_r(map_copy, " \t\n", &saveptr); + while (token != NULL && argc < 255) { + argv[argc++] = token; + token = strtok_r(NULL, " \t\n", &saveptr); + } + if (token != NULL) { + fprintf(stderr, "too many arguments for map_ids (max 255)\n"); + free(map_copy); + return 1; + } + } + argv[argc] = NULL; + + const pid_t pid = fork(); + if (pid == -1) { + perror("fork"); + if (map_copy) { + free(map_copy); + } + return 1; + } else if (pid == 0) { + execvp(path, argv); + perror(path); + _exit(1); + } + + if (map_copy) { + free(map_copy); + } + + int status; + if (waitpid(pid, &status, 0) == -1) { + perror("waitpid failed"); + return 1; + } + if (WIFEXITED(status)) { + return WEXITSTATUS(status); + } else if (WIFSIGNALED(status)) { + kill(getpid(), WTERMSIG(status)); + } + return 1; +} + +// deny_setgroups writes "deny" to /proc//setgroups. +// This is required by the kernel before an unprivileged user can write to gid_map. +int deny_setgroups(pid_t pid) { + char path[128]; + snprintf(path, sizeof(path), "/proc/%d/setgroups", pid); + + FILE *f = fopen(path, "w"); if (!f) { - perror("fopen /proc/self/setgroups"); + perror("fopen setgroups"); return 1; } - if (fputs("deny\n", f) < 0) { - perror("fputs"); + + if (fputs("deny", f) < 0) { + perror("fputs setgroups"); + fclose(f); + return 1; + } + + if (fclose(f) != 0) { + perror("fclose setgroups"); return 1; } - return fclose(f); + return 0; } -// map_ids maps the user id or group id inside the namespace to those outside. -// Without this we fail to create directories in the tmpfs with an EOVERFLOW. -int map_ids(int out_id, const char* path) { - FILE* f = fopen(path, "w"); +// write_id_map writes a 1-to-1 mapping directly to /proc// +int write_id_map(pid_t pid, const char *file, uid_t inside_id, uid_t outside_id) { + char path[128]; + snprintf(path, sizeof(path), "/proc/%d/%s", pid, file); + + FILE *f = fopen(path, "w"); if (!f) { - perror("fopen"); + perror("fopen map"); return 1; } - if (fprintf(f, "%d %d 1\n", out_id, out_id) < 0) { - perror("fprintf"); + + if (fprintf(f, "%u %u 1\n", inside_id, outside_id) < 0) { + perror("fprintf map"); + fclose(f); return 1; } + if (fclose(f) != 0) { - perror("fclose"); + perror("fclose map"); return 1; } return 0; @@ -179,30 +278,37 @@ int map_ids(int out_id, const char* path) { // bind mounts the test directory to /tmp/plz_sandbox. // If the given string pointer (the argv[0] of the new process) is within the old temp dir // then it will be replaced with a new version pointing into the new sandbox dir. -int mount_tmp(char** argv0) { +int mount_tmp(char** argv0, bool sandbox_dir) { // Don't mount on /tmp if our tmp dir is under there, otherwise we won't be able to see it. const char* dir = getenv("TMP_DIR"); const char* d = "/tmp/plz_sandbox"; - if (dir) { - if (strncmp(dir, "/tmp/", 5) == 0) { - fputs("Not mounting tmpfs on /tmp since TMP_DIR is a subdir\n", stderr); - return 0; - } - } + // Remounting / as private is necessary so that the tmpfs mount isn't visible to anyone else. if (mount("none", "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0) { perror("remount"); return 1; } const int flags = MS_LAZYTIME | MS_NOATIME | MS_NODEV | MS_NOSUID; - if (mount("tmpfs", "/tmp", "tmpfs", flags, NULL) != 0) { - perror("mount"); - return 1; + // Skip the tmpfs when TMP_DIR is /tmp itself or lives under it. + if (!dir || (strcmp(dir, "/tmp") != 0 && strncmp(dir, "/tmp/", 5) != 0)) { + if (mount("tmpfs", "/tmp", "tmpfs", flags, NULL) != 0) { + perror("mount"); + return 1; + } + if (setenv("TMPDIR", "/tmp", 1) != 0) { + perror("setenv"); + return 1; + } } - if (setenv("TMPDIR", "/tmp", 1) != 0) { - perror("setenv"); - return 1; + + // Mount over /dev/shm as well so nothing can be inadvertently shared through it and we'll clean it up. + if (mount("tmpfs", "/dev/shm", "tmpfs", flags, NULL) != 0) { + if (errno != ENOENT) { // tolerate absent /dev/shm + perror("mount /dev/shm"); + return 1; + } } + // If SANDBOX_DIRS is set, we expect a comma-separated list of directories to mount a tmpfs over in order to hide them. // If one or more directories don't exist, that is OK, but any other error is fatal. char* dirs = getenv("SANDBOX_DIRS"); @@ -223,15 +329,18 @@ int mount_tmp(char** argv0) { // Remove the env var; downstream things don't need to know what these were. unsetenv("SANDBOX_DIRS"); } + if (!sandbox_dir) { + return 0; + } if (!dir) { fputs("TMP_DIR not set, will not bind-mount to /tmp/plz_sandbox\n", stderr); return 0; } - if (mkdir(d, S_IRWXU) != 0) { + if (mkdir(d, S_IRWXU) != 0 && errno != EEXIST) { perror("mkdir /tmp/plz_sandbox"); return 1; } - if (mount(dir, d, "", MS_BIND, NULL) != 0) { + if (mount(dir, d, "", MS_BIND|MS_REC, NULL) != 0) { perror("bind mount"); return 1; } @@ -242,8 +351,48 @@ int mount_tmp(char** argv0) { perror("setenv"); return 1; } + + // If SANDBOX_FILE_MOUNTS is set, we expect a comma-separated list of key:value pairs to mount files into the new tree. + // Currently all these are expected to exist. + char* files = getenv("SANDBOX_FILE_MOUNTS"); + if (files != NULL) { + char *token = strtok(files, ","); + while(token) { + char* separator = strchr(token, ':'); + if (!separator) { + fprintf(stderr, "Invalid sandbox file mount: %s\n", token); + } else { + *separator = '\0'; + if (mount(token, separator+1, "", MS_RDONLY | MS_BIND, NULL) != 0) { + perror("bind mount sandbox file"); + return 1; + } + } + token = strtok(NULL, ","); + } + unsetenv("SANDBOX_FILE_MOUNTS"); + } + // Now make root readonly (once we have bind-mounted in the non-readonly workdir) - if (mount("none", "/", NULL, MS_REMOUNT | MS_RDONLY | MS_BIND, NULL) != 0) { + struct statvfs st; + if (statvfs("/", &st) != 0) { + perror("statvfs /"); + return 1; + } + unsigned long remount_flags = MS_REMOUNT | MS_RDONLY | MS_BIND; + if (st.f_flag & ST_NOSUID) { + remount_flags |= MS_NOSUID; + } + if (st.f_flag & ST_NODEV) { + remount_flags |= MS_NODEV; + } + if (st.f_flag & ST_NOEXEC) { + remount_flags |= MS_NOEXEC; + } + if (st.f_flag & ST_NOATIME) { + remount_flags |= MS_NOATIME; + } + if (mount("none", "/", NULL, remount_flags, NULL) != 0) { perror("remount ro"); return 1; } @@ -263,7 +412,7 @@ int mount_tmpfs(const char* dir) { // mount_proc mounts a new procfs on /proc int mount_proc() { - if (mount("proc", "/proc", "proc", 0, NULL) != 0) { + if (mount("proc", "/proc", "proc", MS_NODEV | MS_NOSUID | MS_NOEXEC | MS_RDONLY, NULL) != 0) { perror("mount proc"); return 1; } @@ -275,25 +424,59 @@ typedef struct _clone_arg { uid_t gid; bool net; bool mount; + bool sandbox_dir; + bool mount_proc; + int sync_fd[2]; char** argv; } clone_arg; +int set_parent_uid(uid_t parent) { + const size_t uid_size = snprintf(NULL, 0, "%d", parent); + char uid[uid_size+1]; + + snprintf(uid, uid_size+1, "%d", parent); + if (setenv("PARENT_UID", uid, 1) != 0) { + perror("setenv"); + return 1; + } + return 0; +} + // contain_child is the entrypoint for the child process. int contain_child(void* p) { clone_arg* arg = p; - if (deny_groups() != 0) { + + // This process is PID 1 in the new namespace. SIGKILL is required to kill it + // with default signal handlers. Graceful termination of a live parent is + // handled separately by forward_sigterm. + if (prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) { + perror("failed to set PDEATHSIG"); + return 1; + } + + char c; + close(arg->sync_fd[1]); // close unused write end + ssize_t n = read(arg->sync_fd[0], &c, 1); // block until parent sends a byte or closes + if (n < 0) { + perror("read sync pipe"); + return 1; + } else if (n == 0) { + fprintf(stderr, "sandbox parent died before completing setup\n"); return 1; } - if (map_ids(arg->uid, "/proc/self/uid_map") != 0 || - map_ids(arg->gid, "/proc/self/gid_map") != 0) { + close(arg->sync_fd[0]); + + if (set_parent_uid(arg->uid) != 0) { return 1; } if (arg->mount) { - if (mount_tmp(&arg->argv[0]) != 0) { + if (mount_tmp(&arg->argv[0], arg->sandbox_dir) != 0) { return 1; } - if (mount_proc() != 0) { - return 1; + if (arg->mount_proc) { + if (mount_proc() != 0) { + return 1; + } } } if (arg->net) { @@ -301,24 +484,32 @@ int contain_child(void* p) { return 1; } } - if (prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) { - perror("failed to set PDEATHSIG"); - return 1; - } execvp(arg->argv[0], arg->argv); // If this returns, an error has occurred. fprintf(stderr, "exec %s: ", arg->argv[0]); perror(""); return 1; } +// if a sigterm is received, forward it to the sandboxed child process +void forward_sigterm (int signum) { + kill(cloned_pid, signum); +} + // contain separates the process into new namespaces to sandbox it. -int contain(char* argv[], bool net, bool mount) { +int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_proc) { clone_arg arg; arg.uid = getuid(); arg.gid = getgid(); arg.argv = argv; arg.net = net; arg.mount = mount; + arg.sandbox_dir = sandbox_dir; + arg.mount_proc = mount_proc; + + if (pipe2(arg.sync_fd, O_CLOEXEC)) { + perror("pipe"); + return 1; + } static const int stack_size = 100 * 1024; char* stack = mmap(NULL, stack_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); @@ -334,6 +525,44 @@ int contain(char* argv[], bool net, bool mount) { fputs("please_sandbox requires support for user namespaces (usually >= Linux 3.10)\n", stderr); return 1; } + close(arg.sync_fd[0]); + + // set pid to global cloned_pid value for forwarding on sigterm + cloned_pid = pid; + signal (SIGTERM, forward_sigterm); + + // set up UID/GID mappings for the child + const char* uid_map = getenv("SANDBOX_UID_MAP"); + const char* gid_map = getenv("SANDBOX_GID_MAP"); + + if (uid_map != NULL || gid_map != NULL) { + if (uid_map == NULL || gid_map == NULL) { + fputs("SANDBOX_UID_MAP and SANDBOX_GID_MAP must be set together\n", stderr); + return 1; + } + + if (map_ids(pid, "newuidmap", uid_map) != 0 || + map_ids(pid, "newgidmap", gid_map) != 0) { + return 1; + } + } else { + // Otherwise deny setgroups, map single current user inside to current user outside + if (deny_setgroups(pid) != 0 || + write_id_map(pid, "uid_map", getuid(), getuid()) != 0 || + write_id_map(pid, "gid_map", getgid(), getgid()) != 0) { + return 1; + } + } + + // signal the child to proceed + if (write(arg.sync_fd[1], "1", 1) != 1) { + perror("write to sync pipe"); + return 1; + } else if (close(arg.sync_fd[1]) != 0) { + perror("close sync pipe"); + return 1; + } + // We're the parent process; wait on the child and exit with its status. int status = 0; if (waitpid(pid, &status, 0) == -1) { @@ -352,7 +581,7 @@ int contain(char* argv[], bool net, bool mount) { // On non-Linux systems contain simply execs a subprocess. // It's not really expected to be used there, this is simply to make it compile. -int contain(char* argv[], bool net, bool mount) { +int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_proc) { return execvp(argv[0], argv); } @@ -364,16 +593,23 @@ char* exec_name(const char* old_name, const char* old_dir, const char* new_dir) return change_path(old_name, old_dir, new_dir, 0); } +// check_valid_path_suffix makes sure given suffix can be appended to an +// absolute path without any issues. +bool check_valid_path_suffix(const char* suffix) { + return suffix != NULL && (suffix[0] == '/' || suffix[0] == 0); +} + // change_path takes a string or environment variable and changes a prefix from one path to another. char* change_path(const char* old_name, const char* old_dir, const char* new_dir, int prefix_len) { const int new_dir_len = strlen(new_dir); const int old_dir_len = strlen(old_dir); const int old_name_len = strlen(old_name); - if (strncmp(old_dir, old_name + prefix_len, old_dir_len) != 0) { // is the value of old_name prefixed with old_dir + if ((old_name_len > prefix_len + old_dir_len && !check_valid_path_suffix(old_name + prefix_len + old_dir_len)) || + strncmp(old_dir, old_name + prefix_len, old_dir_len) != 0) { // is the value of old_name prefixed with old_dir return (char*)old_name; // Dodgy cast but we know we don't alter it again later. } const int new_len = new_dir_len + old_name_len - old_dir_len + 1; - char* new_name = malloc(new_len + 1); + char* new_name = malloc((new_len + 1) * sizeof(char)); strncpy(new_name, old_name, prefix_len); strcpy(new_name + prefix_len, new_dir); strcpy(new_name + prefix_len + new_dir_len, old_name + prefix_len + old_dir_len); diff --git a/tools/sandbox/sandbox.h b/tools/sandbox/sandbox.h index 18b94664ae..10f22e0ec3 100644 --- a/tools/sandbox/sandbox.h +++ b/tools/sandbox/sandbox.h @@ -3,8 +3,11 @@ // contain separates the process into new namespaces to sandbox it. // It should be passed the argv for the new process, and booleans indicating // whether it should move to new network and mount namespaces. +// The sandbox_dir argument indicates whether it should attempt to create a sandbox +// for TMP_DIR in /tmp/plz_sandbox or not. +// The mount_proc argument indicates whether it should attempt to mount a new /proc. // It returns an exit code (so 0 on success, nonzero on failure). -int contain(char* argv[], bool net, bool mount); +int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_proc); // exec_name returns the name of the new binary to exec() as. // old_name is the current name; if it's within old_dir it will be re-prefixed to new_dir. diff --git a/tools/sandbox/test/BUILD b/tools/sandbox/test/BUILD new file mode 100644 index 0000000000..15ccba455a --- /dev/null +++ b/tools/sandbox/test/BUILD @@ -0,0 +1,16 @@ +go_test( + name = "sandbox_test", + srcs = ["sandbox_test.go"], + data = { + "please_sandbox": "//tools/sandbox:please_sandbox", + }, + labels = [ + # These tests are executed explicitly via machine executors in .circleci/config.yml + # For local testing execute in a container with equivalent configuration. + "manual", + ], + sandbox = False, + deps = [ + "///third_party/go/github.com_stretchr_testify//require", + ], +) diff --git a/tools/sandbox/test/sandbox_test.go b/tools/sandbox/test/sandbox_test.go new file mode 100644 index 0000000000..d1014dfe46 --- /dev/null +++ b/tools/sandbox/test/sandbox_test.go @@ -0,0 +1,333 @@ +package sandbox_test + +import ( + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func runSandbox(t *testing.T, sandbox string, args []string, env []string) string { + t.Helper() + cmd := exec.Command(sandbox, args...) + cmd.Env = env + out, err := cmd.CombinedOutput() + require.NoError(t, err, "sandbox command failed: %q, output: %q", err, string(out)) + return strings.TrimSpace(string(out)) +} + +func pleaseSandbox(t *testing.T, args []string, env []string) string { + t.Helper() + return runSandbox(t, os.Getenv("DATA_PLEASE_SANDBOX"), args, env) +} + +// TestSandboxEnvMatrix enumerates every combination of the environment variables that select +// which namespaces the sandbox creates, asserting each dimension independently: whether the +// network and mount namespaces are new, and whether a fresh /proc hides outer processes. +func TestSandboxEnvMatrix(t *testing.T) { + outerNet, err := os.Readlink("/proc/self/ns/net") + require.NoError(t, err) + outerMnt, err := os.Readlink("/proc/self/ns/mnt") + require.NoError(t, err) + + // The test's own pid exists in the outer /proc but not in a freshly remounted one. + probe := []string{"sh", "-c", fmt.Sprintf( + "readlink /proc/self/ns/net; readlink /proc/self/ns/mnt; test -d /proc/%d && echo visible || echo hidden", + os.Getpid())} + + for _, shareNetwork := range []bool{false, true} { + for _, shareMount := range []bool{false, true} { + for _, mountProc := range []bool{true, false} { + var opts []string + if shareNetwork { + opts = append(opts, "SHARE_NETWORK=1") + } + if shareMount { + opts = append(opts, "SHARE_MOUNT=1") + } + if !mountProc { + opts = append(opts, "MOUNT_PROC=0") + } + name := "default" + if len(opts) > 0 { + name = strings.Join(opts, ",") + } + + t.Run(name, func(t *testing.T) { + lines := strings.Split(pleaseSandbox(t, probe, append([]string{"TMP_DIR=/tmp"}, opts...)), "\n") + require.Len(t, lines, 3) + if shareNetwork { + require.Equal(t, outerNet, lines[0], "network namespace should be shared") + } else { + require.NotEqual(t, outerNet, lines[0], "network namespace should be new") + } + if shareMount { + require.Equal(t, outerMnt, lines[1], "mount namespace should be shared") + } else { + require.NotEqual(t, outerMnt, lines[1], "mount namespace should be new") + } + // /proc is only remounted when a new mount namespace exists and + // MOUNT_PROC is not disabled. + if !shareMount && mountProc { + require.Equal(t, "hidden", lines[2], "fresh /proc should hide outer processes") + } else { + require.Equal(t, "visible", lines[2], "outer /proc should remain visible") + } + }) + } + } + } +} + +func TestSandboxCommon(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("Connection accepted")) + }) + ts := httptest.NewServer(handler) + defer ts.Close() + + if _, err := os.Stat("/dev/null"); err != nil { + t.Fatalf("Test precondition failed: failed to stat /dev/null: %v", err) + } + + if _, err := os.Stat("/tmp"); err != nil { + t.Fatalf("Test precondition failed: failed to stat /tmp: %v", err) + } + + tests := []struct { + name string + env []string + args []string + want string + }{ + { + name: "must share network when SHARE_NETWORK=1", + env: []string{"SHARE_NETWORK=1", "TMP_DIR=/tmp"}, + args: []string{"sh", "-c", "curl -sS " + ts.URL}, + want: "Connection accepted", + }, + { + name: "must isolate network when SHARE_NETWORK=0", + env: []string{"SHARE_NETWORK=0", "TMP_DIR=/tmp"}, + args: []string{"sh", "-c", fmt.Sprintf(`curl -s %s; echo "curl exit: $?"`, ts.URL)}, + want: "curl exit: 7", // exit code 7 represents "Failed to connect to host." + }, + { + name: "must map parent user to its own UID by default", + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", "echo UID=$(id -u)"}, + want: fmt.Sprintf("UID=%d", os.Getuid()), + }, + { + name: "SANDBOX_DIRS must hide the contents of a specified directory", + env: []string{"TMP_DIR=/tmp", "SANDBOX_DIRS=/dev"}, + args: []string{"ls", "/dev"}, + want: "", + }, + { + name: "SANDBOX_FILE_MOUNTS must mount the specified file", + env: []string{"TMP_DIR=/tmp", "SANDBOX_FILE_MOUNTS=/proc/sys/kernel/ostype:/dev/null"}, + args: []string{"cat", "/dev/null"}, + want: "Linux", + }, + { + name: "SANDBOX_UID_MAP maps current uid/gid to 0", + env: []string{ + "TMP_DIR=/tmp", + fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1", os.Getuid()), + fmt.Sprintf("SANDBOX_GID_MAP=0 %d 1", os.Getgid()), + }, + args: []string{"sh", "-c", "echo $(id -u)/$(id -g)"}, + want: "0/0", + }, + { + // Outside ids below 65536 so they exist even in a constrained container uid space (e.g. the default rootless mapping). + name: "SANDBOX_UID_MAP uid and gid ranges", + env: []string{"TMP_DIR=/tmp", "SANDBOX_UID_MAP=0 20000 10 200 23000 40", "SANDBOX_GID_MAP=50 26000 700"}, + args: []string{"sh", "-c", "cat /proc/self/uid_map /proc/self/gid_map | awk '{$1=$1}1'"}, + want: "0 20000 10\n200 23000 40\n50 26000 700", + }, + { + name: "SANDBOX_UID_MAP uid and gid ranges allow us to use chown", + env: []string{ + "TMP_DIR=/var", + fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1 1 20000 40000", os.Getuid()), + fmt.Sprintf("SANDBOX_GID_MAP=0 %d 1 1 20000 40000", os.Getgid()), + }, + args: []string{"sh", "-c", "touch /tmp/f && chown 200:50 /tmp/f && stat -c '%u %g' /tmp/f"}, + want: "200 50", + }, + { + name: "Sandbox denies setgroups by default", + env: []string{"TMP_DIR=/tmp"}, + args: []string{"cat", "/proc/self/setgroups"}, + want: "deny", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := pleaseSandbox(t, tt.args, tt.env) + + if output != tt.want { + t.Errorf("Expected %q, but got %q", tt.want, output) + } + }) + } +} + +func TestSandboxNoTmpDir(t *testing.T) { + marker := fmt.Sprintf("/tmp/sandbox-test-marker-%d", os.Getpid()) + require.NoError(t, os.WriteFile(marker, []byte("outer"), 0o644)) + defer os.Remove(marker) + + probe := []string{"sh", "-c", fmt.Sprintf( + `echo "TMPDIR=$TMPDIR"; test -e %s && echo marker-visible || echo marker-hidden; test -e /tmp/plz_sandbox && echo bind-mounted || echo no-bind-mount`, + marker)} + out := pleaseSandbox(t, probe, []string{}) + + require.Contains(t, out, "TMPDIR=/tmp\n", "TMPDIR should point at the tmpfs mounted over /tmp") + require.Contains(t, out, "marker-hidden", "a file in the outer /tmp should be hidden by the tmpfs mounted over it") + require.Contains(t, out, "no-bind-mount", "nothing should be bind mounted at /tmp/plz_sandbox when TMP_DIR is unset") +} + +func TestSandboxLocalIP(t *testing.T) { + tests := []struct { + name string + env []string + args []string + want string + }{ + { + name: "adds 10.1.1.1 to the loopback interface by default", + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", `grep -o '10\.1\.1\.1' /proc/net/fib_trie | head -n1`}, + want: "10.1.1.1", + }, + { + name: "SANDBOX_LOCAL_IP overrides the default address", + env: []string{"TMP_DIR=/tmp", "SANDBOX_LOCAL_IP=10.2.3.4"}, + args: []string{"sh", "-c", `grep -o '10\.2\.3\.4' /proc/net/fib_trie | head -n1; grep -c '10\.1\.1\.1' /proc/net/fib_trie; true`}, + want: "10.2.3.4\n0", + }, + { + name: "SANDBOX_LOCAL_IP= disables the extra address", + env: []string{"TMP_DIR=/tmp", "SANDBOX_LOCAL_IP="}, + args: []string{"sh", "-c", `grep -c '10\.' /proc/net/fib_trie; true`}, + want: "0", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := pleaseSandbox(t, tt.args, tt.env) + + if output != tt.want { + t.Errorf("Expected %q, but got %q", tt.want, output) + } + }) + } +} + +// TestSandboxNoOrphanOnParentCrash checks that the sandboxed command does not outlive the +// sandbox process itself. Unlike TestSandboxHangOnParentCrash, which tests the window +// between clone() and the sync pipe, this kills the sandbox after the command has exec'd, +// executing the PR_SET_PDEATHSIG path. +func TestSandboxNoOrphanOnParentCrash(t *testing.T) { + sandbox := os.Getenv("DATA_PLEASE_SANDBOX") + if sandbox == "" { + t.Skip("DATA_PLEASE_SANDBOX not set") + } + + r, w, err := os.Pipe() + require.NoError(t, err) + defer r.Close() + + // The sleep only needs to outlive the 2s assertion window below. + cmd := exec.Command(sandbox, "sh", "-c", "echo ready && exec sleep 10") + cmd.Env = []string{"TMP_DIR=/tmp"} + cmd.Stdout = w + require.NoError(t, cmd.Start()) + // Close our copy of the write end so the read below blocks only on the sandboxed command. + w.Close() + + // Wait for the sandboxed command to be running, i.e. definitely past execvp. + buf := make([]byte, 6) + _, err = io.ReadFull(r, buf) + require.NoError(t, err) + require.Equal(t, "ready\n", string(buf)) + + // Simulate a crash of the sandbox process. + require.NoError(t, cmd.Process.Kill()) + _ = cmd.Wait() + + // The sandboxed process must die with it, closing its end of the pipe. + errCh := make(chan error, 1) + go func() { + _, err := r.Read(make([]byte, 1)) + errCh <- err + }() + select { + case err := <-errCh: + require.ErrorIs(t, err, io.EOF, "expected EOF once the sandboxed process died") + case <-time.After(2 * time.Second): + t.Fatal("sandboxed process survived the death of the sandbox process (orphan leak)") + } +} + +func TestSandboxHangOnParentCrash(t *testing.T) { + sandbox := os.Getenv("DATA_PLEASE_SANDBOX") + if sandbox == "" { + t.Skip("DATA_PLEASE_SANDBOX not set") + } + + // We run this in a loop with different delays to reliably hit the race condition window where + // the parent dies after clone() but before writing to the sync pipe. + for i := range 20 { + t.Run(fmt.Sprintf("iteration_%d", i), func(t *testing.T) { + r, w, err := os.Pipe() + require.NoError(t, err) + defer r.Close() + + cmd := exec.Command(sandbox, "echo", "success") + cmd.Stdout = w + + err = cmd.Start() + require.NoError(t, err) + + // Close our copy of the write-end immediately so that the read below can only block on either the sandbox itself or the sandboxed command. + w.Close() + + // Sleep just long enough for the parent to call clone(), + // but (hopefully) before it finishes uid mapping. + time.Sleep(time.Duration(i) * 100 * time.Microsecond) + + _ = cmd.Process.Kill() + _ = cmd.Wait() + + errCh := make(chan error, 1) + go func() { + buf := make([]byte, 128) + _, err := r.Read(buf) + errCh <- err + }() + + select { + case err := <-errCh: + // Success: The read unblocked. The child either finished fast + // or safely aborted when it noticed the parent died. + if err != io.EOF && err != nil { + t.Logf("Expected EOF, got: %v", err) + } + case <-time.After(100 * time.Millisecond): + t.Fatalf("Child process hung indefinitely holding stdout after parent was killed! (Race condition triggered)") + } + }) + } +}