forked from heavy-duty/rig
fix: don't read a missing /run/sshd as a broken sshd config
`sshd -t` folds two questions into one exit code — is the merged config parseable, and is the privilege-separation directory there. Both call sites ran it as `sshd -t 2>/dev/null` and read any non-zero exit as the first question's answer, discarding the line that named the second. Bootstrap aborted with "sshd rejects the merged config", a verdict sshd never reached, and sent the operator to audit /etc/ssh files that were never broken. /run is a tmpfs and /run/sshd is ssh.service's RuntimeDirectory, so it is legitimately absent under socket activation on a box whose SSH door is serving connections normally. Classification is now a pure, sourceable sshd_privsep_gap: the status is the verdict, the text only classifies a failure, so a passing sshd -t is never diverted. sshd_config_ok repairs the gap with an idempotent install -d and retests once. A genuine parse refusal still refuses and the rollback is untouched. Refusals now carry sshd's own stderr. users-close-root had the identical three lines and now reaches the shared judgement through lib/sshd.sh instead of keeping a second copy of it. Fixes #92 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claude-bot-andresmgsl
parent
616ada76b9
commit
2dc47112f2
4 changed files with 136 additions and 7 deletions
34
CHANGELOG.md
34
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
|
||||
|
|
|
|||
|
|
@ -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 <status> <stderr> — 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 <closed|open> — 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"
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
51
test/cli.sh
51
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 <status> <stderr-text>
|
||||
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"
|
||||
|
|
|
|||
Loading…
Reference in a new issue