diff --git a/docs/deprovision-os-account.md b/docs/deprovision-os-account.md index 4eb31b63..f3eeddd9 100644 --- a/docs/deprovision-os-account.md +++ b/docs/deprovision-os-account.md @@ -112,6 +112,26 @@ keeping every byte. rm -rf ``` +**Ownership is not the only link.** `confineUserTree` grants the member a NAMED ACL entry on their whole +tree — `u::rwx` and a `default:` copy, inherited by everything either party creates. `chown` does not +remove them: they are xattrs rather than ownership, and they store the uid **numerically**. Measured — a +`chown -h -R` to the service user leaves `user::rwx` intact on the directory, its children and their +defaults. + +So a tree reassigned to the service user still grants the freed uid read and write on every byte, and the next +account allocated that number inherits it: home, SSH keys, `.credentials.json`, transcripts, container +storage. That is the hazard this function exists to prevent, arriving through ACLs instead of ownership. + +Severing therefore has two parts: + +``` +chown -h -R : +setfacl -R -b # or -x u: -x d:u: to keep the platform's own entry +``` + +`-b` is the simpler answer for a preserved tree: the service user owns every byte afterwards, so a named +entry granting themselves access is redundant. + **This step must complete before step 4.** That is the one ordering choice the manual teardown got wrong: it released the uid first and removed the data afterwards, which leaves a window where the uid is free while files still carry it. If the process dies in that window, the next `useradd` inherits them. Sever first, then @@ -146,6 +166,8 @@ All of these must hold for the freed uid *and* its freed subuid range: - `/var/lib/systemd/linger/` absent - `/run/user/` absent - no processes owned by the uid +- **no ACL entry naming the uid** anywhere under `DATA_PATH` — `getfacl -R -n` and look for + `user::` / `default:user::`. Ownership checks cannot see these, and `chown` does not clear them. `scripts/assert-uid-free.sh` implements exactly this, deliberately **outside** the function: a checker the implementation calls is a restatement of its own beliefs, not an audit. Two modes, and the split matters — diff --git a/scripts/assert-uid-free.sh b/scripts/assert-uid-free.sh index a01c9466..4b6600be 100755 --- a/scripts/assert-uid-free.sh +++ b/scripts/assert-uid-free.sh @@ -60,6 +60,19 @@ owned="$(find "${SEARCH_ROOTS[@]}" -uid "$uid" -print -quit 2>/dev/null)" 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::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"