From 22653ff4ff099b929dcb305a00fe92d42e710b44 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 13:00:11 -0700 Subject: [PATCH] =?UTF-8?q?docs(pam):=20clarify=20safe=5Fuser=20contract?= =?UTF-8?q?=20=E2=80=94=20pre-filter,=20not=20authoritative=20boundary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x..y is correctly NOT rejected by the slash/dot/empty/NUL guard; the recipe now documents this as an intentional regression test against over-rejection. Frames canonicalize-then-verify as Phase 17 follow-up hardening. --- pam/TESTING.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/pam/TESTING.md b/pam/TESTING.md index 3e9e477..cded47a 100644 --- a/pam/TESTING.md +++ b/pam/TESTING.md @@ -38,15 +38,34 @@ pamtester -v authforge "$USER" authenticate ## Smoke test 3: path-traversal regression -These usernames must all be rejected with a logged error and `PAM_AUTH_ERR`: +The guard in `safe_user` rejects names that break the path-as-identifier +invariant: empty strings, `.`, `..`, anything containing `/` or NUL. +Anything else passes through — including unusual but legitimate names that +contain `..` as a substring (LDAP / federated identities can produce +these). The loop below mixes both classes; the assertions live in the +comments: ```bash for u in "../etc" "a/b" "" "." ".." "x..y"; do pamtester -v authforge "$u" authenticate 2>&1 | head -3 echo "---" done +# Expect rejected (logged "rejected unsafe username"): ../etc, a/b, "", ., .. +# Expect passed through (no rejection log; pam_unix runs and prompts): x..y ``` +Keeping `x..y` in the loop is intentional — it documents the boundary and +acts as a regression test against accidental over-rejection if the guard +is ever tightened. + +**Future hardening (deferred — not blocking v1):** the guard is a cheap +pre-filter, not the authoritative boundary. A belt-and-suspenders pattern +would be canonicalize-then-verify: build the per-user path, resolve it +via `fs::canonicalize` (Rust) / `realpath` (C), and confirm the resolved +path is still under the expected parent directory. That covers symlink +shenanigans and any future `..`-substring attack surface uniformly. +Track as a Phase 17 item. + The module logs via `pam_syslog` without setting its own `openlog` ident, so journal entries inherit the calling process's syslog identifier: `pamtester` here, `sudo` / `sshd` / `gdm` / `login` in production. The