From 94289865c056815952416906b821fc50d9001922 Mon Sep 17 00:00:00 2001 From: Breadway Date: Sun, 23 Aug 2026 14:38:50 +0800 Subject: [PATCH] Fail-secure bread.command.lock.unlock: never loginctl Bus clients must not unlock a locked session. Already-unlocked acks .unlock.done; a running locker emits .failed (PAM only). Super+L / hypridle remain loginctl lock-session. --- EVENTS.md | 27 ++++----- README.md | 18 +++--- breadlock/src/bread_events.rs | 104 +++++++++++++++------------------- breadlock/src/main.rs | 13 +++-- 4 files changed, 80 insertions(+), 82 deletions(-) diff --git a/EVENTS.md b/EVENTS.md index 6e0122d..8ab67b6 100644 --- a/EVENTS.md +++ b/EVENTS.md @@ -30,22 +30,23 @@ from session lock/unlock. | `bread.lock.unlocked` | `{}` | PAM authenticated successfully and breadlock sent `unlock` to the compositor, **or** the compositor ended an already-active lock (`SessionLockHandler::finished` after `locked` — breadlock sends `unlock_and_destroy` then emits this). Not emitted when the lock was never acquired (`finished` before `locked`), on a dispatch-error exit (fail-secure: the session stays locked), or a failed/typo password. | | `bread.lock.lock.done` | `{}` | `bread.command.lock.lock` was honored: the locker was already running, or a locker process was started (same no-args invocation as hypridle's `lock_cmd = breadlock`). This is the command confirmation, not compositor proof — wait on `bread.lock.locked` if you need the session-lock protocol to have completed. | | `bread.lock.lock.failed` | `{ "error": "" }` | `bread.command.lock.lock` was received but the locker could not be started (e.g. this binary is missing from disk). | -| `bread.lock.unlock.done` | `{}` | `bread.command.lock.unlock` was honored: no locker was running (already unlocked), or `loginctl unlock-session` was invoked for this session. This is the command confirmation, not compositor proof — wait on `bread.lock.unlocked` if you need PAM + `ext-session-lock-v1` unlock. | -| `bread.lock.unlock.failed` | `{ "error": "" }` | `bread.command.lock.unlock` was received but `loginctl unlock-session` could not be run (binary missing, non-zero exit). | +| `bread.lock.unlock.done` | `{}` | `bread.command.lock.unlock` was honored because no locker was running (session already unlocked). This is **not** passwordless compositor unlock and is **not** emitted merely because a bus client asked to unlock. For PAM + `ext-session-lock-v1` unlock, wait on `bread.lock.unlocked`. | +| `bread.lock.unlock.failed` | `{ "error": "" }` | `bread.command.lock.unlock` was received while the locker is running. The bus cannot bypass PAM; authenticate at the lock screen. | ## Commands honored (`bread.command.lock.*`) | Verb | Effect | |------|--------| | `lock` | If a locker is already running, emit `bread.lock.lock.done` and do nothing else. Otherwise start `breadlock` the same way hypridle does (`lock_cmd = breadlock`: this binary, no args) and emit `done` or `failed`. | -| `unlock` | If no locker is running, emit `bread.lock.unlock.done` (already unlocked). Otherwise run `loginctl unlock-session` on the caller's session and emit `done` or `failed`. This is session-level (logind), not a passwordless PAM bypass: breadlock does not call compositor `unlock()` for this verb. | +| `unlock` | If no locker is running, emit `bread.lock.unlock.done` (already unlocked) and do **not** call loginctl. If the locker is running, emit `bread.lock.unlock.failed` — bus clients must never trigger unlock. Compositor `unlock()` stays on the PAM path only. | A Lua workflow that wants the session locked should `bread.wait` / `bread.wait_any` on `bread.lock.lock.done` (or `.failed`) with a timeout. To know the compositor actually locked, wait on `bread.lock.locked`. -The same pattern applies to unlock: wait on `bread.lock.unlock.done` / -`.failed` for the command, and on `bread.lock.unlocked` for PAM + -compositor unlock. +Unlock from the bus is not a substitute for PAM: wait on +`bread.lock.unlock.done` / `.failed` for the command ack (`.done` only +means already unlocked), and on `bread.lock.unlocked` for a typed +password + compositor unlock. ### Who is listening @@ -60,8 +61,8 @@ rule). Two subscribers exist: idempotent `done`. 2. **The locker process** — always subscribes once the lock screen is up, so `lock` during an active lock is an idempotent `done`, and - `unlock` runs `loginctl unlock-session` rather than compositor - `unlock()`. + `unlock` is `.failed` (cannot bypass PAM). Never compositor + `unlock()`, never `loginctl unlock-session`. ### Session-level equivalent @@ -72,11 +73,11 @@ bread command bus. It is the session-level equivalent of same `ext-session-lock-v1` request. Prefer `loginctl lock-session` from a keybind; prefer the bus command from a Lua workflow. -`loginctl unlock-session` is the matching session-level unlock. The -bus verb invokes that same command. Compositor unlock after a typed -password is still PAM on this process (`bread.lock.unlocked`); a -dispatch-error or crash path still does **not** call compositor -`unlock()` (fail-secure). +The bus unlock verb does **not** call `loginctl unlock-session` and +does **not** replace PAM. Super+L / hypridle remain `loginctl +lock-session`. Compositor unlock after a typed password is still PAM +on this process (`bread.lock.unlocked`); a dispatch-error or crash +path still does **not** call compositor `unlock()` (fail-secure). ### Not implemented: `pin` / `blur` diff --git a/README.md b/README.md index 95fe9eb..c8e418c 100644 --- a/README.md +++ b/README.md @@ -17,11 +17,13 @@ honors `bread.command.lock.lock` / `bread.command.lock.unlock` (emits `bread.lock.lock.done` / `.failed` and `bread.lock.unlock.done` / `.failed`). Run `breadlock listen` so both commands work while unlocked; the locker also subscribes while the session is locked. -Unlock is `loginctl unlock-session` at the session level — not a -passwordless compositor `unlock()`. Super+L remains -`loginctl lock-session` (hypridle then runs `breadlock`). See -[EVENTS.md](EVENTS.md). `breadgreet` is not on the bus. There is no -`bakery.toml` (PAM / pacman exception). +Unlock is fail-secure: already-unlocked acks `bread.lock.unlock.done`; +a running locker refuses with `bread.lock.unlock.failed` (only PAM at +the lock screen unlocks). The bus never calls compositor `unlock()` or +`loginctl unlock-session`. Super+L remains `loginctl lock-session` +(hypridle then runs `breadlock`). See [EVENTS.md](EVENTS.md). +`breadgreet` is not on the bus. There is no `bakery.toml` (PAM / +pacman exception). ## Architecture @@ -89,8 +91,10 @@ lock_cmd = breadlock `bread.command.lock.lock` and `bread.command.lock.unlock`. It is not started by hypridle; add it to session startup (`exec-once = breadlock listen`) if a Lua workflow should be able to -lock or unlock the session while it is unlocked. Session-level unlock -is `loginctl unlock-session` (the bus verb runs that). +lock the session while it is unlocked, or to ack already-unlocked. +`bread.command.lock.unlock` does not replace PAM and does not run +`loginctl unlock-session`. Super+L / hypridle remain +`loginctl lock-session`. ## Verification (why this is safe to test without a lockout risk) diff --git a/breadlock/src/bread_events.rs b/breadlock/src/bread_events.rs index 5bd3e11..1a681aa 100644 --- a/breadlock/src/bread_events.rs +++ b/breadlock/src/bread_events.rs @@ -10,9 +10,10 @@ //! locked (already-locked is `bread.lock.lock.done`). `breadlock listen` //! is the unlocked-path subscriber: it starts this same binary the way //! hypridle's `lock_cmd = breadlock` does, and treats unlock as already -//! unlocked (`bread.lock.unlock.done`). Session-level equivalents are -//! `loginctl lock-session` / `loginctl unlock-session`. Unlock never -//! calls compositor `unlock()` — that stays on the PAM path. +//! unlocked (`bread.lock.unlock.done`). If the locker is running, unlock +//! is `bread.lock.unlock.failed` — only PAM at the lock screen may +//! unlock. Super+L / hypridle remain `loginctl lock-session`. Bus unlock +//! never calls compositor `unlock()` or `loginctl unlock-session`. use std::process::{Command, Stdio}; use std::sync::atomic::{AtomicBool, Ordering}; @@ -119,29 +120,16 @@ pub fn honor_lock_command() { honor_lock_command_with(locker_is_running(), start_locker); } -/// Session-level unlock (`loginctl unlock-session` on the caller's -/// session). Does not send compositor `unlock` and does not skip PAM — -/// that stays on the typed-password path. `done` means the command was -/// acted on (or the session was already unlocked), not that -/// `ext-session-lock-v1` has been released — wait on -/// `bread.lock.unlocked` for the compositor confirmation. -fn unlock_session() -> Result<(), String> { - let status = Command::new("loginctl") - .arg("unlock-session") - .stdin(Stdio::null()) - .status() - .map_err(|e| format!("failed to run loginctl unlock-session: {e}"))?; - if status.success() { - Ok(()) - } else { - Err(format!("loginctl unlock-session exited with {status}")) - } -} +/// Payload on `bread.lock.unlock.failed` while the locker is running. +/// Bus clients cannot unlock; only PAM at the lock screen can. +const UNLOCK_REFUSED_WHILE_LOCKED: &str = + "bus unlock cannot bypass PAM; authenticate at the lock screen"; -/// Honor `bread.command.lock.unlock`: already unlocked is success; -/// otherwise ask logind to unlock this session. +/// Honor `bread.command.lock.unlock`. Fail-secure: never compositor +/// `unlock()`, never `loginctl unlock-session`. Already unlocked is +/// `.done`; a running locker is `.failed`. pub fn honor_unlock_command() { - honor_unlock_command_with(locker_is_running(), unlock_session); + honor_unlock_command_with(locker_is_running(), emit_unlock_done, emit_unlock_failed); } fn honor_lock_command_with(locked: bool, start: impl FnOnce() -> Result<(), String>) { @@ -162,22 +150,21 @@ fn honor_lock_command_with(locked: bool, start: impl FnOnce() -> Result<(), Stri } } -fn honor_unlock_command_with(locked: bool, unlock: impl FnOnce() -> Result<(), String>) { +fn honor_unlock_command_with( + locked: bool, + emit_done: impl FnOnce(), + emit_failed: impl FnOnce(&str), +) { if !locked { tracing::info!("bread.command.lock.unlock: already unlocked"); - emit_unlock_done(); + emit_done(); return; } - match unlock() { - Ok(()) => { - tracing::info!("bread.command.lock.unlock: loginctl unlock-session"); - emit_unlock_done(); - } - Err(error) => { - tracing::error!(%error, "bread.command.lock.unlock: failed"); - emit_unlock_failed(&error); - } - } + tracing::error!( + error = UNLOCK_REFUSED_WHILE_LOCKED, + "bread.command.lock.unlock: refused while locked" + ); + emit_failed(UNLOCK_REFUSED_WHILE_LOCKED); } /// Reacts to `bread.command.lock.*`. Unknown verbs are ignored, not stubbed. @@ -312,33 +299,36 @@ mod tests { } #[test] - fn honor_unlock_command_already_unlocked_does_not_call_loginctl() { - let called = std::cell::Cell::new(false); - honor_unlock_command_with(false, || { - called.set(true); - Err("should not run".into()) - }); - assert!(!called.get()); + fn honor_unlock_command_already_unlocked_emits_done() { + let done = Cell::new(false); + let failed = Cell::new(false); + honor_unlock_command_with(false, || done.set(true), |_| failed.set(true)); + assert!(done.get()); + assert!(!failed.get()); } #[test] - fn honor_unlock_command_with_failed_loginctl_runs_unlock() { - let called = Cell::new(false); - honor_unlock_command_with(true, || { - called.set(true); - Err("boom".into()) - }); - assert!(called.get()); + fn honor_unlock_command_while_locked_emits_failed_not_done() { + let done = Cell::new(false); + let failed = Cell::new(false); + honor_unlock_command_with( + true, + || done.set(true), + |e| { + assert_eq!(e, UNLOCK_REFUSED_WHILE_LOCKED); + failed.set(true); + }, + ); + assert!(!done.get()); + assert!(failed.get()); } #[test] - fn honor_unlock_command_with_successful_loginctl_runs_unlock() { - let called = Cell::new(false); - honor_unlock_command_with(true, || { - called.set(true); - Ok(()) - }); - assert!(called.get()); + fn honor_unlock_command_while_locked_error_mentions_pam() { + assert!( + UNLOCK_REFUSED_WHILE_LOCKED.contains("PAM"), + "bus unlock refusal must say it cannot bypass PAM, got {UNLOCK_REFUSED_WHILE_LOCKED:?}" + ); } #[test] diff --git a/breadlock/src/main.rs b/breadlock/src/main.rs index ae4ab0a..665743a 100644 --- a/breadlock/src/main.rs +++ b/breadlock/src/main.rs @@ -55,7 +55,8 @@ fn print_usage() { (no args) lock this session — hypridle lock_cmd / Super+L via loginctl lock-session\n\ listen subscribe to bread.command.lock.lock / unlock so both work while unlocked\n\ \n\ - Session-level: loginctl lock-session / unlock-session.\n\ + Session-level lock: loginctl lock-session (hypridle then runs breadlock).\n\ + Bus unlock does not replace PAM — type the password at the lock screen.\n\ See EVENTS.md for the bus contract." ); } @@ -80,8 +81,9 @@ fn main() { /// Long-running subscriber so `bread.command.lock.lock` / `.unlock` work /// while the session is unlocked. The locker process also subscribes; /// this path is what actually starts breadlock (the same no-args -/// invocation hypridle uses) and what runs `loginctl unlock-session` -/// when a locker is up. One listen process per session. +/// invocation hypridle uses). Unlock while a locker is running is +/// refused (`.failed`); only PAM may unlock. One listen process per +/// session. fn run_listen() { let _guard = match try_acquire(bread_events::LISTEN_APP) { Ok(Acquire::Acquired(g)) => g, @@ -132,8 +134,9 @@ fn run_lock() { let _running = bread_events::enter_lock_process(); // Honor bread.command.lock.lock / unlock while this locker is up - // (already-locked is bread.lock.lock.done; unlock is loginctl, not - // compositor unlock()). Unlocked-path commands need `breadlock listen`. + // (already-locked is bread.lock.lock.done; unlock is .failed — + // never compositor unlock() or loginctl). Unlocked-path commands + // need `breadlock listen`. let _commands = bread_events::subscribe_commands(); let username = auth::username_from_process().unwrap_or_else(|| {