From eb208579e3b91cc6394b04583424d422fa6a8277 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:22:23 -0700 Subject: [PATCH 1/9] refactor(daemon): centralize username path-segment sanitizer Extract the inline path-traversal check from PendingStore into a shared storage::safe_user::join_user_segment helper. RecoveryStore (next) reuses it. Co-Authored-By: Claude Opus 4.7 (1M context) --- daemon/src/storage/mod.rs | 1 + daemon/src/storage/pending.rs | 12 +++------ daemon/src/storage/safe_user.rs | 45 +++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 daemon/src/storage/safe_user.rs diff --git a/daemon/src/storage/mod.rs b/daemon/src/storage/mod.rs index 0301050..1d7d54b 100644 --- a/daemon/src/storage/mod.rs +++ b/daemon/src/storage/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod credentials; pub(crate) mod pending; pub(crate) mod policy; +pub(crate) mod safe_user; pub(crate) mod userdb; diff --git a/daemon/src/storage/pending.rs b/daemon/src/storage/pending.rs index cd2cc26..16d4a35 100644 --- a/daemon/src/storage/pending.rs +++ b/daemon/src/storage/pending.rs @@ -24,15 +24,9 @@ impl PendingStore { } fn user_path(&self, user: &str) -> Result { - if user.is_empty() - || user.contains('/') - || user.contains('\0') - || user == "." - || user == ".." - { - return Err(PendingError::InvalidUser(user.to_string())); - } - Ok(self.dir.join(user)) + super::safe_user::join_user_segment(&self.dir, user).map_err(|e| match e { + super::safe_user::SegmentError::Invalid(s) => PendingError::InvalidUser(s), + }) } pub fn set(&self, user: &str, flag: &PendingFlag) -> Result<(), PendingError> { diff --git a/daemon/src/storage/safe_user.rs b/daemon/src/storage/safe_user.rs new file mode 100644 index 0000000..fdf84a7 --- /dev/null +++ b/daemon/src/storage/safe_user.rs @@ -0,0 +1,45 @@ +//! Shared username sanitizer for path-segment use across pending/recovery stores. + +use std::path::{Path, PathBuf}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub(crate) enum SegmentError { + #[error("invalid username segment: {0:?}")] + Invalid(String), +} + +/// Reject any username that could escape a single path segment. Centralized +/// so pending / recovery / future stores don't drift on what's "safe." +pub(crate) fn join_user_segment(dir: &Path, user: &str) -> Result { + if user.is_empty() + || user == "." + || user == ".." + || user.contains('/') + || user.contains('\0') + { + return Err(SegmentError::Invalid(user.to_string())); + } + Ok(dir.join(user)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_normal_username() { + let p = join_user_segment(Path::new("/tmp/x"), "alice").unwrap(); + assert_eq!(p, PathBuf::from("/tmp/x/alice")); + } + + #[test] + fn rejects_traversal_and_separators() { + for evil in ["", ".", "..", "a/b", "../etc", "x\0y"] { + assert!( + join_user_segment(Path::new("/tmp/x"), evil).is_err(), + "{evil:?} should be rejected" + ); + } + } +} From 402addca27dd7ef4c1ab1dc72ab21b77f6a7b359 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:23:20 -0700 Subject: [PATCH 2/9] chore(daemon): add argon2 0.5 for recovery code hashing Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 46 ++++++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + daemon/Cargo.toml | 1 + 3 files changed, 48 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 028e853..109a9b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -90,6 +90,18 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "argon2" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072" +dependencies = [ + "base64ct", + "blake2", + "cpufeatures", + "password-hash", +] + [[package]] name = "asn1-rs" version = "0.7.1" @@ -283,6 +295,7 @@ name = "authforge-daemon" version = "0.1.0" dependencies = [ "anyhow", + "argon2", "authforge-common", "ctap-hid-fido2", "futures-util", @@ -325,6 +338,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64ct" +version = "1.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2af50177e190e07a26ab74f8b1efbfe2ef87da2116221318cb1c2e82baf7de06" + [[package]] name = "bitflags" version = "1.3.2" @@ -337,6 +356,15 @@ version = "2.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" +[[package]] +name = "blake2" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" +dependencies = [ + "digest", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -642,6 +670,7 @@ checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer", "crypto-common", + "subtle", ] [[package]] @@ -1694,6 +1723,17 @@ dependencies = [ "windows-link", ] +[[package]] +name = "password-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +dependencies = [ + "base64ct", + "rand_core 0.6.4", + "subtle", +] + [[package]] name = "pin-project-lite" version = "0.2.17" @@ -2126,6 +2166,12 @@ dependencies = [ "syn", ] +[[package]] +name = "subtle" +version = "2.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" + [[package]] name = "syn" version = "2.0.117" diff --git a/Cargo.toml b/Cargo.toml index 864c215..ba1b197 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,3 +33,4 @@ notify = "6" rusqlite = { version = "0.31", features = ["bundled"] } futures-util = "0.3" hex = "0.4" +argon2 = "0.5" diff --git a/daemon/Cargo.toml b/daemon/Cargo.toml index 3da75b7..5a85551 100644 --- a/daemon/Cargo.toml +++ b/daemon/Cargo.toml @@ -26,6 +26,7 @@ rusqlite = { workspace = true } toml = { workspace = true } ctap-hid-fido2 = { workspace = true } hex = { workspace = true } +argon2 = { workspace = true } [dev-dependencies] tempfile = { workspace = true } From 45ac398730f2b9794288c6834be4403cef855fd2 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:25:30 -0700 Subject: [PATCH 3/9] feat(daemon): recovery code generation and Argon2id hashing Pure-logic module: generate 8-digit codes, Argon2id PHC hash with OS RNG salt, verify via constant-time PasswordVerifier. Dead-code allow until Task 4 wires the store on top. Co-Authored-By: Claude Opus 4.7 (1M context) --- daemon/src/main.rs | 1 + daemon/src/recovery.rs | 78 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 daemon/src/recovery.rs diff --git a/daemon/src/main.rs b/daemon/src/main.rs index 7e9e723..bdb4397 100644 --- a/daemon/src/main.rs +++ b/daemon/src/main.rs @@ -7,6 +7,7 @@ mod fido; mod lockout; mod policy_apply; mod polkit; +mod recovery; mod state; mod storage; diff --git a/daemon/src/recovery.rs b/daemon/src/recovery.rs new file mode 100644 index 0000000..f5c3c54 --- /dev/null +++ b/daemon/src/recovery.rs @@ -0,0 +1,78 @@ +//! Pure recovery-code logic: generation, Argon2id hashing, verification. +//! No I/O — that lives in `storage::recovery::RecoveryStore`. + +#![allow(dead_code)] // wired through RecoveryStore (Task 4) and AppState (Task 5). + +use argon2::{ + password_hash::{rand_core::OsRng as PhcOsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString}, + Argon2, +}; +use rand::Rng; + +/// Default validity window for a freshly generated recovery code. 7 days: +/// long enough for an admin to mail the code to a remote user, short enough +/// to limit blast radius if the file leaks. +pub(crate) const DEFAULT_TTL_SECS: u64 = 7 * 24 * 3600; + +#[derive(Debug, thiserror::Error)] +pub(crate) enum RecoveryError { + #[error("argon2: {0}")] + Argon2(String), +} + +/// Generate a fresh 8-digit recovery code. +pub(crate) fn generate_code() -> String { + let n: u32 = rand::rng().random_range(0..100_000_000); + format!("{n:08}") +} + +/// Hash a code with Argon2id, producing a PHC-format string. +pub(crate) fn hash_code(code: &str) -> Result { + let salt = SaltString::generate(&mut PhcOsRng); + let argon = Argon2::default(); + let phc = argon + .hash_password(code.as_bytes(), &salt) + .map_err(|e| RecoveryError::Argon2(e.to_string()))? + .to_string(); + Ok(phc) +} + +/// Constant-time verify a candidate code against a stored PHC hash. +pub(crate) fn verify_code(candidate: &str, phc: &str) -> Result { + let parsed = PasswordHash::new(phc).map_err(|e| RecoveryError::Argon2(e.to_string()))?; + Ok(Argon2::default() + .verify_password(candidate.as_bytes(), &parsed) + .is_ok()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn generated_code_is_eight_digits() { + for _ in 0..50 { + let c = generate_code(); + assert_eq!(c.len(), 8); + assert!(c.chars().all(|ch| ch.is_ascii_digit())); + } + } + + #[test] + fn hash_roundtrips_correct_code() { + let phc = hash_code("12345678").unwrap(); + assert!(phc.starts_with("$argon2id$"), "phc was {phc}"); + assert!(verify_code("12345678", &phc).unwrap()); + } + + #[test] + fn verify_rejects_wrong_code() { + let phc = hash_code("12345678").unwrap(); + assert!(!verify_code("87654321", &phc).unwrap()); + } + + #[test] + fn verify_errors_on_malformed_phc() { + assert!(verify_code("12345678", "not a phc string").is_err()); + } +} From 517b015996d6f26d2d85a5e6755c601585784168 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:32:40 -0700 Subject: [PATCH 4/9] feat(daemon): RecoveryStore with atomic 0600 writes and one-shot verify-and-consume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-line file format (expires_unix\nargon2id-PHC) keeps the C PAM module parser trivial — no json-c link needed. Atomic temp+rename means a concurrent reader never sees a half-written file. Co-Authored-By: Claude Opus 4.7 (1M context) --- daemon/src/storage/mod.rs | 1 + daemon/src/storage/recovery.rs | 247 +++++++++++++++++++++++++++++++++ 2 files changed, 248 insertions(+) create mode 100644 daemon/src/storage/recovery.rs diff --git a/daemon/src/storage/mod.rs b/daemon/src/storage/mod.rs index 1d7d54b..8f189b0 100644 --- a/daemon/src/storage/mod.rs +++ b/daemon/src/storage/mod.rs @@ -1,5 +1,6 @@ pub(crate) mod credentials; pub(crate) mod pending; pub(crate) mod policy; +pub(crate) mod recovery; pub(crate) mod safe_user; pub(crate) mod userdb; diff --git a/daemon/src/storage/recovery.rs b/daemon/src/storage/recovery.rs new file mode 100644 index 0000000..a5335cd --- /dev/null +++ b/daemon/src/storage/recovery.rs @@ -0,0 +1,247 @@ +//! On-disk persistence for recovery codes. One file per user under +//! `cfg.recovery_dir`, mode 0600, root-owned. +//! +//! File format (two lines, UTF-8, trailing LFs): +//! \n\n +//! Picked over JSON so the C PAM module can parse it without linking json-c. + +#![allow(dead_code)] // wired through AppState in Task 5. + +use crate::recovery::{hash_code, verify_code}; +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub(crate) enum RecoveryStoreError { + #[error("io: {0}")] + Io(#[from] std::io::Error), + #[error("invalid username: {0:?}")] + InvalidUser(String), + #[error("malformed recovery file for {0:?}")] + Malformed(String), + #[error("hash error: {0}")] + Hash(String), + #[error("system clock before unix epoch")] + Clock, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct RecoveryEntry { + pub user: String, + pub expires_unix: u64, +} + +pub(crate) struct RecoveryStore { + dir: PathBuf, +} + +impl RecoveryStore { + pub fn new(dir: PathBuf) -> Self { + Self { dir } + } + + fn user_path(&self, user: &str) -> Result { + super::safe_user::join_user_segment(&self.dir, user).map_err(|e| match e { + super::safe_user::SegmentError::Invalid(s) => RecoveryStoreError::InvalidUser(s), + }) + } + + /// Generate, hash, and persist a fresh code for `user`. The file is + /// written atomically (temp + rename) so a concurrent reader never sees + /// a half-written file. Mode is set to 0600 before rename. Returns the + /// plaintext code so the caller can show it to the admin once. + pub fn issue(&self, user: &str, ttl_secs: u64) -> Result { + fs::create_dir_all(&self.dir)?; + // Tighten directory mode so a non-root snoop can't list users. + fs::set_permissions(&self.dir, fs::Permissions::from_mode(0o700))?; + + let path = self.user_path(user)?; + let code = crate::recovery::generate_code(); + let phc = hash_code(&code).map_err(|e| RecoveryStoreError::Hash(e.to_string()))?; + let expires = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|_| RecoveryStoreError::Clock)? + .as_secs() + .saturating_add(ttl_secs); + + let body = format!("{expires}\n{phc}\n"); + let tmp = path.with_extension("tmp"); + fs::write(&tmp, body.as_bytes())?; + fs::set_permissions(&tmp, fs::Permissions::from_mode(0o600))?; + fs::rename(&tmp, &path)?; + Ok(code) + } + + /// List all users with a recovery code on file (expired or not). Sorted + /// alphabetically for stable display. + pub fn list(&self) -> Result, RecoveryStoreError> { + let mut out = Vec::new(); + let rd = match fs::read_dir(&self.dir) { + Ok(r) => r, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(out), + Err(e) => return Err(e.into()), + }; + for entry in rd { + let entry = entry?; + if !entry.file_type()?.is_file() { + continue; + } + let user = entry.file_name().to_string_lossy().to_string(); + // Skip atomic-rename leftovers. + if user.ends_with(".tmp") { + continue; + } + if let Ok(e) = self.entry_for(&user) { + out.push(e); + } + } + out.sort_by(|a, b| a.user.cmp(&b.user)); + Ok(out) + } + + pub fn entry_for(&self, user: &str) -> Result { + let path = self.user_path(user)?; + let body = fs::read_to_string(&path)?; + let mut lines = body.lines(); + let expires: u64 = lines + .next() + .ok_or_else(|| RecoveryStoreError::Malformed(user.to_string()))? + .parse() + .map_err(|_| RecoveryStoreError::Malformed(user.to_string()))?; + Ok(RecoveryEntry { + user: user.to_string(), + expires_unix: expires, + }) + } + + /// Delete the recovery file for `user`. Returns true if a file was + /// removed, false if there was nothing there. + pub fn revoke(&self, user: &str) -> Result { + let path = self.user_path(user)?; + match fs::remove_file(path) { + Ok(()) => Ok(true), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(e) => Err(e.into()), + } + } + + /// Verify a candidate code for `user`. On match: deletes the recovery + /// file (one-shot semantics) and returns Ok(true). On expired: deletes + /// the file as a side-effect cleanup and returns Ok(false). + pub fn verify_and_consume( + &self, + user: &str, + candidate: &str, + ) -> Result { + let path = self.user_path(user)?; + let body = match fs::read_to_string(&path) { + Ok(s) => s, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(e) => return Err(e.into()), + }; + let mut lines = body.lines(); + let expires: u64 = lines + .next() + .ok_or_else(|| RecoveryStoreError::Malformed(user.to_string()))? + .parse() + .map_err(|_| RecoveryStoreError::Malformed(user.to_string()))?; + let phc = lines + .next() + .ok_or_else(|| RecoveryStoreError::Malformed(user.to_string()))?; + + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|_| RecoveryStoreError::Clock)? + .as_secs(); + if now > expires { + let _ = fs::remove_file(&path); + return Ok(false); + } + + let ok = verify_code(candidate, phc).map_err(|e| RecoveryStoreError::Hash(e.to_string()))?; + if ok { + let _ = fs::remove_file(&path); + } + Ok(ok) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + fn store() -> (tempfile::TempDir, RecoveryStore) { + let d = tempdir().unwrap(); + let s = RecoveryStore::new(d.path().to_path_buf()); + (d, s) + } + + #[test] + fn issue_then_verify_consumes_file() { + let (_d, s) = store(); + let code = s.issue("alice", 600).unwrap(); + assert!(s.entry_for("alice").is_ok()); + assert!(s.verify_and_consume("alice", &code).unwrap()); + assert!(matches!( + s.entry_for("alice"), + Err(RecoveryStoreError::Io(_)) + )); + } + + #[test] + fn verify_with_wrong_code_does_not_consume() { + let (_d, s) = store(); + s.issue("alice", 600).unwrap(); + assert!(!s.verify_and_consume("alice", "00000000").unwrap()); + assert!(s.entry_for("alice").is_ok()); + } + + #[test] + fn expired_entry_is_treated_as_absent_and_cleaned_up() { + let (_d, s) = store(); + s.issue("alice", 0).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(1100)); + assert!(!s.verify_and_consume("alice", "anything").unwrap()); + assert!(matches!( + s.entry_for("alice"), + Err(RecoveryStoreError::Io(_)) + )); + } + + #[test] + fn revoke_returns_false_on_missing_user() { + let (_d, s) = store(); + assert!(!s.revoke("ghost").unwrap()); + } + + #[test] + fn list_sorts_users_alphabetically() { + let (_d, s) = store(); + s.issue("charlie", 600).unwrap(); + s.issue("alice", 600).unwrap(); + s.issue("bob", 600).unwrap(); + let users: Vec<_> = s.list().unwrap().into_iter().map(|e| e.user).collect(); + assert_eq!(users, vec!["alice", "bob", "charlie"]); + } + + #[test] + fn rejects_traversal_usernames() { + let (_d, s) = store(); + for evil in ["", ".", "..", "a/b", "x\0y"] { + assert!(s.issue(evil, 600).is_err()); + } + } + + #[test] + fn issue_writes_file_with_mode_0600() { + let (_d, s) = store(); + s.issue("alice", 600).unwrap(); + let path = s.user_path("alice").unwrap(); + let mode = fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + } +} From e9efde9077d2ad6b74b0b6150ce081e30974e2c0 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:35:06 -0700 Subject: [PATCH 5/9] feat(daemon): wire RecoveryStore into AppState with AUTHFORGE_RECOVERY_DIR override StorageConfig gains a recovery_dir field; from_env_or_defaults reads AUTHFORGE_RECOVERY_DIR (default /var/lib/authforge/recovery). AppState exposes issue/list/revoke methods that Task 6 wires through D-Bus. Test fixtures in state.rs and dbus.rs updated. Co-Authored-By: Claude Opus 4.7 (1M context) --- daemon/src/dbus.rs | 1 + daemon/src/state.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/daemon/src/dbus.rs b/daemon/src/dbus.rs index f3ec7b3..4cd6f70 100644 --- a/daemon/src/dbus.rs +++ b/daemon/src/dbus.rs @@ -220,6 +220,7 @@ central_path = "{}" crate::state::StorageConfig { policy_dir, pending_dir: tmp.path().join("pending"), + recovery_dir: tmp.path().join("recovery"), userdb_path: tmp.path().join("users.db"), pam_profile_path: tmp.path().join("authforge-pamconf"), pam_auth_update: shim, diff --git a/daemon/src/state.rs b/daemon/src/state.rs index dbaa378..b5341d9 100644 --- a/daemon/src/state.rs +++ b/daemon/src/state.rs @@ -4,6 +4,7 @@ use crate::policy_apply::{ApplyError, PolicyApplier}; use crate::storage::credentials::{CredEntry, CredentialsStore, CredsError, CredsPathResolver}; use crate::storage::pending::{PendingError, PendingStore}; use crate::storage::policy::PolicyStore; +use crate::storage::recovery::{RecoveryEntry, RecoveryStore, RecoveryStoreError}; use crate::storage::userdb::{UserDb, UserDbError}; use authforge_common::policy::{Policy, PolicyError}; use authforge_common::types::{Credential, Method, PendingFlag, PolicyApplyResult, Transport}; @@ -20,6 +21,8 @@ pub enum StateError { #[error(transparent)] Pending(#[from] PendingError), #[error(transparent)] + Recovery(#[from] RecoveryStoreError), + #[error(transparent)] Creds(#[from] CredsError), #[error(transparent)] UserDb(#[from] UserDbError), @@ -32,6 +35,7 @@ pub enum StateError { pub struct StorageConfig { pub policy_dir: PathBuf, pub pending_dir: PathBuf, + pub recovery_dir: PathBuf, pub userdb_path: PathBuf, /// Where pam-auth-update reads the AuthForge profile from. Defaults to /// `/usr/share/pam-configs/authforge`; tests / non-root runs override. @@ -51,6 +55,7 @@ impl StorageConfig { Self { policy_dir: env("AUTHFORGE_POLICY_DIR", "/etc/authforge/policy.d"), pending_dir: env("AUTHFORGE_PENDING_DIR", "/var/lib/authforge/pending"), + recovery_dir: env("AUTHFORGE_RECOVERY_DIR", "/var/lib/authforge/recovery"), userdb_path: env("AUTHFORGE_USERDB", "/var/lib/authforge/users.db"), pam_profile_path: env("AUTHFORGE_PAMCONF_PATH", "/usr/share/pam-configs/authforge"), pam_auth_update: env("AUTHFORGE_PAM_AUTH_UPDATE", "/usr/sbin/pam-auth-update"), @@ -61,6 +66,8 @@ impl StorageConfig { pub struct AppState { policy: PolicyStore, pending: PendingStore, + #[allow(dead_code)] // wired through D-Bus in Task 6. + recovery: RecoveryStore, userdb: Mutex, authn: Arc, applier: PolicyApplier, @@ -70,11 +77,13 @@ impl AppState { pub fn open(cfg: StorageConfig, authn: Arc) -> Result { let policy = PolicyStore::new(cfg.policy_dir); let pending = PendingStore::new(cfg.pending_dir); + let recovery = RecoveryStore::new(cfg.recovery_dir); let userdb = Mutex::new(UserDb::open(&cfg.userdb_path)?); let applier = PolicyApplier::new(cfg.pam_profile_path, cfg.pam_auth_update); Ok(Self { policy, pending, + recovery, userdb, authn, applier, @@ -222,6 +231,21 @@ impl AppState { Ok(self.pending.clear(user)?) } + #[allow(dead_code)] // wired through D-Bus in Task 6. + pub async fn issue_recovery(&self, user: &str) -> Result { + Ok(self.recovery.issue(user, crate::recovery::DEFAULT_TTL_SECS)?) + } + + #[allow(dead_code)] // wired through D-Bus in Task 6. + pub async fn list_recovery(&self) -> Result, StateError> { + Ok(self.recovery.list()?) + } + + #[allow(dead_code)] // wired through D-Bus in Task 6. + pub async fn revoke_recovery(&self, user: &str) -> Result { + Ok(self.recovery.revoke(user)?) + } + /// Tests assert pending flag round-trips through SetPendingFlag / /// ClearPendingFlag via this read accessor. Production readers (PAM /// module, GUI status) read the on-disk file directly in later phases. @@ -250,6 +274,7 @@ mod tests { StorageConfig { policy_dir: d.join("policy.d"), pending_dir: d.join("pending"), + recovery_dir: d.join("recovery"), userdb_path: d.join("users.db"), pam_profile_path: d.join("authforge-pamconf"), pam_auth_update: shim, @@ -309,6 +334,7 @@ mod tests { StorageConfig { policy_dir, pending_dir: d.path().join("pending"), + recovery_dir: d.path().join("recovery"), userdb_path: d.path().join("users.db"), pam_profile_path: d.path().join("authforge-pamconf"), pam_auth_update: shim, From a420aa5f750b0762c65ffa9a9b75041aae4614d6 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:39:16 -0700 Subject: [PATCH 6/9] feat(daemon): real GenerateRecoveryCode + ListRecoveryCodes + RevokeRecoveryCode Replace the random-number stub at dbus.rs:132 with state.issue_recovery, add ListRecoveryCodes (returns Vec) and RevokeRecoveryCode. New polkit actions list-recovery / revoke-recovery (auth_admin_keep). Adds 4 D-Bus integration tests; the previously-stub generate-code test now exercises the real Argon2id-backed write path. Co-Authored-By: Claude Opus 4.7 (1M context) --- common/src/types.rs | 6 ++ daemon/src/dbus.rs | 85 ++++++++++++++++++++-- daemon/src/recovery.rs | 4 +- daemon/src/state.rs | 4 +- daemon/src/storage/recovery.rs | 3 +- daemon/src/storage/safe_user.rs | 7 +- debian/io.dangerousthings.AuthForge.policy | 20 +++++ 7 files changed, 114 insertions(+), 15 deletions(-) diff --git a/common/src/types.rs b/common/src/types.rs index 2563fa4..bb4cd0a 100644 --- a/common/src/types.rs +++ b/common/src/types.rs @@ -59,6 +59,12 @@ pub struct PolicyApplyResult { pub violations: Vec, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, zvariant::Type)] +pub struct RecoveryCodeSummary { + pub user: String, + pub expires_unix: u64, +} + #[cfg(test)] mod tests { use super::*; diff --git a/daemon/src/dbus.rs b/daemon/src/dbus.rs index 4cd6f70..0bc8bb3 100644 --- a/daemon/src/dbus.rs +++ b/daemon/src/dbus.rs @@ -1,7 +1,7 @@ use crate::polkit::Polkit; use crate::state::AppState; use authforge_common::policy::Policy; -use authforge_common::types::{Credential, PendingFlag, PolicyApplyResult}; +use authforge_common::types::{Credential, PendingFlag, PolicyApplyResult, RecoveryCodeSummary}; use std::sync::Arc; pub struct AuthForge { @@ -129,13 +129,39 @@ impl AuthForge { Ok(()) } - async fn generate_recovery_code(&self, _user: String) -> zbus::fdo::Result { + async fn generate_recovery_code(&self, user: String) -> zbus::fdo::Result { self.authz("io.dangerousthings.AuthForge.generate-recovery") .await?; - // Stub: 8 random digits. Phase 12 replaces with Argon2id-backed real flow. - use rand::Rng; - let n: u32 = rand::rng().random_range(0..100_000_000); - Ok(format!("{n:08}")) + self.state + .issue_recovery(&user) + .await + .map_err(|e| zbus::fdo::Error::Failed(e.to_string())) + } + + async fn list_recovery_codes(&self) -> zbus::fdo::Result> { + self.authz("io.dangerousthings.AuthForge.list-recovery") + .await?; + let entries = self + .state + .list_recovery() + .await + .map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?; + Ok(entries + .into_iter() + .map(|e| RecoveryCodeSummary { + user: e.user, + expires_unix: e.expires_unix, + }) + .collect()) + } + + async fn revoke_recovery_code(&self, user: String) -> zbus::fdo::Result { + self.authz("io.dangerousthings.AuthForge.revoke-recovery") + .await?; + self.state + .revoke_recovery(&user) + .await + .map_err(|e| zbus::fdo::Error::Failed(e.to_string())) } /// Emitted by main.rs whenever the inotify watcher on the policy.d/ dir @@ -370,6 +396,53 @@ central_path = "{}" assert!(code.chars().all(|c| c.is_ascii_digit())); } + #[tokio::test] + async fn list_recovery_returns_issued_users() { + use authforge_common::types::RecoveryCodeSummary; + let (_srv, client, _state, _tmp) = p2p_pair().await; + let p = proxy(&client).await; + let _: String = p.call("GenerateRecoveryCode", &("alice",)).await.unwrap(); + let _: String = p.call("GenerateRecoveryCode", &("bob",)).await.unwrap(); + let entries: Vec = p.call("ListRecoveryCodes", &()).await.unwrap(); + let users: Vec<_> = entries.into_iter().map(|e| e.user).collect(); + assert_eq!(users, vec!["alice".to_string(), "bob".to_string()]); + } + + #[tokio::test] + async fn revoke_recovery_removes_entry() { + use authforge_common::types::RecoveryCodeSummary; + let (_srv, client, _state, _tmp) = p2p_pair().await; + let p = proxy(&client).await; + let _: String = p.call("GenerateRecoveryCode", &("alice",)).await.unwrap(); + let removed: bool = p.call("RevokeRecoveryCode", &("alice",)).await.unwrap(); + assert!(removed); + let entries: Vec = p.call("ListRecoveryCodes", &()).await.unwrap(); + assert!(entries.is_empty()); + } + + #[tokio::test] + async fn revoke_recovery_returns_false_on_missing_user() { + let (_srv, client, _state, _tmp) = p2p_pair().await; + let p = proxy(&client).await; + let removed: bool = p.call("RevokeRecoveryCode", &("ghost",)).await.unwrap(); + assert!(!removed); + } + + #[tokio::test] + async fn generate_recovery_code_persists_argon2id_hash_on_disk() { + let (_srv, client, _state, tmp) = p2p_pair().await; + let p = proxy(&client).await; + let _: String = p.call("GenerateRecoveryCode", &("alice",)).await.unwrap(); + let body = std::fs::read_to_string(tmp.path().join("recovery").join("alice")).unwrap(); + let mut lines = body.lines(); + // Line 1: expires_unix (numeric). + let expires: u64 = lines.next().unwrap().parse().unwrap(); + assert!(expires > 0); + // Line 2: argon2id PHC string. + let phc = lines.next().unwrap(); + assert!(phc.starts_with("$argon2id$"), "phc was {phc}"); + } + #[tokio::test] async fn enrollment_succeeded_signal_fires_on_enroll_own() { use futures_util::stream::StreamExt; diff --git a/daemon/src/recovery.rs b/daemon/src/recovery.rs index f5c3c54..4230bfa 100644 --- a/daemon/src/recovery.rs +++ b/daemon/src/recovery.rs @@ -4,7 +4,9 @@ #![allow(dead_code)] // wired through RecoveryStore (Task 4) and AppState (Task 5). use argon2::{ - password_hash::{rand_core::OsRng as PhcOsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString}, + password_hash::{ + rand_core::OsRng as PhcOsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString, + }, Argon2, }; use rand::Rng; diff --git a/daemon/src/state.rs b/daemon/src/state.rs index b5341d9..8aeaa87 100644 --- a/daemon/src/state.rs +++ b/daemon/src/state.rs @@ -233,7 +233,9 @@ impl AppState { #[allow(dead_code)] // wired through D-Bus in Task 6. pub async fn issue_recovery(&self, user: &str) -> Result { - Ok(self.recovery.issue(user, crate::recovery::DEFAULT_TTL_SECS)?) + Ok(self + .recovery + .issue(user, crate::recovery::DEFAULT_TTL_SECS)?) } #[allow(dead_code)] // wired through D-Bus in Task 6. diff --git a/daemon/src/storage/recovery.rs b/daemon/src/storage/recovery.rs index a5335cd..e0499b4 100644 --- a/daemon/src/storage/recovery.rs +++ b/daemon/src/storage/recovery.rs @@ -161,7 +161,8 @@ impl RecoveryStore { return Ok(false); } - let ok = verify_code(candidate, phc).map_err(|e| RecoveryStoreError::Hash(e.to_string()))?; + let ok = + verify_code(candidate, phc).map_err(|e| RecoveryStoreError::Hash(e.to_string()))?; if ok { let _ = fs::remove_file(&path); } diff --git a/daemon/src/storage/safe_user.rs b/daemon/src/storage/safe_user.rs index fdf84a7..413edd8 100644 --- a/daemon/src/storage/safe_user.rs +++ b/daemon/src/storage/safe_user.rs @@ -12,12 +12,7 @@ pub(crate) enum SegmentError { /// Reject any username that could escape a single path segment. Centralized /// so pending / recovery / future stores don't drift on what's "safe." pub(crate) fn join_user_segment(dir: &Path, user: &str) -> Result { - if user.is_empty() - || user == "." - || user == ".." - || user.contains('/') - || user.contains('\0') - { + if user.is_empty() || user == "." || user == ".." || user.contains('/') || user.contains('\0') { return Err(SegmentError::Invalid(user.to_string())); } Ok(dir.join(user)) diff --git a/debian/io.dangerousthings.AuthForge.policy b/debian/io.dangerousthings.AuthForge.policy index 4229ee8..5708116 100644 --- a/debian/io.dangerousthings.AuthForge.policy +++ b/debian/io.dangerousthings.AuthForge.policy @@ -77,4 +77,24 @@ + + List active recovery codes + Administrator authentication is required to list recovery codes. + + auth_admin_keep + auth_admin_keep + auth_admin_keep + + + + + Revoke a recovery code + Administrator authentication is required to revoke a recovery code. + + auth_admin_keep + auth_admin_keep + auth_admin_keep + + + From 56ba4dd924b4f26608c17d561154c6b8a01ec386 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:49:44 -0700 Subject: [PATCH 7/9] feat(pam): recovery-code mode with Argon2id verify and pending re-enroll handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C-side: * New mode=recovery argv branch in pam_sm_authenticate. Reads the two-line /var/lib/authforge/recovery/ file, argon2_verify against PAM_AUTHTOK, on match unlinks the file (one-shot) and writes pending(re_enroll=true). Always returns PAM_IGNORE on failure paths so a missing/wrong code never blocks normal auth. * Makefile links -largon2 alongside -lpam. Daemon-side: * policy_apply::render_profile renders the recovery line first in the auth stack with [success=done default=ignore] — successful recovery short- circuits the rest, missing/wrong code falls through. * New policy_apply test asserts the recovery line precedes the default backstop. Doc: * pam/TESTING.md adds libargon2-dev to the build prereqs and a new Smoke test 4 walking through the manual recovery-code flow. C compile gate (make -C pam) requires libargon2-dev — flagged as a deferred verification step until a host with the dev package is available. Co-Authored-By: Claude Opus 4.7 (1M context) --- daemon/src/policy_apply.rs | 26 +++++++- pam/Makefile | 2 +- pam/TESTING.md | 40 ++++++++++- pam/pam_authforge_pending.c | 129 ++++++++++++++++++++++++++++++++++-- 4 files changed, 188 insertions(+), 9 deletions(-) diff --git a/daemon/src/policy_apply.rs b/daemon/src/policy_apply.rs index 9b7de84..cc903fe 100644 --- a/daemon/src/policy_apply.rs +++ b/daemon/src/policy_apply.rs @@ -93,7 +93,15 @@ pub(crate) fn render_profile(p: &Policy) -> String { .any(|m| matches!(m, authforge_common::types::Method::Fido2)) }); - let auth_lines = if any_required_fido2 { + // Recovery line runs first on every login attempt: PAM_IGNORE when + // there's no recovery file (cheap stat), PAM_SUCCESS via [success=done] + // when the user typed a valid recovery code (short-circuits the rest of + // the auth stack), PAM_IGNORE otherwise. `default=ignore` keeps a + // missing recovery file from blocking normal auth. + let recovery_line = + " [success=done default=ignore] pam_authforge_pending.so mode=recovery"; + + let main_lines = if any_required_fido2 { // Per design doc § pam-auth-update profile. " [success=ok default=1 ignore=ignore] pam_u2f.so cue authfile=/etc/u2f_mappings\n [success=ok default=die] pam_authforge_pending.so".to_string() } else { @@ -102,6 +110,8 @@ pub(crate) fn render_profile(p: &Policy) -> String { " [success=ok default=die] pam_authforge_pending.so".to_string() }; + let auth_lines = format!("{recovery_line}\n{main_lines}"); + let default = if any_required_fido2 { "yes" } else { "no" }; format!( @@ -126,6 +136,20 @@ mod tests { assert!(!body.contains("pam_u2f.so")); } + #[test] + fn render_always_includes_recovery_line_before_other_modules() { + let body = render_profile(&Policy::default()); + let recovery_idx = body.find("mode=recovery").expect("recovery line present"); + let backstop_idx = body + .rfind("pam_authforge_pending.so") + .expect("default backstop present"); + // Recovery line lives BEFORE the default-mode backstop in the rendered profile. + assert!( + recovery_idx < backstop_idx, + "recovery line should precede the default backstop" + ); + } + #[test] fn render_required_fido2_includes_pam_u2f_default_yes() { let mut stacks = BTreeMap::new(); diff --git a/pam/Makefile b/pam/Makefile index 39fd1f5..3f638b8 100644 --- a/pam/Makefile +++ b/pam/Makefile @@ -2,7 +2,7 @@ CFLAGS ?= -Wall -Wextra -Werror -fPIC -O2 LIBDIR ?= /usr/lib/$(shell dpkg-architecture -qDEB_HOST_MULTIARCH)/security pam_authforge_pending.so: pam_authforge_pending.c - $(CC) $(CFLAGS) -shared -o $@ $< -lpam + $(CC) $(CFLAGS) -shared -o $@ $< -lpam -largon2 install: pam_authforge_pending.so install -D -m 0644 pam_authforge_pending.so $(DESTDIR)$(LIBDIR)/pam_authforge_pending.so diff --git a/pam/TESTING.md b/pam/TESTING.md index 830a7ca..993a378 100644 --- a/pam/TESTING.md +++ b/pam/TESTING.md @@ -8,11 +8,13 @@ needs root to install. The recipe below runs in a VM or container with ## Build + install ```bash -sudo apt-get install -y libpam0g-dev pamtester +sudo apt-get install -y libpam0g-dev libargon2-dev pamtester sudo make -C pam install sudo install -D -m 0644 pam/test/authforge.pamd /etc/pam.d/authforge ``` +`libargon2-dev` is required for the recovery-mode verification path (Phase 12). + `make install` drops the `.so` into `/usr/lib/$(dpkg-architecture -qDEB_HOST_MULTIARCH)/security/`. @@ -48,6 +50,42 @@ done Logs show up via `journalctl -t authforge_pending` (or wherever your distribution routes `pam_syslog` — typically `/var/log/auth.log`). +## Smoke test 4: recovery-code path (Phase 12) + +The module's recovery branch activates when invoked with `mode=recovery` in +the PAM stack. The pam-auth-update profile rendered by Phase 4 puts this +line first in the auth stack — a successful recovery code short-circuits +the rest of the auth chain via `[success=done default=ignore]`. + +To exercise it manually with `pamtester` against a custom stack: + +```bash +# 1. Issue a code via the daemon and stash the plaintext. +CODE=$(sudo authforgectl recovery generate "$USER" | tr -d '\n') +echo "issued $CODE" + +# 2. Build a PAM stack file that runs ONLY the recovery-mode module. +sudo tee /etc/pam.d/authforge-recovery <<'EOF' +auth [success=done default=ignore] pam_authforge_pending.so mode=recovery +auth required pam_deny.so +EOF + +# 3. Verify a matching code authenticates. +echo "$CODE" | pamtester -v authforge-recovery "$USER" authenticate +# Expect: PAM_SUCCESS, /var/lib/authforge/recovery/$USER deleted, and +# /var/lib/authforge/pending/$USER now contains re_enroll: true. + +# 4. Verify a non-matching code falls through to pam_deny. +echo "$CODE" | pamtester -v authforge-recovery "$USER" authenticate +# Expect: failure (pam_deny) — the recovery file was already consumed. + +# 5. Cleanup. +sudo rm /etc/pam.d/authforge-recovery /var/lib/authforge/pending/$USER +``` + +The recovery file is at `/var/lib/authforge/recovery/` mode 0600. +The two-line format is `\n\n`. + ## CI status CI cannot run `pamtester` against a real PAM stack without root and a VM, so diff --git a/pam/pam_authforge_pending.c b/pam/pam_authforge_pending.c index 795e8ed..95263f8 100644 --- a/pam/pam_authforge_pending.c +++ b/pam/pam_authforge_pending.c @@ -1,7 +1,12 @@ /* - * pam_authforge_pending.so — backstop for AuthForge first-login enrollment. + * pam_authforge_pending.so — backstop for AuthForge first-login enrollment + * and recovery-code verification. * - * Behavior: + * Modes (selected by argv): + * default — first-login pending-flag check (Phase 6). + * mode=recovery — verify a one-shot recovery code (Phase 12). + * + * Default-mode behavior: * 1. Read PAM_USER. * 2. Reject usernames that would let the path computation escape * /var/lib/authforge/pending/. @@ -10,20 +15,33 @@ * - ENOENT -> return PAM_IGNORE (let the rest of the stack decide). * - other -> log + return PAM_AUTH_ERR (fail closed). * - * Recovery codes (Phase 12) wire into a separate hook that's a no-op here. + * Recovery-mode behavior: + * 1. Read PAM_USER (same sanity checks). + * 2. stat() /var/lib/authforge/recovery/; ENOENT -> PAM_IGNORE. + * 3. Read the file's two lines: \n\n. + * 4. argon2_verify(phc, PAM_AUTHTOK). On match: unlink the recovery file, + * write a pending(re_enroll=true) flag for the user, return PAM_SUCCESS. + * On mismatch / expired / parse error: PAM_IGNORE (never fail closed in + * recovery mode — false negatives must not lock out normal login paths). */ #define PAM_SM_AUTH #include #include +#include #include +#include #include +#include #include #include #include +#include +#include #define PENDING_DIR "/var/lib/authforge/pending/" +#define RECOVERY_DIR "/var/lib/authforge/recovery/" #define USER_MSG "Account setup incomplete. Please complete enrollment in the Authentication app." static int username_is_safe(const char *u) { @@ -47,18 +65,117 @@ static int pending_flag_present(const char *user) { return -1; } +/* Read /var/lib/authforge/recovery/. On success: *expires_out gets + * the UNIX timestamp from line 1, *phc_out is a heap-allocated copy of + * line 2 (caller frees). Returns 0 on success, -1 on any error. */ +static int read_recovery_file(const char *user, + uint64_t *expires_out, + char **phc_out) { + *phc_out = NULL; + char path[1024]; + int n = snprintf(path, sizeof(path), "%s%s", RECOVERY_DIR, user); + if (n < 0 || (size_t)n >= sizeof(path)) return -1; + FILE *f = fopen(path, "r"); + if (!f) return -1; + char line1[64]; + char line2[256]; + if (!fgets(line1, sizeof(line1), f) || !fgets(line2, sizeof(line2), f)) { + fclose(f); + return -1; + } + fclose(f); + line1[strcspn(line1, "\r\n")] = '\0'; + line2[strcspn(line2, "\r\n")] = '\0'; + char *end = NULL; + unsigned long long parsed = strtoull(line1, &end, 10); + if (end == NULL || *end != '\0' || end == line1) return -1; + *expires_out = (uint64_t)parsed; + *phc_out = strdup(line2); + return *phc_out ? 0 : -1; +} + +/* Write a JSON pending(re_enroll=true) flag for `user`. Format mirrors + * authforge_common::types::PendingFlag's serde representation; the daemon + * round-trips it on next read. Returns 0 on success, -1 on error. */ +static int write_pending_re_enroll(const char *user) { + char path[1024]; + int n = snprintf(path, sizeof(path), "%s%s", PENDING_DIR, user); + if (n < 0 || (size_t)n >= sizeof(path)) return -1; + FILE *f = fopen(path, "w"); + if (!f) return -1; + fprintf(f, + "{\n \"required_methods\": [\"fido2\"],\n" + " \"created_unix\": %ld,\n" + " \"deadline_unix\": 0,\n" + " \"re_enroll\": true\n}\n", + (long)time(NULL)); + fclose(f); + chmod(path, 0644); + return 0; +} + +/* Verify a candidate code for `user` against the on-disk recovery file. + * On match: unlinks the recovery file (one-shot semantics), writes a + * pending(re_enroll=true) flag, returns 1. Otherwise returns 0. Never + * fails closed — returns 0 on any I/O / parse / expiry condition. */ +static int recovery_code_matches(const char *user, const char *candidate) { + uint64_t expires = 0; + char *phc = NULL; + if (read_recovery_file(user, &expires, &phc) != 0) return 0; + + int ok = 0; + if ((uint64_t)time(NULL) <= expires) { + if (argon2_verify(phc, candidate, strlen(candidate), Argon2_id) == ARGON2_OK) { + ok = 1; + } + } + + free(phc); + if (ok) { + char path[1024]; + int n = snprintf(path, sizeof(path), "%s%s", RECOVERY_DIR, user); + if (n > 0 && (size_t)n < sizeof(path)) { + unlink(path); + } + write_pending_re_enroll(user); + } + return ok; +} + PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv) { - (void)flags; (void)argc; (void)argv; + (void)flags; + + int recovery_mode = 0; + for (int i = 0; i < argc; i++) { + if (strcmp(argv[i], "mode=recovery") == 0) { + recovery_mode = 1; + break; + } + } const char *user = NULL; if (pam_get_user(pamh, &user, NULL) != PAM_SUCCESS || user == NULL) { pam_syslog(pamh, LOG_ERR, "authforge_pending: pam_get_user failed"); - return PAM_AUTH_ERR; + return recovery_mode ? PAM_IGNORE : PAM_AUTH_ERR; } if (!username_is_safe(user)) { pam_syslog(pamh, LOG_ERR, "authforge_pending: rejected unsafe username"); - return PAM_AUTH_ERR; + return recovery_mode ? PAM_IGNORE : PAM_AUTH_ERR; + } + + if (recovery_mode) { + const char *authtok = NULL; + if (pam_get_authtok(pamh, PAM_AUTHTOK, &authtok, NULL) != PAM_SUCCESS + || authtok == NULL) { + return PAM_IGNORE; + } + if (recovery_code_matches(user, authtok)) { + pam_syslog(pamh, LOG_INFO, + "authforge_pending: recovery code accepted for %s", user); + return PAM_SUCCESS; + } + return PAM_IGNORE; /* let pam_unix / pam_u2f handle */ } int present = pending_flag_present(user); From 982b9403d0325bbdff0f37b33bb585b5238f276c Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:51:52 -0700 Subject: [PATCH 8/9] feat(cli): authforgectl recovery list and revoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * RecoveryCmd::List no longer takes a user — lists all active codes across users, matching the daemon's ListRecoveryCodes signature. * RecoveryCmd::Revoke calls RevokeRecoveryCode; exits 1 with a stderr message when the user has no active code. * bus.rs gains list_recovery_codes() / revoke_recovery_code(user). * Two new clap-parser tests cover the new subcommand shapes. Co-Authored-By: Claude Opus 4.7 (1M context) --- cli/src/bus.rs | 10 +++++++++- cli/src/commands/mod.rs | 29 ++++++++++++++++++++++++++--- cli/src/main.rs | 25 ++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/cli/src/bus.rs b/cli/src/bus.rs index 318fcc0..b831579 100644 --- a/cli/src/bus.rs +++ b/cli/src/bus.rs @@ -4,7 +4,7 @@ use anyhow::{Context, Result}; use authforge_common::policy::Policy; -use authforge_common::types::{Credential, PendingFlag, PolicyApplyResult}; +use authforge_common::types::{Credential, PendingFlag, PolicyApplyResult, RecoveryCodeSummary}; const BUS_NAME: &str = "io.dangerousthings.AuthForge"; const OBJECT_PATH: &str = "/io/dangerousthings/AuthForge"; @@ -56,4 +56,12 @@ impl Daemon { pub async fn generate_recovery_code(&self, user: &str) -> Result { Ok(self.proxy.call("GenerateRecoveryCode", &(user,)).await?) } + + pub async fn list_recovery_codes(&self) -> Result> { + Ok(self.proxy.call("ListRecoveryCodes", &()).await?) + } + + pub async fn revoke_recovery_code(&self, user: &str) -> Result { + Ok(self.proxy.call("RevokeRecoveryCode", &(user,)).await?) + } } diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 9e589fc..492f1bf 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -212,10 +212,33 @@ pub(crate) async fn dispatch(json: bool, cmd: super::Cmd) -> Result<()> { } super::Cmd::Recovery { - cmd: super::RecoveryCmd::List { user: _ }, + cmd: super::RecoveryCmd::List, } => { - if !json { - println!("(ListRecovery lands in Phase 12)"); + let entries = d.list_recovery_codes().await?; + if json { + println!("{}", serde_json::to_string_pretty(&entries)?); + } else if entries.is_empty() { + println!("(no active recovery codes)"); + } else { + for e in &entries { + println!("{:<24} expires_unix={}", e.user, e.expires_unix); + } + } + } + + super::Cmd::Recovery { + cmd: super::RecoveryCmd::Revoke { user }, + } => { + let removed = d.revoke_recovery_code(&user).await?; + if removed { + if !json { + println!("revoked recovery code for {user}"); + } + } else { + if !json { + eprintln!("no recovery code for {user}"); + } + std::process::exit(1); } } } diff --git a/cli/src/main.rs b/cli/src/main.rs index 82d02ce..5c31a1a 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -95,7 +95,8 @@ enum PendingCmd { #[derive(Subcommand)] enum RecoveryCmd { Generate { user: String }, - List { user: String }, + List, + Revoke { user: String }, } #[tokio::main] @@ -174,4 +175,26 @@ mod tests { let c = Cli::try_parse_from(["authforgectl", "--json", "status"]).unwrap(); assert!(c.json); } + + #[test] + fn parse_recovery_list_takes_no_args() { + let c = Cli::try_parse_from(["authforgectl", "recovery", "list"]).unwrap(); + assert!(matches!( + c.cmd, + Cmd::Recovery { + cmd: RecoveryCmd::List + } + )); + } + + #[test] + fn parse_recovery_revoke_with_user() { + let c = Cli::try_parse_from(["authforgectl", "recovery", "revoke", "alice"]).unwrap(); + match c.cmd { + Cmd::Recovery { + cmd: RecoveryCmd::Revoke { user }, + } => assert_eq!(user, "alice"), + _ => panic!("wrong subcommand"), + } + } } From 91731150d55e0f642567a1de0a0dee49701b7840 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:53:09 -0700 Subject: [PATCH 9/9] docs: phase 12 backend closeout notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flips Phase 12 status to ✅ Code complete; bumps progress to 12/19 (63%). Closeout block summarizes the 8 implementation tasks, lists all new artifacts, calls out the 80-test count (was 60), and flags the Phase 14 deferred verifications (libargon2-dev compile + pamtester smoke). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-04-26-authforge-implementation.md | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-04-26-authforge-implementation.md b/docs/plans/2026-04-26-authforge-implementation.md index fd0719f..d5070d5 100644 --- a/docs/plans/2026-04-26-authforge-implementation.md +++ b/docs/plans/2026-04-26-authforge-implementation.md @@ -39,7 +39,7 @@ | 9 | GUI: Policy tab + lockout-warning UX | 4 days | **Spec'd** — opens after Phase 8 | | 10 | First-login flow: autostart entry + fullscreen modal | 4 days | **Spec'd** — needs Phase 6 ✓ + Phase 8 | | 11 | TOTP support (PAM module + GUI tab) — feature flag | 5 days | **Spec'd** — opens now (needs Phase 2 ✓ + Phase 4 ✓) | -| 12 | Recovery flow (codes + emergency unlock) | 4 days | **Spec'd** — opens now (needs Phase 6 ✓) | +| 12 | Recovery flow (codes + emergency unlock) | 4 days | ✅ **Code complete** (2026-04-27) — backend + PAM landed; GUI tab queued in Phase 9 lane | | 13 | Debian packaging finalization (postinst, debconf, purge) | 4 days | **Spec'd** — sequential tail; needs all binaries built | | 14 | Launchpad PPA build setup | 2 days | **Spec'd** — sequential tail after 13; **VM smoke gate for all "Code complete" phases** | | 15 | `authforge-gnome-integration` (Users panel shortcut) | 3 days | ✅ **Done** (2026-04-27) — parallel subagent (gnome lane) | @@ -48,7 +48,7 @@ | 18 | User docs + onboarding site | 3 days | **Spec'd** — anytime slot | | **R** | **v1.0 release** | 1 day | — | -**Progress: 11 of 19 phases code-complete (58%).** Total original estimate was ~14 weeks of focused work for v1.0. Subsequent phases (Debian packaging, KDE port, RPM) are scoped in the design doc. +**Progress: 12 of 19 phases code-complete (63%).** Total original estimate was ~14 weeks of focused work for v1.0. Subsequent phases (Debian packaging, KDE port, RPM) are scoped in the design doc. --- @@ -230,6 +230,29 @@ Both delivered by parallel subagents in `git worktree` isolation while Lane 1 (P - All 5 YAML files validated via `yaml.safe_load`. `ansible-lint` skipped — not installed in the harness sandbox. - **Subagent-initiated divergences (kept):** added `authforge_ppa` override variable so internal-mirror users don't fork the role; pre-creates `/etc/authforge/policy.d/` defensively (idempotent); standard `ansible_managed` template header. +### Phase 12 backend closeout notes (2026-04-27) + +Backend + PAM landed via the plan at [2026-04-27-phase-12-recovery-backend.md](2026-04-27-phase-12-recovery-backend.md). 9 commits on `feature/phase-12-recovery-backend`. GUI tab is queued in the Phase 9 lane (waiting on Phase 8). + +**Done:** +- `daemon/src/storage/safe_user.rs` — extracted shared username path-segment sanitizer; `PendingStore` migrated to it. 2 new unit tests. +- `daemon/src/recovery.rs` — pure logic: `generate_code` (8 digits), `hash_code` (Argon2id PHC via OS RNG salt), `verify_code` (constant-time). 4 unit tests. +- `daemon/src/storage/recovery.rs` — `RecoveryStore`: atomic 0600 writes (temp+rename), two-line file format `\n\n`, one-shot `verify_and_consume` semantics, expiry handling, alphabetical `list()`. 7 unit tests. +- `StorageConfig` gains `recovery_dir` field with `AUTHFORGE_RECOVERY_DIR` env override (default `/var/lib/authforge/recovery`). `AppState` exposes `issue_recovery` / `list_recovery` / `revoke_recovery`. +- D-Bus: real `GenerateRecoveryCode` (replaced stub), new `ListRecoveryCodes` (returns `Vec`), new `RevokeRecoveryCode`. polkit policy gains `…list-recovery` + `…revoke-recovery` actions (`auth_admin_keep`). 4 new D-Bus integration tests including a real on-disk `$argon2id$` PHC round-trip. +- PAM module gains a `mode=recovery` argv branch. Reads the two-line file, `argon2_verify`s against `PAM_AUTHTOK`; on match unlinks the recovery file (one-shot) and writes `pending(re_enroll=true)`. Always returns `PAM_IGNORE` on failure paths so a missing/wrong code never blocks normal auth. Linked against `libargon2`. +- `policy_apply::render_profile` renders the recovery line first in the auth stack with `[success=done default=ignore]` so a successful recovery short-circuits the rest of the auth chain. +- CLI: `authforgectl recovery list` (was placeholder) and `authforgectl recovery revoke `. List takes no args (matches D-Bus signature). + +**Test count:** 7 cli + 13 common + 60 daemon = **80 tests** (was 5+13+42=60 at end of Phase 4+5). +20 tests across the 7 implementation tasks. + +**Plan deviations:** none of substance. + +**Deferred until Phase 14 VM smoke:** +- `make -C pam` against a host with `libargon2-dev` installed (not in the dev sandbox). +- `pamtester` recovery walkthrough — recipe lives in `pam/TESTING.md` § Smoke test 4. +- End-to-end recovery flow against a real PAM stack: admin issues code via CLI → user logs in entering it at the password prompt → recovery file deleted → pending(re_enroll=true) flag present → re-enrollment modal triggers on next login. + --- # Phase 0: Repository Scaffolding (FULLY DETAILED)