pastilhasandClaude Opus 5
cec8fbe57e
Review of 76cd7c2. The ACL finding is right and the fix is correct — verified here that `setfacl -R -P -b`
removes the default entries as well as the access ones, which the man page splits between -b and -k and
does not settle. `acl` is already a core package in setup.sh, so the new hard dependency is real.
But the checker it added cannot fail in the way it is documented to be run.
`assert-uid-free.sh` is invoked as `sudo ./assert-uid-free.sh --check ...`, and sudo's env_reset DROPS
DATA_PATH, so the script falls back to the hardcoded `/home/pastilhas/officerdev/data` — which is not this
machine's data directory and does not exist. Every check in the file is "look for X, report ok when nothing
is found", so a missing root reports clean without looking. Demonstrated: a tree carrying both
`user:65534:rwx` and `default:user:65534:rwx` was reported as `ok no ACL entries naming uid 65534`.
The ACL check is the one that fails silently and completely, because it is the only one scoped to DATA_PATH
alone — the uid and subuid scans still walk /home and would catch something. So the check just added to
catch the hazard ownership cannot see is the check a wrong DATA_PATH disables.
Fixed by refusing rather than passing:
require_roots every search root must exist, or exit 2 naming it and showing the sudo invocation
that preserves DATA_PATH
numeric guard uid/start/count must be numbers. deprovisionOsAccount logs '<no-subuid-range>' in
that position for an account with no /etc/subuid entry, and pasting that log line in
— which is exactly how it is meant to be used — made sub_end empty and turned the
range scan into a no-op.
The handler's audit line now prints DATA_PATH inside the command it tells the operator to copy, and says
so explicitly when there is no subuid range rather than emitting a command that cannot work.
Verified: bogus root exits 2, non-numeric range exits 2, and the ACL check FAILS on a specimen tree
carrying the entries — the "make it fail before trusting it to pass" step from the spec's own subuid
section, now done for the ACL half too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
122 lines
6.4 KiB
Bash
Executable File
122 lines
6.4 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Verify that a deprovisioned account has genuinely released its uid.
|
|
#
|
|
# Written as the verification half of `docs/deprovision-os-account.md`, deliberately OUTSIDE the
|
|
# implementation: if the function under test calls its own checker, the check is a restatement rather than
|
|
# an audit. This runs against the machine and knows nothing about the code that was supposed to clean it.
|
|
#
|
|
# The subuid range must be captured BEFORE the account is deleted, because `userdel` removes the
|
|
# /etc/subuid entry along with the account — after which there is no way to ask what range it held, and a
|
|
# check that silently skips that half is the failure mode this whole file exists to prevent.
|
|
#
|
|
# ./assert-uid-free.sh --capture green # before: prints "green 1001 165536 65536"
|
|
# ./assert-uid-free.sh --check green 1001 165536 65536 # after: exits non-zero unless clean
|
|
#
|
|
set -uo pipefail
|
|
|
|
DATA_PATH="${DATA_PATH:-/home/pastilhas/officerdev/data}"
|
|
SEARCH_ROOTS=("$DATA_PATH" /home)
|
|
|
|
usage() { echo "usage: $0 --capture <user> | --check <user> <uid> <subuid_start> <subuid_count>" >&2; exit 2; }
|
|
|
|
# ── A search root that does not exist makes this whole script lie ──
|
|
#
|
|
# Every check below is "look for X; report ok when nothing is found", so a root that is missing reports
|
|
# clean without having looked. That is not hypothetical here: this script is documented to run under
|
|
# `sudo`, and sudo's env_reset DROPS DATA_PATH, so the fallback above is what actually gets used. On a
|
|
# host where the fallback is wrong, `--check` scans a directory that does not exist, finds nothing, and
|
|
# prints "CLEAN — uid safe to reissue".
|
|
#
|
|
# The ACL check is the one that fails silently and completely, because it is scoped to DATA_PATH alone.
|
|
# The exact check added to catch the hazard ownership cannot see is the one a missing DATA_PATH disables.
|
|
#
|
|
# So: refuse to run rather than pass vacuously. Same posture the subuid section of the spec argues for.
|
|
require_roots() {
|
|
local missing=()
|
|
for root in "${SEARCH_ROOTS[@]}"; do
|
|
[[ -d "$root" ]] || missing+=("$root")
|
|
done
|
|
if (( ${#missing[@]} )); then
|
|
echo "refusing to check: these search roots do not exist: ${missing[*]}" >&2
|
|
echo "" >&2
|
|
echo "DATA_PATH is currently '$DATA_PATH'. sudo strips it from the environment, so pass it through:" >&2
|
|
echo " sudo DATA_PATH=/path/to/data $0 --check ..." >&2
|
|
echo " (or: sudo -E $0 --check ...)" >&2
|
|
echo "" >&2
|
|
echo "Every check here reports 'ok' on finding nothing, so a wrong root reports CLEAN without looking." >&2
|
|
exit 2
|
|
fi
|
|
}
|
|
|
|
if [[ "${1:-}" == "--capture" ]]; then
|
|
user="${2:?user required}"
|
|
uid="$(id -u "$user" 2>/dev/null)" || { echo "no such account: $user" >&2; exit 1; }
|
|
range="$(awk -F: -v u="$user" '$1==u {print $2" "$3; exit}' /etc/subuid)"
|
|
[[ -n "$range" ]] || { echo "no /etc/subuid entry for $user — capture it another way or it is unverifiable" >&2; exit 1; }
|
|
echo "$user $uid $range"
|
|
exit 0
|
|
fi
|
|
|
|
[[ "${1:-}" == "--check" ]] || usage
|
|
user="${2:?}"; uid="${3:?}"; sub_start="${4:?}"; sub_count="${5:?}"
|
|
require_roots
|
|
|
|
# The range arithmetic has to be numbers. `deprovisionOsAccount` logs '<no-subuid-range>' in this position
|
|
# when the account had no /etc/subuid entry, and pasting that log line straight in — which is exactly how
|
|
# it is meant to be used — would otherwise make sub_end empty and turn the range scan into a no-op.
|
|
[[ "$uid" =~ ^[0-9]+$ && "$sub_start" =~ ^[0-9]+$ && "$sub_count" =~ ^[0-9]+$ ]] || {
|
|
echo "uid, subuid_start and subuid_count must all be numbers (got: '$uid' '$sub_start' '$sub_count')" >&2
|
|
echo "an account with no /etc/subuid range has nothing to scan for — verify the uid half by hand" >&2
|
|
exit 2
|
|
}
|
|
sub_end=$(( sub_start + sub_count - 1 ))
|
|
|
|
fails=0
|
|
ok() { printf ' ok %s\n' "$1"; }
|
|
bad() { printf ' FAIL %s\n' "$1"; fails=$((fails+1)); }
|
|
|
|
echo "checking $user (uid $uid, subuids $sub_start-$sub_end)"
|
|
|
|
getent passwd "$user" >/dev/null 2>&1 && bad "passwd entry still exists" || ok "no passwd entry"
|
|
getent passwd "$uid" >/dev/null 2>&1 && bad "uid $uid reassigned or still present" || ok "uid $uid unused"
|
|
|
|
grep -q "^$user:" /etc/subuid 2>/dev/null && bad "/etc/subuid entry remains" || ok "no /etc/subuid entry"
|
|
grep -q "^$user:" /etc/subgid 2>/dev/null && bad "/etc/subgid entry remains" || ok "no /etc/subgid entry"
|
|
|
|
[[ -e "/var/lib/systemd/linger/$user" ]] && bad "linger marker remains" || ok "no linger marker"
|
|
[[ -d "/run/user/$uid" ]] && bad "/run/user/$uid remains" || ok "no runtime directory"
|
|
|
|
procs="$(pgrep -u "$uid" 2>/dev/null | wc -l)"
|
|
[[ "$procs" -eq 0 ]] && ok "no processes" || bad "$procs process(es) still owned by uid $uid"
|
|
|
|
# The uid half.
|
|
owned="$(find "${SEARCH_ROOTS[@]}" -uid "$uid" -print -quit 2>/dev/null)"
|
|
[[ -z "$owned" ]] && ok "no files owned by uid $uid" || bad "files owned by uid $uid (e.g. $owned)"
|
|
|
|
# The subuid half — the one a uid-only check passes straight through. Container processes running as a
|
|
# non-root user inside their namespace write files owned by a MAPPED id, not by the member's uid, and
|
|
# `userdel` frees the whole range for reallocation.
|
|
mapped="$(find "${SEARCH_ROOTS[@]}" -uid +"$((sub_start-1))" ! -uid +"$sub_end" -print -quit 2>/dev/null)"
|
|
[[ -z "$mapped" ]] && ok "no files in the freed subuid range" || bad "files owned by the freed subuid range (e.g. $mapped)"
|
|
|
|
# ACL entries, which ownership checks cannot see. `confineUserTree` grants the member a NAMED entry on their
|
|
# whole tree — `u:<uid>:rwx` plus a `default:` copy — and `chown` does not remove them: they are xattrs, not
|
|
# ownership, and they store the uid NUMERICALLY. So a tree reassigned to the service user can still carry
|
|
# `user:1001:rwx` on every file, and the next account allocated 1001 inherits read/write on all of it.
|
|
#
|
|
# `-n` forces numeric output; after `userdel` the uid has no name to resolve to, and relying on the name
|
|
# would make this check depend on the very passwd entry that is supposed to be gone.
|
|
#
|
|
# Scoped to DATA_PATH: member trees live there, and a recursive getfacl over /home would walk the owner's
|
|
# entire account for no gain.
|
|
acl_hit="$(getfacl -R -n -p "$DATA_PATH" 2>/dev/null | grep -m1 -E "^(default:)?user:$uid:")"
|
|
[[ -z "$acl_hit" ]] && ok "no ACL entries naming uid $uid" || bad "ACL entries still grant uid $uid ($acl_hit)"
|
|
|
|
echo
|
|
if [[ "$fails" -eq 0 ]]; then
|
|
echo "CLEAN — uid $uid and its subuid range are safe to reissue"
|
|
exit 0
|
|
fi
|
|
echo "NOT CLEAN — $fails check(s) failed; do not reissue this uid"
|
|
exit 1
|