diff --git a/CHANGELOG.md b/CHANGELOG.md index 2075c5d..94290a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,40 @@ on the way to cutting its first release, and this file starts there. are pinned in `test/labels-reconcile.sh`. Ported from heavy-duty/box#137 so the three repos' reconcilers stay byte-identical; fixtures 19 → 51. +- **A missing `/run/sshd` no longer reads as a broken sshd config** (#92) — + `sshd -t` folds two questions into one exit code: is the merged config + parseable, and is the privilege-separation directory there. Both call sites + that validate before bouncing the daemon ran it as `sshd -t 2>/dev/null` and + read *any* non-zero exit as the first question's answer, discarding the one + line that named the second. Bootstrap aborted with `sshd rejects the merged + config` — a verdict sshd never reached — and the follow-up hint, `Run 'sshd + -t' to see which file is bad`, sent the operator to audit `/etc/ssh` files + that were never broken, on a box whose SSH door was serving their own session + at that moment. + + The absent directory is not an exotic state. `/run` is a tmpfs and + `/run/sshd` is `ssh.service`'s `RuntimeDirectory`, which systemd creates with + that unit and removes when it stops — so it is legitimately gone under socket + activation (`ssh.socket`, the default on current Debian/Ubuntu), where + per-connection `ssh@.service` instances serve the door and no long-lived unit + holds the directory open. Reported from a box mid-bootstrap right after an + `openssh-server` upgrade. + + The classification is now a pure, sourceable `sshd_privsep_gap` — the STATUS + is the verdict and the text only classifies a *failure*, so a passing `sshd + -t` is never diverted whatever its output says — and `sshd_config_ok` repairs + the gap with an idempotent `install -d` and retests once. The repair creates + exactly what systemd would and is not meant to outlive a reboot; by then the + ssh unit has recreated it. **A genuine parse refusal still refuses, and the + rollback is untouched**: the daemon is never bounced into a config it rejects. + + The refusals now carry sshd's own stderr instead of asserting a cause and + withholding the evidence for it. `rig users close-root` had the identical + three lines — the more dangerous copy, since it is the command that shuts the + root door — and now reaches the shared judgement through `lib/sshd.sh` rather + than keeping a second copy of it, which is what #31 extracted that lib for + and what let this bug ship twice. + ### Added - **`rig platform` — what is this machine, calculated at run time, stored diff --git a/commands/lib/sshd.sh b/commands/lib/sshd.sh index d666250..4c73955 100644 --- a/commands/lib/sshd.sh +++ b/commands/lib/sshd.sh @@ -6,6 +6,48 @@ # a hardening block is drift by construction, the same law that keeps rig's # hands off Incus. Callers provide log/warn/die. +# sshd_privsep_gap — true when a FAILED `sshd -t` failed for +# the missing privilege-separation directory rather than for anything in the +# config. Pure and sourceable (repo precedent: parse_users_file, deny_verdict) +# so the distinction is provable without root or a live sshd. +# +# `sshd -t` folds two questions into one exit code: is the merged config +# parseable, and is /run/sshd there. The second is not a fact about the config +# at all — /run is a tmpfs and /run/sshd is ssh.service's RuntimeDirectory, +# which systemd creates with that unit and removes when it stops, so the +# directory is legitimately absent under socket activation (ssh.socket, the +# default on current Debian/Ubuntu) on a box whose SSH door is serving +# connections perfectly well. Reading that as a config refusal aborted +# bootstrap with a verdict sshd never reached (#92). +# +# The STATUS is the verdict; this text match only classifies a failure. A +# passing `sshd -t` is never diverted here, whatever its output happens to say. +sshd_privsep_gap() { + [ "$1" -ne 0 ] || return 1 + case "$2" in + *"Missing privilege separation directory"*) return 0 ;; + *) return 1 ;; + esac +} + +# sshd_config_ok — validate the merged config, repairing a privsep gap once and +# retesting. Returns sshd's verdict and leaves sshd's own stderr in $sshd_err +# for the caller's refusal message: a message that asserts a cause must carry +# the evidence for it, or the operator greps /etc/ssh blind (#92). +# +# The repair is `install -d`, which is idempotent and creates exactly what +# systemd would. It does not survive a reboot and is not meant to — by then the +# ssh unit has recreated it. +sshd_config_ok() { + local rc + sshd_err="$(sshd -t 2>&1)"; rc=$? + if sshd_privsep_gap "$rc" "$sshd_err"; then + install -d -m 0755 /run/sshd + sshd_err="$(sshd -t 2>&1)"; rc=$? + fi + return "$rc" +} + # harden_sshd — install the 00-rig.conf hardening drop-in, # validate the merged config before touching the daemon, restart only when the # drop-in actually changed, and assert the EFFECTIVE config (sshd -T), with the @@ -42,10 +84,10 @@ EOF # leaves no listener and no way back in. `sshd -t` parses everything sshd # would parse — our drop-in, cloud-init's, and any third-party file — so a # broken neighbour is caught here rather than after the door has shut. - if ! sshd -t 2>/dev/null; then + if ! sshd_config_ok; then if [ -n "$backup" ]; then cp -a "$backup" "$dropin"; else rm -f "$dropin"; fi rm -f "$tmp" "$backup" - die "sshd rejects the merged config; drop-in rolled back, daemon untouched. Run 'sshd -t' to see which file is bad." + die "sshd rejects the merged config; drop-in rolled back, daemon untouched: $sshd_err" fi rm -f "$backup" diff --git a/commands/users-close-root.sh b/commands/users-close-root.sh index e3463c3..afc4492 100755 --- a/commands/users-close-root.sh +++ b/commands/users-close-root.sh @@ -11,6 +11,12 @@ set -euo pipefail HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)" # shellcheck source=SCRIPTDIR/lib/users-config.sh . "$HERE/lib/users-config.sh" +# The sshd validator is shared with bootstrap's hardening on purpose: both +# commands ask sshd the same question before bouncing the daemon, and two +# copies of that judgement is drift by construction (#31's law, #92's bug — +# the flaw this fixes was present in both copies). +# shellcheck source=SCRIPTDIR/lib/sshd.sh +. "$HERE/lib/sshd.sh" log() { printf 'rig-users: %s\n' "$*"; } warn() { printf 'rig-users: WARNING: %s\n' "$*" >&2; } @@ -270,13 +276,13 @@ if [ "$RESTART" -eq 1 ]; then # to become — restarting into a config the daemon refuses to parse leaves # no listener and no way back in. Roll back (when we installed anything) # and stop rather than shut the door on a maybe. - if ! sshd -t 2>/dev/null; then + if ! sshd_config_ok; then if [ "$INSTALLED" -eq 1 ]; then if [ -n "$BACKUP" ]; then cp -a "$BACKUP" "$DROPIN"; else rm -f "$DROPIN"; fi rm -f "$BACKUP" - die "sshd rejects the merged config; drop-in rolled back, daemon untouched. Run 'sshd -t' to see which file is bad." + die "sshd rejects the merged config; drop-in rolled back, daemon untouched: $sshd_err" fi - die "sshd rejects the current config; daemon untouched. Run 'sshd -t' to see which file is bad." + die "sshd rejects the current config; daemon untouched: $sshd_err" fi rm -f "$BACKUP" systemctl restart ssh diff --git a/test/cli.sh b/test/cli.sh index f7b6695..68d0141 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -1623,7 +1623,7 @@ check "users close-root: drop-in name is the load-bearing one" 0 "" \ # bouncing the daemon into a config it refuses to parse leaves no way back in. # Match the call, not the word (repo precedent: the repo-guard ordering check); # defaults fail closed. -sshdt_at="$(grep -nE '^[[:space:]]*if ! sshd -t' "$ROOT/commands/users-close-root.sh" | head -n1 | cut -d: -f1)" +sshdt_at="$(grep -nE '^[[:space:]]*if ! sshd_config_ok' "$ROOT/commands/users-close-root.sh" | head -n1 | cut -d: -f1)" restart_at="$(grep -n 'systemctl restart ssh' "$ROOT/commands/users-close-root.sh" | head -n1 | cut -d: -f1)" check "users close-root: sshd -t precedes the ssh restart" \ 0 "" test "${sshdt_at:-999999}" -lt "${restart_at:-0}" @@ -1879,10 +1879,57 @@ check "sshd lib: root-door=open refusal names the stale close-root drop-in" 0 "" grep -q "leftover /etc/ssh/sshd_config.d/00-rig-users.conf" "$ROOT/commands/lib/sshd.sh" # Validate-then-apply survived the extraction: sshd -t on the merged config # must still precede the restart (same idiom as the close-root ordering check). -libt_at="$(grep -nE '^[[:space:]]*if ! sshd -t' "$ROOT/commands/lib/sshd.sh" | head -n1 | cut -d: -f1)" +libt_at="$(grep -nE '^[[:space:]]*if ! sshd_config_ok' "$ROOT/commands/lib/sshd.sh" | head -n1 | cut -d: -f1)" librestart_at="$(grep -nE '^[[:space:]]*systemctl restart ssh$' "$ROOT/commands/lib/sshd.sh" | head -n1 | cut -d: -f1)" check "sshd lib: sshd -t precedes the ssh restart" \ 0 "" test "${libt_at:-999999}" -lt "${librestart_at:-0}" +# `sshd -t` answers TWO questions through ONE exit code: is the merged config +# parseable, and is the privilege-separation directory there. /run is a tmpfs +# and /run/sshd is ssh.service's RuntimeDirectory — systemd removes it when +# that unit stops — so it is legitimately absent under socket activation +# (ssh.socket, the default on current Debian/Ubuntu) on a box whose SSH door is +# serving connections normally. Reading that as "the config is bad" aborted +# bootstrap with a verdict sshd never reached, and sent the operator to audit +# /etc/ssh files that were never broken (#92). The classifier is pure and +# sourceable so the distinction is proven here, non-root and without a live +# sshd (repo precedent: parse_users_file, deny_verdict). +privsep_gap() { # privsep_gap + bash -c 'set -euo pipefail + . "$1/commands/lib/sshd.sh" + sshd_privsep_gap "$2" "$3"' _ "$ROOT" "$1" "$2" +} +check "sshd lib: a missing privsep dir is not a config verdict" 0 "" \ + privsep_gap 1 "Missing privilege separation directory: /run/sshd" +check "sshd lib: a genuine parse refusal stays a config verdict" 1 "" \ + privsep_gap 1 "/etc/ssh/sshd_config.d/50-cloud-init.conf: line 3: Bad configuration option: frobnicate" +# The STATUS is the verdict; the text only classifies a failure. A passing +# sshd -t is never diverted, whatever its output happens to say — otherwise a +# box could be sent down the repair path with nothing wrong with it. +check "sshd lib: a passing sshd -t is never read as a privsep gap" 1 "" \ + privsep_gap 0 "Missing privilege separation directory: /run/sshd" +# Surfacing sshd's own words is the substance of #92: the old message asserted a +# cause and then discarded, via 2>/dev/null, the one line that named the real +# one. Both call sites make the claim, so both are pinned. +# shellcheck disable=SC2016 # the literal source line is the pattern, unexpanded +check "sshd lib: the refusal quotes sshd's own stderr" 0 "" \ + grep -qF 'daemon untouched: $sshd_err' "$ROOT/commands/lib/sshd.sh" +# shellcheck disable=SC2016 +check "users close-root: the refusal quotes sshd's own stderr" 0 "" \ + grep -qF 'daemon untouched: $sshd_err' "$ROOT/commands/users-close-root.sh" +# ...and the gap is REPAIRED, not merely diagnosed: a message the operator must +# act on by hand is still a blocked bootstrap. +check "sshd lib: the privsep gap is repaired before the retest" 0 "" \ + grep -qF 'install -d -m 0755 /run/sshd' "$ROOT/commands/lib/sshd.sh" +# close-root does NOT carry its own copy of that repair — it reaches it through +# the shared lib. #31 extracted ONE sshd converger precisely so a judgement +# cannot drift between the two commands, and #92 is what drift costs: the same +# flawed three lines sat in both files and had to be fixed twice. Pin the +# sharing, not a duplicate: the source line and the call. +# shellcheck disable=SC2016 +check "users close-root: validates through the shared sshd lib" 0 "" \ + grep -qE '^\. "\$HERE/lib/sshd\.sh"$' "$ROOT/commands/users-close-root.sh" +check "users close-root: no second copy of the privsep repair" 1 "" \ + grep -qF 'install -d -m 0755 /run/sshd' "$ROOT/commands/users-close-root.sh" # shellcheck disable=SC2016 check "bootstrap: hardening runs through the shared lib" 0 "" \ grep -qE '^harden_sshd "\$ROOT_DOOR"$' "$ROOT/commands/bootstrap.sh"