From 3eeab687d06a2372c01f533881fa293c51acbb89 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 20:01:19 +0000 Subject: [PATCH] =?UTF-8?q?fix(users):=20review=20findings=20=E2=80=94=20i?= =?UTF-8?q?nvoker=20gate,=20real=20SSH=20revocation,=20StrictModes-shaped?= =?UTF-8?q?=20close-root=20gate,=20trait-aware=20box=20role?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven review findings on the users family, each with the harness check that would have caught it: - Invoker gate (apply + close-root): %rig's sudoers rule is binary-scoped but not argument-scoped, so `sudo rig users apply --file ` made role rig silently root-equivalent through the very command that granted it. Identity management now refuses any sudo invoker outside rig-admin; direct root (bring-up, a root shell) proceeds. - Offboarding revokes SSH, not just the password: a '!'-locked password is not a closed door under UsePAM — Debian sshd still honors the pubkey. A dropped user's account is now expired (usermod -L -e 1, the switch PAM actually enforces) and authorized_keys is renamed to authorized_keys.revoked-by-rig — access revoked, data kept, convergence never destroys. Present users get their expiry cleared idempotently, so a re-added user comes back to life. - The ledger remembers: two-field lines ('name active' / 'name revoked', legacy bare names read as active), so dropped users no longer vanish from rig's memory on the next rewrite. status now reports the ledger state corroborated by the account's real expiry — passwd -S read L for everyone (apply locks all passwords always), so its locked/active was meaningless — and flags a mismatch loudly as drift. - Perms are part of the converged state: ~/.ssh and authorized_keys ownership and mode converge on every run, not only when content changes — StrictModes treats them as load-bearing, so drifted perms were a broken login that "already converged" lied about. Only the content write stays cmp-guarded. - close-root's admin-door gate checks the StrictModes shape per candidate — ownership, group/world-writability of home/.ssh/authorized_keys, a real login shell, an unexpired account — and names which check failed. It proves the door SHOULD open, not that it does; the separate-session advisory stays load-bearing. - Usernames are validated in the parser's one-pass refusal matrix (^[a-z_][a-z0-9_-]{0,31}$): 'fo|o' corrupted the parser's own '|'-delimited stream, and a leading '-' read as a useradd flag mid-convergence. - The box role is trait-aware: on a host=no box an absent incus group skips the role with a warning and converges everything else — one box-role user in a fleet-wide file must not abort apply everywhere VMs don't live. host=yes still dies pointing at box setup-host; a classless marker warns toward a bootstrap re-run. Co-Authored-By: Claude Fable 5 --- README.md | 63 +++++++++++++++------ commands/lib/users-config.sh | 12 +++- commands/users-apply.sh | 104 +++++++++++++++++++++++++++-------- commands/users-close-root.sh | 84 ++++++++++++++++++++++++---- commands/users-status.sh | 49 ++++++++++++----- test/cli.sh | 40 ++++++++++++++ 6 files changed, 288 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index 2a299ce..47fbc06 100644 --- a/README.md +++ b/README.md @@ -415,11 +415,24 @@ holds nothing secret anyway: usernames, roles, and *public* keys. | `rig` | NOPASSWD sudo for `/usr/local/bin/rig` only | `rig` | | `box` | Incus **restricted** tier, no sudo | `incus` | -`box` carries a refusal with it: rig never installs Incus — box's `setup-host` -owns the daemon — so an absent `incus` group means that never ran, and apply -dies pointing at `box setup-host` rather than conjure a group the -(nonexistent) daemon would never consult. `incus-admin` is deliberately -**not** a role: that group is host-root-equivalent, break-glass by hand only. +**The honest limit of the `rig` role:** its sudo grant is binary-scoped, not +argument-scoped — it trusts its holder with every rig verb *except* identity +management. The `rig users` commands gate their **invoker**: run under sudo +by anyone outside `rig-admin`, they refuse. Without that gate, `sudo rig +users apply` against a file naming yourself admin would make the scoped grant +silently root-equivalent through the very tool it scopes. Direct root — a +bring-up shell, before any admin exists — proceeds. + +`box` binds where VMs live, and a users file is fleet-wide — its box grants +are not. rig never installs Incus — box's `setup-host` owns the daemon — so +when the `incus` group is absent, the `host=` trait decides: on `host=yes` +apply dies pointing at `box setup-host` (a VM host missing Incus is a real +problem) rather than conjure a group the (nonexistent) daemon would never +consult; on `host=no` the box role is **skipped with a warning** and +everything else — admins included — still converges, because one box-role +user somewhere in the fleet must not stop apply everywhere VMs don't live. +`incus-admin` is deliberately **not** a role: that group is +host-root-equivalent, break-glass by hand only. **All passwords stay locked, always** — created or found. The SSH key at the door is the authentication, and NOPASSWD sudo does not weaken it: there was @@ -427,14 +440,21 @@ never a password to guess or rotate. Convergence is exact. Membership in the three rig-managed groups is made to match the file — added *and* removed — while every other group is left alone: -not rig's to converge. `authorized_keys` becomes exactly the file's keys. A -user dropped from the file is found via the `/etc/rig/users` ledger and -**locked, never deleted** — deletion frees the uid for reuse and orphans file -ownership, so attribution would rot; home stays for the same reason. And the -sudoers rules land in `/etc/sudoers.d/rig-roles` only after `visudo -c` -passes on the candidate — a bad file under `/etc/sudoers.d` can take down -*all* of sudo, locking every admin out of the very escalation path apply just -granted. +not rig's to converge. `authorized_keys` becomes exactly the file's keys, and +its ownership and mode (and `.ssh`'s) are converged on **every** run, not +just when content changes — sshd's `StrictModes` treats them as +load-bearing, so drifted perms are a broken login that "already converged" +would lie about. A user dropped from the file is found via the +`/etc/rig/users` ledger and **revoked, never deleted**: the account is +expired — the switch PAM actually enforces; a locked password alone still +lets a pubkey in under Debian's `UsePAM` — and `authorized_keys` is renamed +to `authorized_keys.revoked-by-rig`. Access revoked, data kept: deletion +frees the uid for reuse and orphans file ownership, so attribution would rot; +home stays for the same reason, and re-adding the user to the file brings +them back, fresh keys and all. And the sudoers rules land in +`/etc/sudoers.d/rig-roles` only after `visudo -c` passes on the candidate — a +bad file under `/etc/sudoers.d` can take down *all* of sudo, locking every +admin out of the very escalation path apply just granted. ### `rig users status` @@ -444,8 +464,12 @@ rig users status Read-only truth: per rig-managed user, the roles derived from the groups the user is **actually** in — not the ledger's memory of an apply — plus the -`authorized_keys` count and whether the account is locked or active. Reads the -box only; no network, no writes. Run as root (shadow is read). +`authorized_keys` count (`revoked` when only the `.revoked-by-rig` rename +remains) and the user's state, **active** or **revoked**. The state is the +ledger's word corroborated by the account's real expiry — the switch that +actually revokes — and a mismatch is flagged loudly as drift: a box someone +changed behind rig's back must never read as healthy. Reads the box only; no +network, no writes. Run as root (shadow is read). ### `rig users close-root` @@ -459,8 +483,13 @@ must say `class=human` — an absent marker refuses (never shut the root door blind; re-run bootstrap so the box knows what it is), and `class=server` refuses with no `--force`, because root there is the control plane's automation identity and closing it severs fleet management. Then at least one -`rig-admin` member must hold a non-empty `authorized_keys` — never close the -only door. +`rig-admin` member must hold a login sshd would plausibly **accept** — a +non-empty `authorized_keys` alone proves a file, not a door: the gate checks +the `StrictModes` shape (home, `.ssh`, and `authorized_keys` owned by the +user and not group/world-writable), a real login shell, and an unexpired +account, and its refusal names which check failed, per candidate. It proves +the door *should* open, not that it does — 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 e78f874..69251ee 100644 --- a/commands/lib/users-config.sh +++ b/commands/lib/users-config.sh @@ -24,7 +24,9 @@ # Refusals: unknown role (the valid set is named), differing roles across one # user's lines, root as username (root's keys are class policy's business, not # this file's), malformed line (fewer than 3 fields, or a key field that does -# not start with an SSH key type), duplicate identical key line. +# not start with an SSH key type), invalid username (the charset below — +# '|' would corrupt this parser's own delimited stream, a leading '-' reads +# as a useradd flag), duplicate identical key line. parse_users_file() { local path="$1" local -a errs=() out=() rlist=() @@ -44,6 +46,14 @@ parse_users_file() { errs+=("line $n: malformed — key field must start with an SSH key type (ssh-..., ecdsa-...)") continue ;; esac + # The username feeds this parser's own '|'-delimited stream and then + # useradd: 'fo|o' silently becomes user 'fo' with garbage keys, and a + # leading '-' reads as a useradd flag mid-convergence. One safe charset + # refuses both by construction (and ':', which would corrupt passwd). + if ! [[ "$u" =~ ^[a-z_][a-z0-9_-]{0,31}$ ]]; then + errs+=("line $n: invalid username '$u' — must match ^[a-z_][a-z0-9_-]{0,31}\$ (lowercase letter or '_' first, then lowercase, digits, '_', '-'; max 32)") + continue + fi if [ "$u" = "root" ]; then errs+=("line $n: 'root' is not a rig-managed user — this file names operators; root SSH's fate is class policy") continue diff --git a/commands/users-apply.sh b/commands/users-apply.sh index 36977d2..39021bd 100755 --- a/commands/users-apply.sh +++ b/commands/users-apply.sh @@ -42,8 +42,12 @@ All passwords stay locked, always — the SSH key at the door is the authentication, and NOPASSWD sudo does not weaken it. Convergent: membership in the three rig-managed groups is made exact (other groups are never touched), authorized_keys becomes exactly the file's keys, and a user dropped -from the file is locked and stripped of the rig groups — home kept, never -deleted. Run as root. +from the file is REVOKED: account expired (which blocks SSH keys too, not +just the password), authorized_keys renamed to authorized_keys.revoked-by-rig, +rig groups stripped — home kept, nothing deleted, and re-adding the user +brings them back. Run as root; under sudo, only rig-admin members may — the +users family changes who holds root, so role rig's scoped sudo does not reach +it. EOF } @@ -76,6 +80,7 @@ PARSED="$(parse_users_file "$FILE")" \ declare -A USER_ROLES=() USER_KEYS=() USERS=() +BOX_USERS=() NEED_SUDO=0 NEED_INCUS=0 while IFS='|' read -r u r k; do @@ -83,6 +88,7 @@ while IFS='|' read -r u r k; do if [ -z "${USER_ROLES[$u]:-}" ]; then USERS+=("$u") USER_ROLES[$u]="$r" + case ",$r," in *,box,*) BOX_USERS+=("$u") ;; esac fi USER_KEYS[$u]="${USER_KEYS[$u]:-}$k"$'\n' case ",$r," in *,admin,*|*,rig,*) NEED_SUDO=1 ;; esac @@ -92,9 +98,19 @@ done <<< "$PARSED" # --- guards ------------------------------------------------------------------ [ "$(id -u)" -eq 0 ] || die "must run as root" +# Identity management gates its INVOKER, not just its uid: %rig's sudoers rule +# is binary-scoped but not argument-scoped, so without this gate a rig-role +# user could run `sudo rig users apply --file ` — the scoped +# grant silently root-equivalent through this very command. Direct root (no +# SUDO_USER: bring-up, a root shell) proceeds. +if [ -n "${SUDO_USER:-}" ] && [ "$SUDO_USER" != "root" ] \ + && ! id -nG "$SUDO_USER" 2>/dev/null | tr ' ' '\n' | grep -qx rig-admin; then + die "the users family changes who holds root — only rig-admin members (or root itself) may run it; role rig grants operational rig use, not identity management (invoker: $SUDO_USER)" +fi + # Class is a note, never a refusal: #26's call is that operators belong on # EVERY class — what differs is root SSH's fate once they exist. -case "$(read_role_marker /etc/rig/role)" in +case "$(read_role_marker "${RIG_ROLE_MARKER:-/etc/rig/role}")" in *class=server*) log "class=server: root SSH stays — it is the control plane's automation door" ;; *class=human*) log "class=human: once your admin key works, 'rig users close-root' shuts the root door" ;; "") warn "no /etc/rig/role marker — re-run rig bootstrap so this box knows what it is" ;; @@ -113,10 +129,22 @@ fi groupadd -f rig-admin groupadd -f rig # rig NEVER installs Incus: box's setup-host owns the daemon and its group. An -# absent incus group means that never ran — refuse with the pointer rather -# than conjure a group the (nonexistent) daemon would never consult. -if [ "$NEED_INCUS" -eq 1 ] && ! getent group incus >/dev/null; then - die "a user carries role box but group incus is absent — install the box CLI and run 'box setup-host' first; rig never installs Incus" +# absent incus group means that never ran — but what that MEANS is the host= +# trait's call. The box role binds where VMs live; a users file is fleet-wide, +# its box grants are not. So on host=yes an absent group is a broken VM host +# (refuse, point at setup-host), while on host=no it is simply not this box's +# role to converge — skip it, never abort the admins the file also carries. +INCUS_OK=0 +if getent group incus >/dev/null; then INCUS_OK=1; fi +if [ "$NEED_INCUS" -eq 1 ] && [ "$INCUS_OK" -eq 0 ]; then + case "$(read_role_marker "${RIG_ROLE_MARKER:-/etc/rig/role}")" in + *host=yes*) + die "a user carries role box and this box hosts VMs (host=yes) but group incus is absent — install the box CLI and run 'box setup-host' first; rig never installs Incus" ;; + *host=no*) + warn "box role skipped for ${BOX_USERS[*]}: this box does not host VMs (host=no); everything else converges" ;; + *) + warn "box role skipped for ${BOX_USERS[*]}: the role marker names no host= trait — re-run rig bootstrap so this box knows whether it hosts VMs" ;; + esac fi in_group() { id -nG "$1" 2>/dev/null | tr ' ' '\n' | grep -qx "$2"; } @@ -129,8 +157,10 @@ for u in "${USERS[@]}"; do CHANGED=1 fi # Locked always, created or found: no password ever exists to guess or - # rotate — the SSH key at the door is the authentication. Idempotent. - usermod -L "$u" + # rotate — the SSH key at the door is the authentication. The expiry is + # cleared just as idempotently: revocation below IS an expiry date, so a + # user dropped once and re-added comes back to life on this line. + usermod -L -e '' "$u" # Membership in the three rig-managed groups is made EXACT — added and # removed to match the file. Other groups are never touched: they are not @@ -139,7 +169,10 @@ for u in "${USERS[@]}"; do want="" case ",$roles," in *,admin,*) want="$want rig-admin" ;; esac case ",$roles," in *,rig,*) want="$want rig" ;; esac - case ",$roles," in *,box,*) want="$want incus" ;; esac + # incus joins the wanted set only when the group exists (host=no boxes + # skipped it above): converging membership in a conjured group would hand + # the daemon's arrival an audience it never granted. + case ",$roles," in *,box,*) if [ "$INCUS_OK" -eq 1 ]; then want="$want incus"; fi ;; esac for g in rig-admin rig incus; do case " $want " in *" $g "*) @@ -157,43 +190,68 @@ for u in "${USERS[@]}"; do esac done - # authorized_keys becomes exactly the file's keys — cmp-guarded like every - # file rig converges, so an unchanged file is a clean no-op. + # authorized_keys becomes exactly the file's keys — only the content WRITE + # is cmp-guarded, so an unchanged file is a clean no-op. Ownership and mode + # converge UNCONDITIONALLY: sshd's StrictModes treats them as load-bearing + # (a group-writable .ssh is a rejected key), so drifted perms behind + # matching content would otherwise stay broken while apply logs "already + # converged". Perms are part of the converged state. home="$(getent passwd "$u" | cut -d: -f6)" ugroup="$(id -gn "$u")" + mkdir -p "$home/.ssh" AK_TMP="$(mktemp)" printf '%s' "${USER_KEYS[$u]}" > "$AK_TMP" if ! cmp -s "$AK_TMP" "$home/.ssh/authorized_keys" 2>/dev/null; then - mkdir -p "$home/.ssh" - chmod 0700 "$home/.ssh" - chown "$u:$ugroup" "$home/.ssh" install -m 0600 -o "$u" -g "$ugroup" "$AK_TMP" "$home/.ssh/authorized_keys" log "authorized_keys for $u: $(grep -c . "$AK_TMP") key(s)" CHANGED=1 fi rm -f "$AK_TMP" + chmod 0700 "$home/.ssh" + chown "$u:$ugroup" "$home/.ssh" + chmod 0600 "$home/.ssh/authorized_keys" + chown "$u:$ugroup" "$home/.ssh/authorized_keys" done # --- previously managed users no longer in the file -------------------------- -# The ledger is what lets a REMOVED user be found at all. Locked, not deleted: -# deleting frees the uid for reuse and orphans file ownership — attribution -# would rot. Home stays for the same reason. +# The ledger is what lets a REMOVED user be found at all — so it must REMEMBER +# them: two-field lines, 'name active' / 'name revoked' (a legacy bare name +# reads as active). Revoked, not deleted: deleting frees the uid for reuse and +# orphans file ownership — attribution would rot. Home stays for the same +# reason. But revoked must actually mean revoked: a '!'-locked password is not +# a closed door under UsePAM — Debian sshd still honors the pubkey — so the +# lock alone left a dropped operator with working SSH. Account expiry (a date +# in the past) is the switch PAM actually enforces, against every auth method +# including keys; the keys themselves are renamed, never deleted — access +# revoked, data kept, convergence never destroys. LEDGER=/etc/rig/users +REVOKED=() if [ -r "$LEDGER" ]; then - while IFS= read -r prev; do + while read -r prev pstate _; do [ -n "$prev" ] || continue case " ${USERS[*]:-} " in *" $prev "*) continue ;; esac id -u "$prev" >/dev/null 2>&1 || continue - usermod -L "$prev" + usermod -L -e 1 "$prev" + prevhome="$(getent passwd "$prev" | cut -d: -f6)" + if [ -f "$prevhome/.ssh/authorized_keys" ]; then + mv "$prevhome/.ssh/authorized_keys" "$prevhome/.ssh/authorized_keys.revoked-by-rig" + fi for g in rig-admin rig incus; do if in_group "$prev" "$g"; then gpasswd -d "$prev" "$g" >/dev/null; fi done - warn "$prev is no longer in the file: locked and stripped of the rig groups (home kept — rig never deletes a user)" - CHANGED=1 + REVOKED+=("$prev") + # Warn on the TRANSITION only: an already-revoked user is converged above + # (quietly — repairing drift, not announcing news) so a second identical + # run stays a clean no-op. + if [ "${pstate:-active}" != "revoked" ]; then + warn "$prev is no longer in the file: account expired (blocks SSH keys too, not just the password), authorized_keys renamed to authorized_keys.revoked-by-rig, rig groups stripped (home kept — rig never deletes a user)" + CHANGED=1 + fi done < "$LEDGER" fi LEDGER_TMP="$(mktemp)" -if [ "${#USERS[@]}" -gt 0 ]; then printf '%s\n' "${USERS[@]}" > "$LEDGER_TMP"; fi +if [ "${#USERS[@]}" -gt 0 ]; then printf '%s active\n' "${USERS[@]}" > "$LEDGER_TMP"; fi +if [ "${#REVOKED[@]}" -gt 0 ]; then printf '%s revoked\n' "${REVOKED[@]}" >> "$LEDGER_TMP"; fi if ! cmp -s "$LEDGER_TMP" "$LEDGER" 2>/dev/null; then mkdir -p /etc/rig install -m 0644 "$LEDGER_TMP" "$LEDGER" diff --git a/commands/users-close-root.sh b/commands/users-close-root.sh index 9dc6eee..093ebe5 100755 --- a/commands/users-close-root.sh +++ b/commands/users-close-root.sh @@ -26,8 +26,11 @@ Human class ONLY. On class=server, root SSH is the control plane's (Coolify's) automation identity — closing it severs fleet management — so close-root refuses there, with no --force. It also refuses without a role marker (re-run rig bootstrap; never shut the root door blind) and refuses while no rig-admin -member holds a working authorized_keys (run rig users apply first; never close -the only door). +member holds a login sshd would plausibly accept — authorized_keys present +and non-empty, home/.ssh/keys owned by the user and not group/world-writable +(sshd's StrictModes rejects the key otherwise), a real login shell, account +not expired. 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 @@ -48,6 +51,16 @@ done # --- guards ------------------------------------------------------------------ [ "$(id -u)" -eq 0 ] || die "must run as root" +# Identity management gates its INVOKER, not just its uid: %rig's sudoers rule +# is binary-scoped but not argument-scoped, so without this gate a rig-role +# user could reshape who enters this box as whom — the scoped grant silently +# root-equivalent through the users family. Direct root (no SUDO_USER: +# bring-up, a root shell) proceeds. +if [ -n "${SUDO_USER:-}" ] && [ "$SUDO_USER" != "root" ] \ + && ! id -nG "$SUDO_USER" 2>/dev/null | tr ' ' '\n' | grep -qx rig-admin; then + die "the users family changes who holds root — only rig-admin members (or root itself) may run it; role rig grants operational rig use, not identity management (invoker: $SUDO_USER)" +fi + # Marker gate — the policy lives in assert_marker_human (lib) so the harness # can prove its refusals against fixture markers as non-root; RIG_ROLE_MARKER # exists for the same reason: it keeps the command's own gate pointable at @@ -56,18 +69,69 @@ if ! WHY="$(assert_marker_human "${RIG_ROLE_MARKER:-/etc/rig/role}")"; then die "$WHY" fi -# Admin-door gate — never close the only door. Root SSH goes away below, so at -# least one rig-admin member must already hold a non-empty authorized_keys: -# "verified in a separate session" cannot be automated, but a key at the door -# can be, and its absence is proof enough to stop. +# Admin-door gate — never close the only door. Root SSH goes away below, so +# at least one rig-admin member must hold a login sshd would plausibly ACCEPT +# — a non-empty authorized_keys alone proves a file exists, not a door: +# StrictModes rejects keys behind wrongly-owned or group/world-writable +# paths, a nologin shell never logs in, and an expired account fails PAM +# before the key is read. So every candidate is checked for the StrictModes +# shape, and the refusal names, per candidate, WHICH check failed — an +# operator staring at a refusal must see the repair. Honestly: this proves +# the door SHOULD open per StrictModes, not that it does — the +# verify-in-a-separate-session advisory in --help stays load-bearing. +today=$(( $(date +%s) / 86400 )) +# path_strict