diff --git a/README.md b/README.md index ecf8c9a..e3e1aa4 100644 --- a/README.md +++ b/README.md @@ -629,11 +629,16 @@ account — and then two **reachability** proofs (#17): `sudo -n true` under `runuser` must answer, so NOPASSWD sudo is effective rather than merely written, and `sshd -T -C user=` must resolve a per-user effective config that accepts the login (`pubkeyauthentication yes`, no `DenyUsers` -hit, `AllowUsers` — if set — names them), so a `Match` block elsewhere cannot -quietly exclude the admin while every file looks right. The refusal names -which check failed, per candidate. What no local check can prove is that you -*hold* the private key — which is why the separate-session verification below -stays load-bearing. Never close the only door. +hit — where any pattern or `USER@HOST` entry counts as a hit, fail closed, +since `DenyUsers dan*` really denies admin `dan` and rig will not re-implement +sshd's pattern engine to prove a miss — and `AllowUsers`, if set, names them +literally), so a `Match` block elsewhere cannot quietly exclude the admin +while every file looks right. The refusal names which check failed, per +candidate. What no local check can prove: that you *hold* the private key, +and how a `Match Address` rule treats your real client address (the probe +resolves against a synthetic `addr=127.0.0.1`) — which is why the +separate-session verification below stays load-bearing. Never close the only +door. Before running it, prove the admin door in a **separate** session — `ssh @` while this one stays open. Root SSH is being welded shut; the diff --git a/commands/lib/users-config.sh b/commands/lib/users-config.sh index 5cd037a..7ce5578 100644 --- a/commands/lib/users-config.sh +++ b/commands/lib/users-config.sh @@ -145,3 +145,34 @@ assert_marker_human() { return 1 ;; esac } + +# deny_verdict +# +# Judge sshd's effective DenyUsers list against ONE candidate, fail closed. +# Empty output = every token is PROVABLY irrelevant to : literal (no +# sshd pattern metacharacters, no host qualifier) and not this username. +# Anything else prints the reason and the caller flags the candidate: +# +# - a literal hit — DenyUsers really names them; +# - ANY pattern token (* or ?) — 'DenyUsers dan*' genuinely denies admin +# 'dan', and this side of sshd cannot re-implement its pattern engine +# just to prove a miss, so an unprovable token counts as a hit; +# - ANY host-qualified token (USER@HOST) — whether it bites depends on the +# client's address, which no local probe knows. +# +# The asymmetry with AllowUsers is deliberate and points the same direction: +# AllowUsers must name the admin literally (a pattern that WOULD admit them +# still refuses — over-refusing is safe), DenyUsers refuses on anything it +# cannot prove misses. Both errors close toward "repair first", never toward +# a welded-shut root door. Pure text→text, sourced by the harness. +deny_verdict() { + local u="$1" tok; shift + for tok in "$@"; do + case "$tok" in + "$u") printf 'sshd DenyUsers names this user'; return 0 ;; + *[*?]*) printf "sshd DenyUsers has pattern entry '%s' — cannot prove it misses this user; make it literal or remove it, then re-run" "$tok"; return 0 ;; + *@*) printf "sshd DenyUsers has host-qualified entry '%s' — whether it bites depends on the client address, which no local check can prove; make it literal or remove it, then re-run" "$tok"; return 0 ;; + esac + done + return 0 +} diff --git a/commands/users-close-root.sh b/commands/users-close-root.sh index 09846d4..893b88f 100755 --- a/commands/users-close-root.sh +++ b/commands/users-close-root.sh @@ -33,13 +33,17 @@ owned by the user and not group/world-writable, a real login shell, account not expired), then two reachability proofs (#17) — `sudo -n true` under runuser must answer (NOPASSWD sudo is effective, not merely written), and `sshd -T -C user=...` must resolve a per-user effective config that accepts -the login (pubkeyauthentication yes, no DenyUsers hit, AllowUsers — if set — -names them). The refusal names which check failed, per candidate. Run rig -users apply first; never close the only door. +the login (pubkeyauthentication yes, no DenyUsers hit — where any pattern or +host-qualified Deny entry counts as a hit, fail closed — and AllowUsers, if +set, names them literally). The refusal names which check failed, per +candidate. Run rig users apply first; never close the only door. Before running, verify your admin login in a SEPARATE session — `ssh @` while this one stays open. Root SSH is the door being welded -shut; the admin door must be proven, not presumed. +shut; the admin door must be proven, not presumed. This is not ceremony: the +local probe resolves Match blocks against a synthetic loopback client +(addr=127.0.0.1), so a `Match Address` rule that treats real inbound clients +differently is invisible to it — only a real login proves the real door. Run as root. Convergent: once root is closed, a re-run is a clean no-op. EOF @@ -157,16 +161,24 @@ while IFS= read -r a; do # Reachability proof 2 — sshd's per-user EFFECTIVE config accepts them. # `sshd -T -C user=...` resolves Match blocks for exactly this login, so an # exclusion the global `sshd -T` never shows is caught. Allow/Deny entries - # are matched LITERALLY: a pattern that would in fact admit the admin still - # flags here — fail closed, the operator proves patterns by hand — while a - # denying pattern this literal match misses is one more reason the - # separate-session verification stays load-bearing. + # are judged fail-closed in BOTH directions: AllowUsers must name the admin + # literally (a pattern that would in fact admit them still flags — the + # operator proves patterns by hand), and DenyUsers flags on a literal hit + # OR on any pattern/host-qualified token (deny_verdict, in the lib) — + # 'DenyUsers dan*' really denies admin 'dan', and a token this check cannot + # prove irrelevant must count as a hit, never as a pass. What no local + # probe can resolve is a Match on the CLIENT's address — the -C probe pins + # addr=127.0.0.1 — which is why the separate-session verification stays + # load-bearing. if perT="$(sshd -T -C "user=$a,host=$(hostname),addr=127.0.0.1" 2>/dev/null)"; then if ! printf '%s\n' "$perT" | grep -qx 'pubkeyauthentication yes'; then flag "sshd resolves pubkeyauthentication != yes for this user" fi - if printf '%s\n' "$perT" | grep -i '^denyusers ' | tr ' ' '\n' | grep -qx "$a"; then - flag "sshd DenyUsers names this user" + deny_line="$(printf '%s\n' "$perT" | grep -i '^denyusers ' | head -n1)" + if [ -n "$deny_line" ]; then + # shellcheck disable=SC2086 # word-splitting the tokens is the point + deny_reason="$(deny_verdict "$a" ${deny_line#* })" + [ -n "$deny_reason" ] && flag "$deny_reason" fi if printf '%s\n' "$perT" | grep -qi '^allowusers ' \ && ! printf '%s\n' "$perT" | grep -i '^allowusers ' | tr ' ' '\n' | grep -qx "$a"; then diff --git a/test/cli.sh b/test/cli.sh index db4ebc0..38bfd90 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -567,6 +567,31 @@ check "users close-root: per-user sshd resolve precedes the drop-in install" \ # never die on a missing prover. Grep the graceful branch. check "users close-root: a missing runuser skips the sudo proof, loudly" 0 "" \ grep -q "runuser not found" "$ROOT/commands/users-close-root.sh" +# DenyUsers judged fail-closed through the lib's pure deny_verdict — sshd +# accepts patterns and USER@HOST forms, and 'DenyUsers dan*' REALLY denies +# admin 'dan', so a token the check cannot prove irrelevant must flag, never +# pass (the review's regression: a wildcard denial). Empty output is the only +# pass; every hit names its reason. +deny_v() { # deny_v + bash -c 'set -euo pipefail + . "$1/commands/lib/users-config.sh"; shift + deny_verdict "$@"' _ "$ROOT" "$@" +} +check "users close-root: deny_verdict flags a literal hit" \ + 0 "names this user" deny_v admin root admin +check "users close-root: deny_verdict fails closed on a wildcard (dan* vs dan)" \ + 0 "pattern entry 'dan*'" deny_v dan "dan*" +check "users close-root: deny_verdict fails closed on '?' patterns" \ + 0 "pattern entry" deny_v admin "admi?" +check "users close-root: deny_verdict fails closed on USER@HOST forms" \ + 0 "host-qualified" deny_v admin "admin@10.0.0.1" +deny_pass() { [ -z "$(deny_v "$@")" ]; } # empty verdict IS the pass +check "users close-root: deny_verdict passes provably-irrelevant literals" \ + 0 "" deny_pass admin root git backup +# ...and the shipped gate must actually consult it (call, not comment). +# shellcheck disable=SC2016 +check "users close-root: the gate consults deny_verdict" 0 "" \ + grep -qE '^[[:space:]]*deny_reason="\$\(deny_verdict ' "$ROOT/commands/users-close-root.sh" # Marker-gate refusals through the sourced lib against fixture markers: the CLI # path sits behind the root check, so the gate is a pure lib function on # purpose (repo precedent: parse_users_file, assert_runner_repo). The command