fix(close-root): DenyUsers judged fail-closed — patterns and USER@HOST flag

All three reviewers, same substance, and they were right that it was
lockout-adjacent: the literal grep passed a candidate whom a DenyUsers
PATTERN really denies ('DenyUsers dan*' vs admin 'dan'), and the door
closed on a false proof. The judgment now lives in the lib as a pure
deny_verdict: a literal hit flags, and so does ANY pattern or
host-qualified token — a token the check cannot prove irrelevant counts
as a hit, never as a pass. The asymmetry with AllowUsers is now the
same direction on both sides: every error closes toward repair, never
toward a welded-shut door.

Also (claude-bot): the -C probe resolves Match blocks against a
synthetic addr=127.0.0.1, so Match Address is out of the local proof's
scope — named in --help, the README, and the gate's comment, so the
separate-session advisory reads as load-bearing, not ceremony.

Regressions ride the sourced lib: wildcard (the review's dan* case),
'?', USER@HOST, literal hit, irrelevant-literals pass, plus a grep
guard that the shipped gate consults deny_verdict.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot 2026-07-18 17:02:48 +00:00
parent 2396880f09
commit a5b48d5b2c
4 changed files with 88 additions and 15 deletions

View file

@ -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 `runuser` must answer, so NOPASSWD sudo is effective rather than merely
written, and `sshd -T -C user=<admin>` must resolve a per-user effective written, and `sshd -T -C user=<admin>` must resolve a per-user effective
config that accepts the login (`pubkeyauthentication yes`, no `DenyUsers` config that accepts the login (`pubkeyauthentication yes`, no `DenyUsers`
hit, `AllowUsers` — if set — names them), so a `Match` block elsewhere cannot hit — where any pattern or `USER@HOST` entry counts as a hit, fail closed,
quietly exclude the admin while every file looks right. The refusal names since `DenyUsers dan*` really denies admin `dan` and rig will not re-implement
which check failed, per candidate. What no local check can prove is that you sshd's pattern engine to prove a miss — and `AllowUsers`, if set, names them
*hold* the private key — which is why the separate-session verification below literally), so a `Match` block elsewhere cannot quietly exclude the admin
stays load-bearing. Never close the only door. 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 Before running it, prove the admin door in a **separate** session — `ssh
<admin>@<box>` while this one stays open. Root SSH is being welded shut; the <admin>@<box>` while this one stays open. Root SSH is being welded shut; the

View file

@ -145,3 +145,34 @@ assert_marker_human() {
return 1 ;; return 1 ;;
esac esac
} }
# deny_verdict <user> <denyusers token...>
#
# Judge sshd's effective DenyUsers list against ONE candidate, fail closed.
# Empty output = every token is PROVABLY irrelevant to <user>: 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
}

View file

@ -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 not expired), then two reachability proofs (#17) — `sudo -n true` under
runuser must answer (NOPASSWD sudo is effective, not merely written), and runuser must answer (NOPASSWD sudo is effective, not merely written), and
`sshd -T -C user=...` must resolve a per-user effective config that accepts `sshd -T -C user=...` must resolve a per-user effective config that accepts
the login (pubkeyauthentication yes, no DenyUsers hit, AllowUsers — if set the login (pubkeyauthentication yes, no DenyUsers hit — where any pattern or
names them). The refusal names which check failed, per candidate. Run rig host-qualified Deny entry counts as a hit, fail closed — and AllowUsers, if
users apply first; never close the only door. 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 Before running, verify your admin login in a SEPARATE session — `ssh
<admin>@<box>` while this one stays open. Root SSH is the door being welded <admin>@<box>` 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. Run as root. Convergent: once root is closed, a re-run is a clean no-op.
EOF EOF
@ -157,16 +161,24 @@ while IFS= read -r a; do
# Reachability proof 2 — sshd's per-user EFFECTIVE config accepts them. # Reachability proof 2 — sshd's per-user EFFECTIVE config accepts them.
# `sshd -T -C user=...` resolves Match blocks for exactly this login, so an # `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 # 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 # are judged fail-closed in BOTH directions: AllowUsers must name the admin
# flags here — fail closed, the operator proves patterns by hand — while a # literally (a pattern that would in fact admit them still flags — the
# denying pattern this literal match misses is one more reason the # operator proves patterns by hand), and DenyUsers flags on a literal hit
# separate-session verification stays load-bearing. # 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 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 if ! printf '%s\n' "$perT" | grep -qx 'pubkeyauthentication yes'; then
flag "sshd resolves pubkeyauthentication != yes for this user" flag "sshd resolves pubkeyauthentication != yes for this user"
fi fi
if printf '%s\n' "$perT" | grep -i '^denyusers ' | tr ' ' '\n' | grep -qx "$a"; then deny_line="$(printf '%s\n' "$perT" | grep -i '^denyusers ' | head -n1)"
flag "sshd DenyUsers names this user" 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 fi
if printf '%s\n' "$perT" | grep -qi '^allowusers ' \ if printf '%s\n' "$perT" | grep -qi '^allowusers ' \
&& ! printf '%s\n' "$perT" | grep -i '^allowusers ' | tr ' ' '\n' | grep -qx "$a"; then && ! printf '%s\n' "$perT" | grep -i '^allowusers ' | tr ' ' '\n' | grep -qx "$a"; then

View file

@ -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. # never die on a missing prover. Grep the graceful branch.
check "users close-root: a missing runuser skips the sudo proof, loudly" 0 "" \ check "users close-root: a missing runuser skips the sudo proof, loudly" 0 "" \
grep -q "runuser not found" "$ROOT/commands/users-close-root.sh" 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 <user> <token...>
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 # 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 # 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 # purpose (repo precedent: parse_users_file, assert_runner_repo). The command