diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5479da..a53abd6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: run: >- docker run --rm --network none -v "$PWD:/mnt:ro" -w /mnt koalaman/shellcheck:v0.11.0@sha256:61862eba1fcf09a484ebcc6feea46f1782532571a34ed51fedf90dd25f925a8d - -x bin/devenv tests/devenv/unit.sh tests/devenv/integration.sh tests/devenv/tally.sh + -x bin/devenv tests/devenv/unit.sh tests/devenv/cross-user.sh tests/devenv/integration.sh tests/devenv/tally.sh tests/app/lib.sh tests/app/backend.sh tests/app/frontend.sh tests/app/stack.sh tests/app/prodlike.sh tests/app/scan.sh docker/api/entrypoint.sh docker/api/healthcheck.sh docker/db/flowcrm-db-roles docker/db/initdb-roles.sh @@ -112,6 +112,9 @@ jobs: - name: Allocator unit tests run: tests/devenv/unit.sh + - name: Secrets unreadable by another local user (trusted, outside the isolation) + run: tests/devenv/cross-user.sh + - name: Allocator integration tests (real Docker) run: tests/devenv/integration.sh diff --git a/docs/dev-environment.md b/docs/dev-environment.md index 318233d..f30ea59 100644 --- a/docs/dev-environment.md +++ b/docs/dev-environment.md @@ -409,6 +409,7 @@ Its regression suite is `scripts/ci/tests/guard-dockerfile-frontend.test.sh`. ```console $ make test-devenv # allocator: unit + integration (fixture) $ tests/devenv/unit.sh # no Docker needed +$ tests/devenv/cross-user.sh # secrets vs. another local user (passwordless sudo) $ DEVENV_TEST_PORT_BASE=18080 tests/devenv/integration.sh $ make test-app # application: the four suites below ``` @@ -484,9 +485,9 @@ command substitution, `-H`, `--context` or an inherited `DOCKER_HOST`, ports 2375 and 2376 and every listening loopback port. If the isolation cannot be applied or proven, the suite does not run (exit 125). - Setuid programs cannot run inside, so `sudo` is unavailable. The U-16 - check that another local user cannot read a secret therefore prints INFO - and is not counted, in CI as well as locally; the U-16 mode checks still - run. + check that another local user cannot read a secret is therefore not part + of `unit.sh` (which prints INFO and counts only the U-16 mode checks); it + belongs to `tests/devenv/cross-user.sh`, described below. - On Linux the probe finds an engine socket owned by the calling user only in the places listed above. Writes are not restricted on Linux, so a mutant could change files that run later outside the isolation. CI @@ -534,6 +535,60 @@ Mutation testing: - BE-18 mutates a copy of the backend inside the api test container, which has no Docker socket. +Cross-user check of the secrets: + +`tests/devenv/cross-user.sh` (CU-xx, its own CI step in the `Dev +environment` job) checks that the user `nobody` can neither list the +secrets directory nor read a secret that `bin/devenv` generated. It needs +`sudo`, so it runs outside the isolation, and is built so that no code under +test runs there: + +- The checkout's own `bin/devenv`, never a copy named by an input, runs + `ensure_secrets` under `no-docker.sh`, with its runtime directory in a new + directory below `/var/tmp` and a fake `docker` that reports no volume. + That directory is the isolated step's `TMPDIR`: on macOS the sandbox lets + it write only there and in `/dev`. `/var/tmp` is used because it is + `1777` on Linux and macOS; the macOS `$TMPDIR` is `0700`, so `nobody` + could not reach anything below it, and the macOS isolation denies writes + to `/tmp` and refuses a `TMPDIR` below it. +- Outside the isolation the script checks the result only as data (file + type, owner, modes `0700`/`0444`), then runs fixed commands as `nobody` + on fixed paths with an empty environment (`/usr/bin/env -i`): `/bin/ls` + of the directory and `/bin/cat` of each secret. +- A control makes a pass meaningful: `nobody` must be able to read a + `0444` file that the script puts in a `0755` directory next to the + secrets directory, and to list that directory. This proves that the path is reachable (below a `0700` + directory such as the macOS `$TMPDIR` every read would fail) and that the + read check detects an exposed file. If the control fails, its result + shows the error and the mode of every parent directory. +- A denial counts only if the secrets were generated (CU-01), the control + passed (CU-02) and the command failed with `Permission denied`; any other + failure, such as `sudo` itself failing, makes the result a failure. +- CU-04 is denied by the `0700` secrets directory, not by the file mode: + the secret files are `0444` by design, so that the containers' + unprivileged users can read the bind-mounted copy. CU-01 checks both + modes as data. +- The isolated code runs as the caller and knows the temporary directory, + so it could replace anything in it, for example with a symbolic link to a + file under `$HOME`. After it returns, nothing outside the isolation + writes, creates, executes or reads a file there: those paths are only + tested and `stat`ed, and read by `nobody`. Error output is kept in + variables, and the result tally is in a new directory made after the + isolated step in the caller's own `$TMPDIR`, outside the temporary + directory. +- Neither isolation ends processes that the isolated step leaves behind. + Such a process stays isolated, but it could still change a result or keep + the step waiting on its output until the CI job times out. On macOS it + cannot write the tally, which is outside its sandbox; on Linux the + isolation does not restrict file writes, but such a process has no more + file access than the caller. +- It certifies committed code and is not a mutation harness: it refuses + (exit 64) arguments, `DEVENV`, `GUARD`, `ADAPTED`, a run inside + `no-docker.sh`, a directory that is not a Git checkout, and local + changes to `bin/devenv`, itself, `tally.sh`, `no-docker.sh` or the probe. +- Without passwordless sudo to `nobody` it prints INFO (CU-01 to CU-04 not + run, no PASS) and exits 0; in CI (`CI=true`) that is a failure (CU-00). + Application suites (`tests/app/`), each in its own Compose projects: - `scan.sh` (SC-xx): Trivy scans of the runtime images (production-like api @@ -642,6 +697,9 @@ run, so several runs from one checkout do not interfere. (targets and the commands they run), the test runners' image removal (by name only, see "Removing test images") and checks that the result tally detects failed and lost results. +- `tests/devenv/cross-user.sh` checks that another local user cannot list + or read the generated secrets (CU-01 to CU-04, 7 results; see "Cross-user check of + the secrets" under "Isolation from Docker"). - `tests/devenv/integration.sh` runs scenarios PA-01 to PA-23 and the Makefile targets against real Docker in temporary checkout copies. It uses the 20-port block starting at `DEVENV_TEST_PORT_BASE` (default 8080) and @@ -658,7 +716,7 @@ run, so several runs from one checkout do not interfere. and verifies that the lock is held until they end. PA-23 checks that an invalid Compose file fails with exit 2, keeps the verified state and tries no other port. -- Both suites compare the printed PASS/FAIL records with their counters and +- The suites compare the printed PASS/FAIL records with their counters and exit 2 on a mismatch; a scenario without results or a failed setup step counts as a failure. diff --git a/tests/devenv/cross-user.sh b/tests/devenv/cross-user.sh new file mode 100755 index 0000000..7563d7c --- /dev/null +++ b/tests/devenv/cross-user.sh @@ -0,0 +1,197 @@ +#!/usr/bin/env bash +# +# Cross-user check of the generated secrets (U-16): another local user +# (nobody) can neither list the secrets directory nor read a secret that +# bin/devenv generated. +# +# This suite owns that check; tests/devenv/unit.sh cannot run it, because +# setuid programs such as sudo do not work inside scripts/ci/no-docker.sh. +# It is split in two parts so that no code under test runs with the +# caller's privileges outside the isolation: +# 1. The checkout's own bin/devenv (never a copy named by DEVENV or any +# other input) runs ensure_secrets under scripts/ci/no-docker.sh, with +# its runtime directory in a fresh directory below /var/tmp and a fake +# docker that reports no volume. +# 2. Outside the isolation, this script checks the result as data (file +# type, owner, modes; never the contents) and then runs only fixed +# commands as nobody on fixed paths, with an empty environment: +# /usr/bin/env -i /bin/cat FILE and /usr/bin/env -i /bin/ls DIR. A +# denial counts only as "Permission denied" after a passed control. +# It never writes, creates or reads a file below the directory that +# the isolated code was given (which that code could have replaced). +# A control file that this script makes in a 0755 directory next to the +# secrets directory, and that nobody must be able to read (and the +# directory list), proves that the path is reachable for nobody and that +# both checks detect an exposed file. +# +# The check needs passwordless sudo. Without it the suite prints INFO (CU-01 +# to CU-04 NOT RUN, no PASS) and exits 0, except in CI (CI=true), where that +# is a failure (CU-00). +# +# It certifies the committed code, so it refuses (exit 64) when DEVENV, +# GUARD, ADAPTED or NO_DOCKER_ACTIVE is set, outside a Git checkout, and +# when one of the files it runs differs from HEAD. It is not a mutation +# harness: run mutated code only through unit.sh and scripts/ci/tests/. +# +# Usage: tests/devenv/cross-user.sh +# Exit codes: 0 passed (or skipped locally), 1 a check failed, +# 2 result tally mismatch, 64 refused + +# shellcheck source-path=SCRIPTDIR + +set -u + +here=$(cd -P "$(dirname "$0")" && pwd -P) || exit 64 +root=$(cd -P "$here/../.." && pwd -P) || exit 64 + +refuse() { + printf 'cross-user: refused: %s\n' "$1" >&2 + exit 64 +} + +[ $# -eq 0 ] || refuse "no arguments are accepted" +for var in DEVENV GUARD ADAPTED NO_DOCKER_ACTIVE; do + [ -z "${!var+set}" ] || refuse "$var is set; this check runs only the checkout's own code, outside the isolation" +done + +# The files this suite runs, outside or inside the isolation. +involved='bin/devenv tests/devenv/cross-user.sh tests/devenv/tally.sh scripts/ci/no-docker.sh scripts/ci/no-docker-probe.py' +# shellcheck disable=SC2086 # the fixed list of paths is split into words +{ + git -C "$root" rev-parse --is-inside-work-tree >/dev/null 2>&1 && + git -C "$root" ls-files --error-unmatch -- $involved >/dev/null 2>&1 +} || refuse "not a Git checkout with $involved tracked" +# shellcheck disable=SC2086 +git -C "$root" -c core.fsmonitor=false diff --no-ext-diff --no-textconv --quiet HEAD -- $involved || + refuse "local modifications to $involved; commit them first (an edited file cannot be told from a mutated one)" + +# shellcheck source=tally.sh +. "$here/tally.sh" + +# Printable text of at most 2000 bytes of standard input, on one line: error +# output goes into the log this way (never a secret, which is never read). +clean() { head -c 2000 | tr '\n' ' ' | tr -cd '[:print:]'; } + +cd / || exit 64 +# Every command as nobody is a fixed absolute binary on fixed arguments, with +# an empty environment (env -i: nothing of the caller passes, and messages +# are in the C locale). +if ! probe_err=$(sudo -n -u nobody -- /usr/bin/env -i /usr/bin/true 2>&1); then + if [ "${CI-}" = true ]; then + printf 'FAIL CU-00 passwordless sudo to the user nobody is required in CI (%s)\n' "$(printf '%s' "$probe_err" | clean)" + exit 1 + fi + printf 'INFO CU-01 to CU-04 NOT RUN: the cross-user read check needs passwordless sudo to the user nobody (runs in CI)\n' + exit 0 +fi + +# Below /var/tmp (1777 on Linux and macOS), not $TMPDIR: on macOS $TMPDIR is +# a 0700 per-user directory, so nobody could not reach anything below it and +# the check would pass for the wrong reason. Not /tmp either: the macOS +# isolation denies writes there and refuses a TMPDIR below it. The parents +# of the runtime directory are 0755 for the same reason; the control file +# below proves it. The isolated step gets this directory as its TMPDIR, the +# only place (with /dev) where the macOS sandbox lets it write. +base=$(mktemp -d /var/tmp/flowcrm-cross-user.XXXXXX) || exit 2 +base=$(cd -P "$base" && pwd -P) || exit 2 +tally_dir= +trap 'rm -rf "$base" ${tally_dir:+"$tally_dir"}' EXIT + +result() { # name ok(0|1) [detail] + if [ "$2" -eq 0 ]; then pass_result "$1"; else fail_result "$1${3:+ $3}"; fi +} + +rt=$base/rt +secrets_dir=$rt/secrets +names='db_password db_owner_password db_app_password app_secret' +mkdir "$base/bin" "$rt" && chmod 0755 "$base" "$rt" || exit 2 +printf '#!/bin/sh\nexit 1\n' >"$base/bin/docker" && chmod 0755 "$base/bin/docker" || exit 2 +# The control: a directory as open as the secrets directory must not be, +# with a 0444 file at the depth of a secret, made by this script. +{ mkdir "$rt/control" && chmod 0755 "$rt/control" && printf 'control\n' >"$rt/control/file" && + chmod 0444 "$rt/control/file"; } || exit 2 + +# --- part 1, isolated: the checkout's bin/devenv generates the secrets +# The isolated code knows $base and runs as the caller, so it can replace +# anything below it, for example with a symbolic link to a file under $HOME +# (which the macOS sandbox would not let it write). The trusted part +# therefore never writes, creates or reads a file below $base after this +# step: error output is kept in variables (the last 4000 bytes) and the +# tally is in a directory made after this step, outside $base, in the +# caller's own TMPDIR (which the macOS sandbox does not let it write). +# shellcheck disable=SC2016 # expanded by the isolated shell +gen_err=$({ TMPDIR=$base "$root/scripts/ci/no-docker.sh" "$BASH" -c ' + set -u + . "$1" || exit 2 + FLOWCRM_STACK=app + init_paths + RUNTIME_DIR=$2 SECRETS_DIR=$2/secrets STATE_FILE=$2/dev.env LOCK_DIR=$2/lock + GATE_DIR=$2/gate GATE_FILE=$2/gate/security-gate + PATH=$3:$PATH + ensure_secrets +' cross-user "$root/bin/devenv" "$rt" "$base/bin" 2>&1 >/dev/null | tail -c 4000 + exit "${PIPESTATUS[0]}"; }) +rc=$? +tally_dir=$(mktemp -d "${TMPDIR:-/tmp}/flowcrm-cross-user-tally.XXXXXX") || exit 2 +tally_init "$tally_dir/tally" + +# --- part 2, outside the isolation: checks as data, then fixed reads as nobody +mode_of() { # GNU stat first: its -f means file system status + stat -c '%a' "$1" 2>/dev/null || stat -f '%Lp' "$1" +} +ok=0 +{ [ "$rc" = 0 ] && [ ! -L "$rt" ] && [ -d "$rt" ] && [ ! -L "$secrets_dir" ] && [ -d "$secrets_dir" ] && + [ -O "$secrets_dir" ] && [ "$(mode_of "$secrets_dir")" = 700 ]; } || ok=1 +for name in $names; do + f=$secrets_dir/$name + { [ ! -L "$f" ] && [ -f "$f" ] && [ -s "$f" ] && [ -O "$f" ] && [ "$(mode_of "$f")" = 444 ]; } || ok=1 +done +result "CU-01 the checkout's bin/devenv created the secrets in the isolation (directory 0700, regular files 0444)" $ok \ + "(rc $rc: $(printf '%s' "$gen_err" | tail -c 2000 | clean))" +generated=$ok + +# Stdout is discarded (it would be a secret); stderr is kept in nobody_err. +as_nobody() { nobody_err=$(sudo -n -u nobody -- /usr/bin/env -i "$@" 2>&1 >/dev/null); } +modes_up() { # path: the mode of it and of every parent, for a failed control + local p=$1 out= + while :; do + out="$out $p=$(mode_of "$p")" + [ "$p" != / ] || break + p=$(dirname "$p") + done + printf '%s' "${out# }" +} + +# Both commands of the negative checks must work on the control, so that +# neither can pass because the command itself cannot run as nobody. +reachable=1 +if as_nobody /bin/cat "$rt/control/file" && as_nobody /bin/ls "$rt/control"; then reachable=0; fi +result "CU-02 control: nobody can read a 0444 file in, and list, a 0755 directory next to the secrets directory (the path is reachable and the checks detect an exposed file)" $reachable \ + "(as nobody: $(printf '%s' "$nobody_err" | clean); modes: $(modes_up "$rt/control/file"))" + +# A denial counts only if the secrets were generated (CU-01), the control +# was read (CU-02) and the command failed with "Permission denied": any +# other failure, such as sudo itself failing, is not a denial. +denied() { # name command path + if [ "$generated" != 0 ] || [ "$reachable" != 0 ]; then + fail_result "$1 (not checked: CU-01 or CU-02 failed)" + elif as_nobody "$2" "$3"; then + fail_result "$1 (nobody succeeded)" + elif case $nobody_err in *'Permission denied'*) true ;; *) false ;; esac; then + pass_result "$1" + else + fail_result "$1 (failed, but not with Permission denied: $(printf '%s' "$nobody_err" | clean))" + fi +} + +denied "CU-03 nobody cannot list the secrets directory" /bin/ls "$secrets_dir" +for name in $names; do + denied "CU-04 nobody cannot read the secret $name" /bin/cat "$secrets_dir/$name" +done +# Nothing here writes, creates, follows into a write or executes a path that +# the isolated code could have replaced (such as a symbolic link to a file +# of the caller): those paths are only tested and stat'ed, and read by +# nobody. CU-04 is denied by the 0700 directory: the secret files are 0444 +# by design (the containers' unprivileged users read the bind mount). + +tally_finish "" diff --git a/tests/devenv/unit.sh b/tests/devenv/unit.sh index 04d238e..6101e32 100755 --- a/tests/devenv/unit.sh +++ b/tests/devenv/unit.sh @@ -803,12 +803,9 @@ chmod 0644 "$SECRETS_DIR/db_password" r=$(secrets_rc exists) [ "$r" = 0 ] && [ "$(mode_of "$SECRETS_DIR")" = 700 ] && [ "$(mode_of "$SECRETS_DIR/db_password")" = 444 ] result "U-16 a too-open secrets directory and secret file are tightened (0700, 0444)" $? "(rc $r)" -if sudo -n -u nobody true 2>/dev/null; then - if sudo -n -u nobody cat "$SECRETS_DIR/db_password" >/dev/null 2>&1; then r=1; else r=0; fi - result "U-16 another local user (nobody) cannot read a secret" $r -else - printf 'INFO U-16 cross-user read check needs passwordless sudo (runs in CI); permissions checked above\n' -fi +# That another local user cannot read a secret needs sudo, which does not +# work in the isolation: tests/devenv/cross-user.sh owns that check (CU-xx). +printf 'INFO U-16 cross-user read check: tests/devenv/cross-user.sh (outside the isolation); permissions checked above\n' rm -f "$SECRETS_DIR/db_password" r=$(secrets_rc exists) [ "$r" = "$EX_SAFETY" ] && [ ! -e "$SECRETS_DIR/db_password" ] && grep -q "db-data was initialised with it" "$work/secrets.err" &&