diff --git a/docker/entrypoint-go.sh b/docker/entrypoint-go.sh index f2f5c1e5e..2c95907e3 100644 --- a/docker/entrypoint-go.sh +++ b/docker/entrypoint-go.sh @@ -17,8 +17,8 @@ fi # Which user runs the two Go services, and what they are handed. supervisord # itself stays root so that mosquitto and caddy can still bind their ports. # Unprivileged operation is opt-in via RUN_AS_UID, taken from the container's -# environment before .env is read; .env is then read (with the services -# unprivileged, only if root owns it) and the data directory handed over. The +# environment before .env is read; the data directory is handed over, and .env +# is read last (with the services unprivileged, only if root owns it). The # order matters and lives in one function: see docker/select-service-user.sh, # and "Running the Go services unprivileged" in docs/deployment.md. . /select-service-user.sh diff --git a/docker/select-service-user.sh b/docker/select-service-user.sh index 3a00ba64c..8136355ec 100644 --- a/docker/select-service-user.sh +++ b/docker/select-service-user.sh @@ -3,8 +3,8 @@ # hands over the paths they have to write. # # Sourced by docker/entrypoint-go.sh, which calls prepare_service_user and -# nothing else from here: the order of the steps (resolve the account, read -# .env, hand over the data directory) is part of what keeps .env from choosing +# nothing else from here: the order of the steps (resolve the account, hand +# over the data directory, read .env) is part of what keeps .env from choosing # the account, so it lives in one function and the test drives that function. # docker/test-select-service-user.sh replaces getent/adduser/addgroup/deluser/ # delgroup/id/chown/find/stat with stubs: keep every external command here @@ -34,36 +34,63 @@ SERVICE_DATA_DIR=/app/data SERVICE_ENV_FILE=/app/data/.env SERVICE_MOUNTINFO=/proc/self/mountinfo +# printf, not echo: dash's echo turns a \012 in a logged path into a newline. svc_log() { - echo "[entrypoint] $*" >&2 + printf '%s\n' "[entrypoint] $*" >&2 } # prepare_service_user # -# The start-up sequence, in the only order that is safe: +# The start-up sequence: # # 1. resolve the account, from RUN_AS_UID/RUN_AS_GID in the container's # environment; -# 2. read /app/data/.env (see load_env_file for when it is trusted); -# 3. hand the data directory to the account, when it is not root. +# 2. hand the data directory to the account, when it is not root; +# 3. read /app/data/.env (see load_env_file for when it is trusted), then +# put RUN_AS_UID/RUN_AS_GID back to what the container had. # # Returns non-zero, with the reason on stderr, when RUN_AS_UID was set and no # account could be provided. On success exports CORESCOPE_SERVICE_USER. # -# The resolved account is kept in the positional parameter, not a variable: the -# sourced .env runs in this shell, and a line in it such as `account=root` would -# otherwise change which account is handed the data directory and exported to -# supervisord. +# .env comes last, and what runs after it reads only this function's +# positional parameters. A sourced file shares the shell's variables: every +# global, the locals of load_env_file, which sources it, and, by dynamic +# scoping, the locals of this function too, so `_account=root` or +# `SERVICE_DATA_DIR=/` in .env would reach any step that still read them. It +# also gets load_env_file's positional parameters, so `set -- root` there +# lands in that function's $1 and not in this one's. Hence the account, the +# container's RUN_AS_UID/RUN_AS_GID and the file's path are parked in $1..$6 +# before .env is read, the hand-over, which reads the SERVICE_* paths, is done +# before it, and nothing else follows. +# +# A .env that passes the trust check is root's own code, and this does not +# try to defend against it: it can redefine these functions or set PATH. What +# it cannot do by assigning variables is change the account, where the +# hand-over goes, or RUN_AS_UID/RUN_AS_GID as the services see them. prepare_service_user() { local _account _account=$(resolve_service_account) || return 1 - set -- "$_account" + set -- "$_account" "${RUN_AS_UID+set}" "${RUN_AS_UID-}" \ + "${RUN_AS_GID+set}" "${RUN_AS_GID-}" "$SERVICE_ENV_FILE" unset _account - load_env_file "$SERVICE_ENV_FILE" "$1" if [ "$1" != root ]; then claim_service_paths "$1" fi + + load_env_file "$6" "$1" + + # The account was resolved before .env was read, so a RUN_AS_UID or + # RUN_AS_GID in it must not reach supervisord and the services either: put + # back what the container had, set or unset. + if [ "${RUN_AS_UID+set}" != "$2" ] || [ "${RUN_AS_UID-}" != "$3" ] || + [ "${RUN_AS_GID+set}" != "$4" ] || [ "${RUN_AS_GID-}" != "$5" ]; then + svc_log "ignoring RUN_AS_UID/RUN_AS_GID from $6: set them on the container" + svc_log "(docker run -e, or environment: in compose), where the services cannot remove them." + if [ -n "$2" ]; then RUN_AS_UID=$3; export RUN_AS_UID; else unset RUN_AS_UID; fi + if [ -n "$4" ]; then RUN_AS_GID=$5; export RUN_AS_GID; else unset RUN_AS_GID; fi + fi + CORESCOPE_SERVICE_USER=$1 export CORESCOPE_SERVICE_USER } @@ -162,11 +189,11 @@ resolve_service_account() { # writable by no one else. Running as root changes nothing — whoever could # write the file could already run code as root. # -# Local names are underscored: the file is sourced into this function. +# Nothing runs here after the file: it shares this function's variables and +# positional parameters, so anything read afterwards would be whatever it +# chose to leave. prepare_service_user does the rest from its own parameters. load_env_file() { local _env_file="$1" _env_account="$2" - local _uid_before="${RUN_AS_UID-}" _gid_before="${RUN_AS_GID-}" - local _uid_was="${RUN_AS_UID+set}" _gid_was="${RUN_AS_GID+set}" [ -f "$_env_file" ] || return 0 if [ "$_env_account" != root ] && ! env_file_trusted "$_env_file"; then @@ -179,17 +206,6 @@ load_env_file() { set -a . "$_env_file" set +a - - # The account was resolved before this file was read, so a RUN_AS_UID or - # RUN_AS_GID in it must not reach supervisord and the services either: put - # back what the container had, set or unset. - if [ "${RUN_AS_UID+set}" != "$_uid_was" ] || [ "${RUN_AS_UID-}" != "$_uid_before" ] || - [ "${RUN_AS_GID+set}" != "$_gid_was" ] || [ "${RUN_AS_GID-}" != "$_gid_before" ]; then - svc_log "ignoring RUN_AS_UID/RUN_AS_GID from $_env_file: set them on the container" - svc_log "(docker run -e, or environment: in compose), where the services cannot remove them." - if [ -n "$_uid_was" ]; then RUN_AS_UID=$_uid_before; export RUN_AS_UID; else unset RUN_AS_UID; fi - if [ -n "$_gid_was" ]; then RUN_AS_GID=$_gid_before; export RUN_AS_GID; else unset RUN_AS_GID; fi - fi } # env_file_trusted diff --git a/docker/test-select-service-user.sh b/docker/test-select-service-user.sh index 7269bb4aa..40cbb6d8c 100755 --- a/docker/test-select-service-user.sh +++ b/docker/test-select-service-user.sh @@ -5,19 +5,25 @@ # Every branch is exercised without docker and without root: getent, adduser, # addgroup, deluser, delgroup, id, chown, find and stat are replaced by stubs # on PATH, and the mount table is a fixture file. The account stubs read and -# write a passwd/group pair of files seeded with the accounts the real image has — verified against alpine:3.20 with -# mosquitto, supervisor and caddy installed, plus the Dockerfile's corescope — -# so an account created mid-run is visible to the lookups that follow it. chown -# and find record their arguments rather than touching the filesystem, which is -# what lets the ownership hand-over be asserted exactly. +# write a passwd/group pair of files seeded with the accounts the real image +# has (verified against alpine:3.20 with mosquitto, supervisor and caddy +# installed, plus the Dockerfile's corescope), so an account created mid-run is +# visible to the lookups that follow it. chown and find record their arguments +# rather than touching the filesystem, which is what lets the ownership +# hand-over be asserted exactly. # -# The driver calls prepare_service_user, the one function docker/entrypoint-go.sh -# calls: resolve the account, read .env, hand over the data directory. A static -# check at the end pins that the entrypoint calls it and nothing else from the -# script, so what gets tested is the sequence the container runs: a non-zero -# return must never be read as root, and .env cannot choose the account. +# The driver calls prepare_service_user, the one function the entrypoint +# (docker/entrypoint-go.sh) calls: resolve the account, hand over the data +# directory, read .env. A static check at the end pins that the entrypoint +# calls it and nothing else from the script, so what gets tested is the +# sequence the container runs: a non-zero return must never be read as root, +# and .env cannot choose the account. # # Run: bash docker/test-select-service-user.sh +# TEST_SHELL picks the shell the sequence runs in, default sh; the image runs +# busybox ash, so TEST_SHELL=/path/to/busybox-sh is worth a run. A busybox +# built to prefer its own applets over PATH (Ubuntu's package) bypasses the +# stubs and fails most cases; Debian's and Alpine's look up PATH first. # Exits non-zero if any case fails. set -u @@ -209,7 +215,7 @@ STUB # the services would see them. drive() { make_fixture - OUT=$(env -i PATH="$FIX/bin:/usr/bin:/bin" FIX="$FIX" SCRIPT="$SCRIPT" "$@" sh -c ' + OUT=$(env -i PATH="$FIX/bin:/usr/bin:/bin" FIX="$FIX" SCRIPT="$SCRIPT" "$@" "${TEST_SHELL:-sh}" -c ' . "$SCRIPT" SERVICE_ENV_FILE="$FIX/env" SERVICE_MOUNTINFO="$FIX/mountinfo" @@ -419,6 +425,53 @@ CORESCOPE_SERVICE_USER=root' drive RUN_AS_UID=1000 STUB_STAT='0 -rw-r--r--' assert_eq "account" "corescope" "$OUT" assert_has "the data dir was still handed over to it" "$CALLS" "chown -h 1000:1000" +# The file is sourced inside load_env_file, which has positional parameters of +# its own, so a `set --` in it cannot reach the account prepare_service_user +# keeps in $1. Sourcing it in prepare_service_user instead would make this fail. +CASE=".env runs set -- root" +FIX_ENV='set -- root' drive RUN_AS_UID=1000 STUB_STAT='0 -rw-r--r--' +assert_eq "account" "corescope" "$OUT" +assert_has "the data dir was still handed over to it" "$CALLS" "chown -h 1000:1000" + +# A sourced file shares every variable of the shell, the locals of the +# functions on the call stack included. The container's RUN_AS_UID/RUN_AS_GID +# are therefore put back from positional parameters, not from variables a line +# in .env could pre-empt. The uid and gid differ so that a mixed-up parameter +# shows. +CASE=".env assigns the names the restore could have used" +FIX_ENV='_uid_before=0 +_uid_was=set +_gid_before=0 +_gid_was=set +_env_file=/nowhere +_env_account=root +SERVICE_ENV_FILE=/nowhere +RUN_AS_UID=0 +RUN_AS_GID=0' drive RUN_AS_UID=1000 RUN_AS_GID=1001 STUB_STAT='0 -rw-r--r--' +assert_eq "account" "corescope" "$OUT" +assert_eq "both put back" "RUN_AS_UID=1000 RUN_AS_GID=1001" "$FINAL_ENV" +assert_has "ignored, and the log names the real file" "$LOG" "ignoring RUN_AS_UID/RUN_AS_GID from $FIX/env" + +# Each of the two is compared on its own: a .env that changes only the gid is +# noticed too. +CASE="RUN_AS_GID alone in a trusted .env, 1000:1000 on the container" +FIX_ENV='RUN_AS_GID=0' drive RUN_AS_UID=1000 RUN_AS_GID=1000 STUB_STAT='0 -rw-r--r--' +assert_eq "gid put back" "RUN_AS_UID=1000 RUN_AS_GID=1000" "$FINAL_ENV" +assert_has "ignored, and said so" "$LOG" "ignoring RUN_AS_UID/RUN_AS_GID" + +# The hand-over reads the SERVICE_* paths, so it happens before .env is read: +# a .env that points them elsewhere must find the data directory already dealt +# with, and .env itself already left out of it. +CASE=".env points the SERVICE_* paths elsewhere" +FIX_ENV='SERVICE_DATA_DIR=/ +SERVICE_ENV_FILE=/nowhere +SERVICE_MOUNTINFO=/dev/null' drive RUN_AS_UID=1000 STUB_STAT='0 -rw-r--r--' +assert_eq "account" "corescope" "$OUT" +assert_has "the data dir was handed over, .env left out" "$CALLS" \ + "find /app/data ( -path $FIX/env ) -prune -o ! -user 1000 -exec chown -h 1000:1000 {} +" +assert_lacks "nothing else was walked" "$CALLS" "find / (" +assert_lacks "and the real .env was the one excluded" "$CALLS" "/nowhere" + # ----- 15. a file mounted into the data directory keeps its host owner ----- # The documented single-file config mount, plus one with a space in its path, # as the kernel escapes it. The data directory's own mount is not excluded. @@ -463,6 +516,12 @@ assert_eq "startup continues" 0 "$RC" assert_has "warning names the count" "$LOG" "WARNING: 2 path(s) in /app/data could not be given to uid 1000" assert_has "and the paths" "$LOG" " /app/data/meshcore.db" +# A logged path is printed as it is: dash's echo would turn the \012 into a +# newline and cut the path in two. +CASE="chown refused on a path with a backslash escape" +drive RUN_AS_UID=1000 STUB_LEFTOVER='/app/data/x\012y' +assert_has "path logged intact" "$LOG" ' /app/data/x\012y' + # ----- 17. the entrypoint runs the sequence through prepare_service_user ---- # The driver above tests the function; this pins that the entrypoint uses it # and does not rebuild the sequence from the steps, in an order of its own. @@ -473,6 +532,7 @@ assert_has "calls prepare_service_user" "$EP" "prepare_service_user || exit 1" assert_lacks "does not resolve the account itself" "$EP" "resolve_service_account" assert_lacks "does not read .env itself" "$EP" "load_env_file" assert_lacks "does not hand over paths itself" "$EP" "claim_service_paths" +assert_lacks "does not set the account itself" "$EP" "CORESCOPE_SERVICE_USER=" # ----- report --------------------------------------------------------------- echo "select-service-user: $PASS assertions passed, $FAIL failed"