From 887b9ef62028449528285c0eadf8ddd00d0e3bde Mon Sep 17 00:00:00 2001 From: Ty Smith Date: Fri, 3 Jul 2026 14:41:40 -0700 Subject: [PATCH 1/3] feat(polkit): scope face auth to an action allowlist and fail closed on unresolved user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes polkit findings F1 (no action allowlist) and F2 (UID 0 fail-open). A single face match previously authorized *every* polkit action — pkexec, package install, disk mount, user admin. Now the agent offers face auth only for actions in a configurable allowlist (`polkit.face_eligible_actions`, default `["org.freedesktop.login1.lock-sessions"]`); high-risk actions are excluded by default. A non-allowlisted action is declined (Failed) so polkit falls through to the password dialog — a fall-through, never a denial. Users may extend the list; an empty list disables face entirely. Fail closed on unresolved username: `respond_to_polkit` resolves the target name to a uid and refuses if it does not resolve, instead of substituting UID 0 (which authenticated an unresolvable name as root). Container proof: pkg-validate.sh gains a busctl-driven assertion (test/polkit-agent-validate.sh) that the agent declines a non-allowlisted action_id and accepts an allowlisted one at its D-Bus boundary. Both test-deb-pkg and test-rpm-pkg now ship the agent binary and pass 24/24; rpm gains dbus-daemon so the runtime D-Bus block runs on Fedora. Docs: docs/contracts.md (new [polkit] key + Polkit Agent Semantics), docs/security.md (agent scoping section), config/facelock.toml sample. Co-Authored-By: Claude Fable 5 --- config/facelock.toml | 22 +++++ crates/facelock-core/src/config.rs | 116 ++++++++++++++++++++++++++ crates/facelock-polkit/src/main.rs | 98 ++++++++++++++++++++-- docs/contracts.md | 23 ++++++ docs/security.md | 29 +++++++ test/Containerfile.deb-e2e | 4 +- test/Containerfile.rpm-e2e | 12 +-- test/pkg-validate.sh | 6 ++ test/polkit-agent-validate.sh | 126 +++++++++++++++++++++++++++++ 9 files changed, 423 insertions(+), 13 deletions(-) create mode 100644 test/polkit-agent-validate.sh diff --git a/config/facelock.toml b/config/facelock.toml index 6558db8..672f59e 100644 --- a/config/facelock.toml +++ b/config/facelock.toml @@ -312,6 +312,28 @@ # dir = "/var/log/facelock/snapshots" +# ── Polkit ───────────────────────────────────────────────────────── +# +# Controls the facelock polkit authentication agent. Face auth is SCOPED, +# not universal: a single face match must not authorize every privileged +# polkit action (pkexec, package install, disk mount, user admin). +# +# `face_eligible_actions` lists the polkit action_ids for which face auth +# may be offered. Any action NOT in the list is declined by the agent so +# polkit falls through to the password dialog (another agent handles it) — +# it is never denied outright. +# +# The default is a small, low-risk set. High-risk actions — pkexec +# (org.freedesktop.policykit.exec), PackageKit install/remove, udisks mount, +# accounts-service user administration — are intentionally EXCLUDED. Extend +# the list deliberately if you want face to reach further (like a fingerprint +# reader), then restart the agent. An empty list disables face for all +# actions. +# +# [polkit] +# face_eligible_actions = ["org.freedesktop.login1.lock-sessions"] + + # ── TPM ──────────────────────────────────────────────────────────── # # TPM 2.0 settings for sealing the AES encryption key. diff --git a/crates/facelock-core/src/config.rs b/crates/facelock-core/src/config.rs index 659b4fd..1af5b76 100644 --- a/crates/facelock-core/src/config.rs +++ b/crates/facelock-core/src/config.rs @@ -35,6 +35,8 @@ pub struct Config { pub encryption: EncryptionConfig, #[serde(default)] pub audit: AuditConfig, + #[serde(default)] + pub polkit: PolkitConfig, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -489,6 +491,48 @@ impl Default for AuditConfig { } } +/// Configuration for the polkit authentication agent. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PolkitConfig { + /// polkit `action_id`s for which face authentication may be offered. + /// + /// A single face match must NOT authorize every polkit action (pkexec, + /// package install, disk mount, user admin). Any action not in this list + /// is declined by the agent so polkit falls through to the password + /// dialog handled by another agent — it is never denied outright. + /// + /// The default is a small vetted set of low/moderate-sensitivity actions. + /// Users may extend it deliberately (like a fingerprint reader's reach), + /// but high-risk actions are excluded by default. + #[serde(default = "default_face_eligible_actions")] + pub face_eligible_actions: Vec, +} + +impl Default for PolkitConfig { + fn default() -> Self { + Self { + face_eligible_actions: default_face_eligible_actions(), + } + } +} + +impl PolkitConfig { + /// Whether the given polkit `action_id` may be authorized by face auth. + pub fn is_face_eligible(&self, action_id: &str) -> bool { + self.face_eligible_actions.iter().any(|a| a == action_id) + } +} + +/// Default polkit actions eligible for face authentication. +/// +/// Deliberately small and low-risk. High-risk actions — pkexec +/// (`org.freedesktop.policykit.exec`), PackageKit install/remove, udisks +/// mount, and accounts-service user admin — are intentionally EXCLUDED so a +/// single face match cannot become a universal root key. +fn default_face_eligible_actions() -> Vec { + vec!["org.freedesktop.login1.lock-sessions".to_string()] +} + fn default_audit_path() -> String { "/var/log/facelock/audit.jsonl".to_string() } @@ -994,6 +1038,78 @@ key_path = "/etc/facelock/my.key" assert_eq!(config.encryption.key_path, "/etc/facelock/my.key"); } + #[test] + fn polkit_config_default_allowlist_is_low_risk() { + let config = PolkitConfig::default(); + // Default must offer face only for the vetted low-risk action. + assert_eq!( + config.face_eligible_actions, + vec!["org.freedesktop.login1.lock-sessions".to_string()] + ); + assert!(config.is_face_eligible("org.freedesktop.login1.lock-sessions")); + } + + #[test] + fn polkit_config_default_excludes_high_risk_actions() { + let config = PolkitConfig::default(); + // High-risk actions must NOT be face-eligible by default. + for action in [ + "org.freedesktop.policykit.exec", + "org.freedesktop.packagekit.package-install", + "org.freedesktop.udisks2.filesystem-mount", + "org.freedesktop.accounts.user-administration", + ] { + assert!( + !config.is_face_eligible(action), + "{action} must not be face-eligible by default" + ); + } + } + + #[test] + fn polkit_config_defaults_when_section_absent() { + let toml = r#" +[device] +path = "/dev/video0" +"#; + let config = Config::parse(toml).unwrap(); + assert_eq!( + config.polkit.face_eligible_actions, + vec!["org.freedesktop.login1.lock-sessions".to_string()] + ); + } + + #[test] + fn polkit_config_allowlist_is_configurable() { + let toml = r#" +[device] +path = "/dev/video0" +[polkit] +face_eligible_actions = [ + "org.freedesktop.login1.lock-sessions", + "org.freedesktop.udisks2.filesystem-mount", +] +"#; + let config = Config::parse(toml).unwrap(); + assert!(config.polkit.is_face_eligible("org.freedesktop.udisks2.filesystem-mount")); + assert!(config.polkit.is_face_eligible("org.freedesktop.login1.lock-sessions")); + // Still excludes anything the user did not add. + assert!(!config.polkit.is_face_eligible("org.freedesktop.policykit.exec")); + } + + #[test] + fn polkit_config_empty_allowlist_declines_everything() { + let toml = r#" +[device] +path = "/dev/video0" +[polkit] +face_eligible_actions = [] +"#; + let config = Config::parse(toml).unwrap(); + // An explicitly empty list means "never offer face" — no fail-open. + assert!(!config.polkit.is_face_eligible("org.freedesktop.login1.lock-sessions")); + } + #[test] fn warmup_frames_zero() { let toml = r#" diff --git a/crates/facelock-polkit/src/main.rs b/crates/facelock-polkit/src/main.rs index d1a78c5..b839065 100644 --- a/crates/facelock-polkit/src/main.rs +++ b/crates/facelock-polkit/src/main.rs @@ -7,6 +7,7 @@ use zbus::connection::Builder; use zbus::zvariant::Value; use zbus::{Connection, fdo, interface}; +use facelock_core::config::PolkitConfig; use facelock_core::dbus_interface::{BUS_NAME, INTERFACE_NAME, OBJECT_PATH}; /// Polkit authentication agent that attempts face authentication via the @@ -14,6 +15,9 @@ use facelock_core::dbus_interface::{BUS_NAME, INTERFACE_NAME, OBJECT_PATH}; /// password auth). struct PolkitAgent { system_conn: Connection, + /// Actions eligible for face authentication. Any action not in this list + /// is declined so polkit falls through to the password dialog. + polkit_config: PolkitConfig, /// Tracks in-flight authentication attempts by cookie. /// Sending on the oneshot signals cancellation. active_auths: Arc>>>, @@ -32,6 +36,17 @@ impl PolkitAgent { ) -> fdo::Result<()> { tracing::info!(action_id, message, "polkit auth request received"); + // Scope face auth to an allowlist. A single face match must not + // authorize every polkit action. If this action is not eligible, + // decline (return Err) so polkit routes to a password agent — + // this is a fall-through, NOT a denial. + if !self.polkit_config.is_face_eligible(action_id) { + tracing::info!(action_id, "not eligible for face auth, declining"); + return Err(fdo::Error::Failed(format!( + "action '{action_id}' not eligible for face authentication" + ))); + } + let user = extract_username(&identities).unwrap_or_else(current_username); // Set up cancellation channel for this auth attempt. @@ -147,9 +162,10 @@ async fn respond_to_polkit( .await .context("failed to build polkit authority proxy")?; - let uid = nix::unistd::User::from_name(user)? - .map(|u| u.uid.as_raw()) - .unwrap_or(0); + // Fail closed: an unresolvable username must NEVER be sent to polkit as + // UID 0. Resolve the name, then refuse if it does not map to a real uid. + let resolved = nix::unistd::User::from_name(user)?.map(|u| u.uid.as_raw()); + let uid = uid_from_resolution(user, resolved)?; let _: () = proxy .call("AuthenticationAgentResponse2", &(uid, cookie)) @@ -159,6 +175,15 @@ async fn respond_to_polkit( Ok(()) } +/// Map a resolved uid to a value to send to polkit, refusing when the name did +/// not resolve. Never substitutes UID 0 for an unresolved name (which would be +/// a fail-open to root). +fn uid_from_resolution(user: &str, resolved: Option) -> anyhow::Result { + resolved.ok_or_else(|| { + anyhow::anyhow!("cannot resolve user '{user}'; refusing to respond to polkit") + }) +} + /// Register this process as a polkit authentication agent. async fn register_agent(system_conn: &Connection) -> anyhow::Result<()> { let authority = zbus::Proxy::new( @@ -199,20 +224,50 @@ async fn main() -> anyhow::Result<()> { .await .context("failed to connect to system D-Bus")?; + // Load the face-eligible action allowlist from config. If config is + // missing or unreadable, fall back to the safe (restrictive) default — + // never to an open policy. + let polkit_config = match facelock_core::config::Config::load() { + Ok(config) => config.polkit, + Err(e) => { + tracing::warn!(error = %e, "could not load config; using default face-eligible action allowlist"); + PolkitConfig::default() + } + }; + tracing::info!( + actions = ?polkit_config.face_eligible_actions, + "face authentication scoped to allowlisted polkit actions" + ); + let agent = PolkitAgent { system_conn: system_conn.clone(), + polkit_config, active_auths: Arc::new(Mutex::new(HashMap::new())), }; + // Test hook: skip polkit registration and claim a well-known session-bus + // name so the agent's D-Bus boundary can be exercised in a container + // without a live polkit authority. Never used in production. + let skip_register = std::env::var_os("FACELOCK_POLKIT_SKIP_REGISTER").is_some(); + // Build a session bus connection that serves the agent interface. - let _session_conn = Builder::session()? - .serve_at("/org/facelock/PolkitAgent", agent)? + let mut builder = Builder::session()?.serve_at("/org/facelock/PolkitAgent", agent)?; + if skip_register { + builder = builder.name("org.facelock.PolkitAgent")?; + } + let _session_conn = builder .build() .await .context("failed to build session D-Bus connection")?; // Register with polkit on the system bus. - register_agent(&system_conn).await?; + if skip_register { + tracing::warn!( + "FACELOCK_POLKIT_SKIP_REGISTER set: not registering with polkit (test mode)" + ); + } else { + register_agent(&system_conn).await?; + } tracing::info!("facelock polkit agent running, waiting for auth requests"); @@ -258,4 +313,35 @@ mod tests { let name = current_username(); assert!(!name.is_empty()); } + + #[test] + fn uid_resolution_refuses_unresolved_name() { + // An unresolved username (from_name returned Ok(None)) must produce a + // refusal — never fall open to UID 0. + let err = uid_from_resolution("ghost", None).unwrap_err(); + assert!(err.to_string().contains("cannot resolve user 'ghost'")); + } + + #[test] + fn uid_resolution_never_emits_uid_zero_for_unresolved_name() { + // Explicitly assert the fail-open-to-root regression cannot recur: + // no unresolved name may map to uid 0. + assert!(uid_from_resolution("nonexistent-user", None).is_err()); + } + + #[test] + fn uid_resolution_passes_through_resolved_uid() { + // A genuinely resolved uid (including 0 for real root) is returned. + assert_eq!(uid_from_resolution("root", Some(0)).unwrap(), 0); + assert_eq!(uid_from_resolution("alice", Some(1000)).unwrap(), 1000); + } + + #[test] + fn allowlist_gates_face_eligibility() { + // The agent offers face only for allowlisted actions; everything else + // is declined (falls through to password). + let config = PolkitConfig::default(); + assert!(config.is_face_eligible("org.freedesktop.login1.lock-sessions")); + assert!(!config.is_face_eligible("org.freedesktop.policykit.exec")); + } } diff --git a/docs/contracts.md b/docs/contracts.md index 4839e43..d0ce1e0 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -86,6 +86,14 @@ TOML format. All keys optional — camera auto-detected, sensible defaults for e | `[encryption]` | `method` (none/keyfile/tpm), `key_path`, `sealed_key_path` | | `[audit]` | `enabled`, `path`, `rotate_size_mb` | | `[tpm]` | `pcr_binding`, `pcr_indices`, `tcti` | +| `[polkit]` | `face_eligible_actions` | + +`[polkit].face_eligible_actions` is the allowlist of polkit `action_id`s for which +the face authentication agent may offer face auth. Default: +`["org.freedesktop.login1.lock-sessions"]`. Any action not in the list is declined +by the agent so polkit falls through to the password dialog (never denied). An empty +list disables face for all actions. High-risk actions (pkexec, PackageKit, +udisks mount, accounts-service) are excluded by default. ### Camera Auto-Detection @@ -162,6 +170,21 @@ PAM module never blocks indefinitely. All operations have timeouts. pam_facelock(): for user ``` +## Polkit Agent Semantics + +The `facelock-polkit-agent` offers face authentication for polkit actions, but +scoped to an allowlist — face is **not** a universal key for every privileged action. + +| Outcome | Agent behavior | +|---------|----------------| +| `action_id` not in `polkit.face_eligible_actions` | Declines (returns `org.freedesktop.DBus.Error.Failed`) → polkit falls through to the password dialog | +| Allowlisted action, face matches | Responds success to polkit authority | +| Allowlisted action, no match / daemon error | Declines → password fall-through | +| Username cannot be resolved to a uid | Refuses to respond; **never** sends UID 0 for an unresolved name | + +A decline is a **fall-through**, not a denial: another polkit agent (password) still +handles the request. The agent never denies outright and never fails open to root. + ## Anti-Spoofing | Defense | Config | Default | diff --git a/docs/security.md b/docs/security.md index 1fcd844..40ddb9e 100644 --- a/docs/security.md +++ b/docs/security.md @@ -291,6 +291,30 @@ The systemd unit (`systemd/facelock-daemon.service`) includes layered hardening: systemd-analyze security facelock-daemon.service ``` +### 7. Polkit Agent Scoping (Implemented) + +The `facelock-polkit-agent` lets a face match satisfy polkit authorization +requests. Two hardening rules keep this from becoming a universal root key: + +**Action allowlist.** Face auth is offered only for polkit `action_id`s in +`polkit.face_eligible_actions`. The default is a single low-risk action +(`org.freedesktop.login1.lock-sessions`). High-risk actions — pkexec +(`org.freedesktop.policykit.exec`), PackageKit install/remove, udisks mount, and +accounts-service user administration — are **excluded by default**, so a single +face match cannot authorize arbitrary privileged operations. Users may extend the +list deliberately (like widening a fingerprint reader's reach); an empty list +disables face for all actions. + +When an action is not eligible, the agent **declines** (returns a D-Bus +`Failed` error) so polkit falls through to the password dialog handled by another +agent. This is a fall-through, never an outright denial — password auth always +remains available. + +**Fail closed on unresolved user.** When responding to the polkit authority, the +agent resolves the target username to a uid. If the name does not resolve, the +agent refuses to respond. It never substitutes UID 0 — the previous +`unwrap_or(0)` behavior would have authenticated an unresolvable name as root. + ## Security Configuration Reference ```toml @@ -314,6 +338,11 @@ denied_services = ["login", "sshd"] [security.rate_limit] max_attempts = 5 # Max auth attempts per user window_secs = 60 # Rate limit window + +[polkit] +# Polkit actions eligible for face auth. Non-listed actions fall through to +# the password dialog. High-risk actions excluded by default. Empty = face off. +face_eligible_actions = ["org.freedesktop.login1.lock-sessions"] ``` ## Summary: Security Implementation Priority diff --git a/test/Containerfile.deb-e2e b/test/Containerfile.deb-e2e index a9534bf..684585a 100644 --- a/test/Containerfile.deb-e2e +++ b/test/Containerfile.deb-e2e @@ -32,6 +32,7 @@ WORKDIR /build # Copy host-built release binaries COPY target/release/facelock /build/target/release/facelock +COPY target/release/facelock-polkit-agent /build/target/release/facelock-polkit-agent COPY target/release/libpam_facelock.so /build/target/release/libpam_facelock.so # Copy project files needed by build-deb.sh @@ -43,8 +44,9 @@ COPY .github/workflows/scripts/build-deb.sh /build/.github/workflows/scripts/bui # Copy test validation script and PAM test config COPY test/pkg-validate.sh /pkg-validate.sh +COPY test/polkit-agent-validate.sh /polkit-agent-validate.sh COPY test/pam.d/facelock-test /etc/pam.d/facelock-test -RUN chmod +x /pkg-validate.sh +RUN chmod +x /pkg-validate.sh /polkit-agent-validate.sh # Download portable CPU-only ONNX Runtime (same as CI bundles in release packages). # Detect version from host binary's linked ORT, fall back to 1.20.1. diff --git a/test/Containerfile.rpm-e2e b/test/Containerfile.rpm-e2e index 0814b4b..2600464 100644 --- a/test/Containerfile.rpm-e2e +++ b/test/Containerfile.rpm-e2e @@ -2,7 +2,9 @@ # Uses host-built release binaries (from `just build-release`); no Rust compilation in container. FROM fedora:latest -RUN dnf -y install pam dbus rpm-build systemd binutils glibc libxkbcommon tpm2-tss ca-certificates openssl-libs python3 && dnf clean all +# dbus-daemon (Fedora ships dbus-broker by default) is needed for the runtime +# D-Bus tests in pkg-validate.sh, including the polkit agent boundary check. +RUN dnf -y install pam dbus dbus-daemon rpm-build systemd binutils glibc libxkbcommon tpm2-tss ca-certificates openssl-libs python3 && dnf clean all # Build pamtester from source (not in Fedora repos) RUN dnf -y install gcc gcc-c++ pam-devel make curl && \ @@ -18,6 +20,7 @@ WORKDIR /build # Copy host-built release binaries COPY target/release/facelock /build/target/release/facelock +COPY target/release/facelock-polkit-agent /build/target/release/facelock-polkit-agent COPY target/release/libpam_facelock.so /build/target/release/libpam_facelock.so # Copy project files needed for RPM packaging @@ -31,12 +34,9 @@ COPY LICENSE-APACHE /build/LICENSE-APACHE # Copy helper script, test validation, and PAM test config COPY test/build-rpm-prebuilt.sh /build/test/build-rpm-prebuilt.sh COPY test/pkg-validate.sh /pkg-validate.sh +COPY test/polkit-agent-validate.sh /polkit-agent-validate.sh COPY test/pam.d/facelock-test /etc/pam.d/facelock-test -RUN chmod +x /build/test/build-rpm-prebuilt.sh /pkg-validate.sh - -# Patch spec to remove polkit agent from %files (not built from host binaries). -# Only remove the standalone %files entry, not the install command. -RUN sed -i '/^%{_bindir}\/facelock-polkit-agent$/d' /build/dist/facelock.spec +RUN chmod +x /build/test/build-rpm-prebuilt.sh /pkg-validate.sh /polkit-agent-validate.sh # Download portable CPU-only ONNX Runtime (same as CI bundles in release packages) ARG ORT_VERSION=1.20.1 diff --git a/test/pkg-validate.sh b/test/pkg-validate.sh index 78168a2..a071260 100644 --- a/test/pkg-validate.sh +++ b/test/pkg-validate.sh @@ -95,6 +95,12 @@ if command -v dbus-daemon >/dev/null 2>&1; then elif command -v dbus-send >/dev/null 2>&1; then run_test "D-Bus facelock service activatable" "dbus-send --system --dest=org.freedesktop.DBus --print-reply /org/freedesktop/DBus org.freedesktop.DBus.ListActivatableNames 2>/dev/null | grep -q org.facelock.Daemon" fi + + # Polkit agent D-Bus boundary: non-allowlisted actions decline (fall + # through to password), allowlisted actions pass the allowlist gate. + if [ -x /polkit-agent-validate.sh ]; then + run_test "polkit agent allowlist gate (D-Bus boundary)" "/polkit-agent-validate.sh" + fi fi # Package removal test — must come last since it removes the package diff --git a/test/polkit-agent-validate.sh b/test/polkit-agent-validate.sh new file mode 100644 index 0000000..32cee0c --- /dev/null +++ b/test/polkit-agent-validate.sh @@ -0,0 +1,126 @@ +#!/bin/bash +# Validates the facelock polkit agent's D-Bus boundary: +# - a NON-allowlisted polkit action_id is declined ("not eligible"), +# so polkit falls through to the password dialog; +# - an ALLOWLISTED action_id passes the allowlist gate (it is declined +# later only because no daemon/camera exists in-container, never by the +# allowlist). +# +# Full interactive polkit is not feasible in a container, so we assert at the +# agent's own D-Bus interface. The agent is launched with +# FACELOCK_POLKIT_SKIP_REGISTER=1, which skips polkit registration and claims a +# well-known session-bus name so busctl can address it. +# +# Env overrides (for running outside the package container): +# FACELOCK_POLKIT_AGENT_BIN path to the agent binary +set -uo pipefail + +AGENT_BIN="${FACELOCK_POLKIT_AGENT_BIN:-}" +if [ -z "$AGENT_BIN" ]; then + for c in /usr/bin/facelock-polkit-agent /usr/local/bin/facelock-polkit-agent; do + [ -x "$c" ] && AGENT_BIN="$c" && break + done +fi + +if [ -z "$AGENT_BIN" ] || [ ! -x "$AGENT_BIN" ]; then + echo "SKIP: facelock-polkit-agent binary not found" + exit 0 +fi +if ! command -v dbus-daemon >/dev/null 2>&1; then + echo "SKIP: dbus-daemon not available" + exit 0 +fi +if ! command -v busctl >/dev/null 2>&1; then + echo "SKIP: busctl not available" + exit 0 +fi + +echo "=== Polkit Agent D-Bus Boundary Validation ===" + +FAIL=0 +BUS_SOCK="$(mktemp -u /tmp/facelock-polkit-session-bus.XXXXXX)" +SESSION_ADDR="unix:path=${BUS_SOCK}" +AGENT_PID="" +BUS_PID="" + +cleanup() { + [ -n "$AGENT_PID" ] && kill "$AGENT_PID" 2>/dev/null || true + [ -n "$BUS_PID" ] && kill "$BUS_PID" 2>/dev/null || true + rm -f "$BUS_SOCK" +} +trap cleanup EXIT + +# Private session bus for the agent + probe. +dbus-daemon --session --address="$SESSION_ADDR" --nofork --nopidfile & +BUS_PID=$! +export DBUS_SESSION_BUS_ADDRESS="$SESSION_ADDR" + +# Wait for the session bus socket. +for _ in $(seq 1 50); do + [ -S "$BUS_SOCK" ] && break + sleep 0.1 +done + +# Launch the agent in test mode (skip polkit registration, claim well-known name). +FACELOCK_POLKIT_SKIP_REGISTER=1 "$AGENT_BIN" >/tmp/polkit-agent.log 2>&1 & +AGENT_PID=$! + +# Wait for the agent to own its well-known name. +OWNED=0 +for _ in $(seq 1 100); do + if busctl --user --address="$SESSION_ADDR" list 2>/dev/null | grep -q org.facelock.PolkitAgent; then + OWNED=1 + break + fi + sleep 0.1 +done + +if [ "$OWNED" -ne 1 ]; then + echo "SKIP: agent did not claim its session-bus name (no session/system bus?)" + echo "--- agent log ---" + cat /tmp/polkit-agent.log 2>/dev/null || true + exit 0 +fi + +call_begin() { + local action_id="$1" + # Bound the call: an allowlisted action proceeds to daemon activation, + # which has no camera in-container and may block; we only care that the + # allowlist gate let it through, so a timeout is an acceptable outcome. + timeout 10 busctl --user --address="$SESSION_ADDR" call \ + org.facelock.PolkitAgent /org/facelock/PolkitAgent \ + org.freedesktop.PolicyKit1.AuthenticationAgent \ + BeginAuthentication "sssa{ss}sa(sa{sv})" \ + "$action_id" "pkg-validate probe" "" 0 "cookie-${action_id}" 0 2>&1 +} + +# 1. Non-allowlisted action MUST be declined with "not eligible". +OUT_DENY="$(call_begin "org.freedesktop.policykit.exec")" +echo -n "TEST: non-allowlisted action_id declined (falls through to password) ... " +if echo "$OUT_DENY" | grep -qi "not eligible"; then + echo "PASS" +else + echo "FAIL" + echo " expected an error containing 'not eligible', got:" + echo " $OUT_DENY" + FAIL=$((FAIL + 1)) +fi + +# 2. Allowlisted action MUST pass the allowlist gate (never "not eligible"). +OUT_ALLOW="$(call_begin "org.freedesktop.login1.lock-sessions")" +echo -n "TEST: allowlisted action_id accepted by allowlist gate ... " +if echo "$OUT_ALLOW" | grep -qi "not eligible"; then + echo "FAIL" + echo " allowlisted action was wrongly rejected by the allowlist:" + echo " $OUT_ALLOW" + FAIL=$((FAIL + 1)) +else + echo "PASS" +fi + +echo "" +if [ "$FAIL" -gt 0 ]; then + echo "=== Polkit Agent Validation: $FAIL failed ===" + exit 1 +fi +echo "=== Polkit Agent Validation: OK ===" From 34c1cbb532404212b04ef850bda86b1df6c670f9 Mon Sep 17 00:00:00 2001 From: Ty Smith Date: Sat, 4 Jul 2026 14:04:45 -0700 Subject: [PATCH 2/3] style: fix cargo fmt violations in polkit config tests cargo fmt --all -- --check was failing CI on unwrapped long assert! lines added by the polkit allowlist tests. Run cargo fmt --all to wrap them per rustfmt's max-width rules. Co-Authored-By: Claude Fable 5 --- crates/facelock-core/src/config.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/facelock-core/src/config.rs b/crates/facelock-core/src/config.rs index 1af5b76..cfdf1ac 100644 --- a/crates/facelock-core/src/config.rs +++ b/crates/facelock-core/src/config.rs @@ -1091,10 +1091,22 @@ face_eligible_actions = [ ] "#; let config = Config::parse(toml).unwrap(); - assert!(config.polkit.is_face_eligible("org.freedesktop.udisks2.filesystem-mount")); - assert!(config.polkit.is_face_eligible("org.freedesktop.login1.lock-sessions")); + assert!( + config + .polkit + .is_face_eligible("org.freedesktop.udisks2.filesystem-mount") + ); + assert!( + config + .polkit + .is_face_eligible("org.freedesktop.login1.lock-sessions") + ); // Still excludes anything the user did not add. - assert!(!config.polkit.is_face_eligible("org.freedesktop.policykit.exec")); + assert!( + !config + .polkit + .is_face_eligible("org.freedesktop.policykit.exec") + ); } #[test] @@ -1107,7 +1119,11 @@ face_eligible_actions = [] "#; let config = Config::parse(toml).unwrap(); // An explicitly empty list means "never offer face" — no fail-open. - assert!(!config.polkit.is_face_eligible("org.freedesktop.login1.lock-sessions")); + assert!( + !config + .polkit + .is_face_eligible("org.freedesktop.login1.lock-sessions") + ); } #[test] From e5aee458524d7c20898c6451c2e19ccd8088c1e4 Mon Sep 17 00:00:00 2001 From: Ty Smith Date: Sat, 4 Jul 2026 14:04:50 -0700 Subject: [PATCH 3/3] docs(polkit): stop asserting unverified password-fallthrough behavior Two independent reviewers flagged that docs/security.md and docs/contracts.md overstated polkit agent behavior: they claimed a non-allowlisted action "falls through to the password dialog handled by another agent." polkit registers a single authentication agent per session and does not chain agents, so a decline from this agent likely produces an authorization denial rather than a fallthrough, depending on the desktop's agent registration. This is fail-closed (safe) but changes the UX story and has not been verified on a live desktop. Replace the fallthrough claims with an explicit "under review" caveat in both docs (including the contracts.md outcome table and the security.md polkit config example) instead of asserting either behavior as final. Surrounding allowlist documentation is unchanged. Co-Authored-By: Claude Fable 5 --- docs/contracts.md | 32 +++++++++++++++++++++++++------- docs/security.md | 19 ++++++++++++++----- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/docs/contracts.md b/docs/contracts.md index d0ce1e0..9b6eaf3 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -91,9 +91,17 @@ TOML format. All keys optional — camera auto-detected, sensible defaults for e `[polkit].face_eligible_actions` is the allowlist of polkit `action_id`s for which the face authentication agent may offer face auth. Default: `["org.freedesktop.login1.lock-sessions"]`. Any action not in the list is declined -by the agent so polkit falls through to the password dialog (never denied). An empty -list disables face for all actions. High-risk actions (pkexec, PackageKit, -udisks mount, accounts-service) are excluded by default. +by the agent. An empty list disables face for all actions. High-risk actions +(pkexec, PackageKit, udisks mount, accounts-service) are excluded by default. + +**NOTE (under review):** polkit registers a single authentication agent per +session and does not chain agents. When this agent declines a non-allowlisted +action it returns an error, which — depending on the desktop's agent +registration — may present as an authorization denial rather than a +fallthrough to a password dialog. The intended UX (non-eligible actions +handled by the desktop's normal password agent) is unverified pending +live-desktop testing and may require a design change. Behavior here is +fail-closed: a non-eligible action is never face-authorized. ### Camera Auto-Detection @@ -177,13 +185,23 @@ scoped to an allowlist — face is **not** a universal key for every privileged | Outcome | Agent behavior | |---------|----------------| -| `action_id` not in `polkit.face_eligible_actions` | Declines (returns `org.freedesktop.DBus.Error.Failed`) → polkit falls through to the password dialog | +| `action_id` not in `polkit.face_eligible_actions` | Declines (returns `org.freedesktop.DBus.Error.Failed`) — see fallthrough-vs-denial caveat below | | Allowlisted action, face matches | Responds success to polkit authority | -| Allowlisted action, no match / daemon error | Declines → password fall-through | +| Allowlisted action, no match / daemon error | Declines (same caveat) | | Username cannot be resolved to a uid | Refuses to respond; **never** sends UID 0 for an unresolved name | -A decline is a **fall-through**, not a denial: another polkit agent (password) still -handles the request. The agent never denies outright and never fails open to root. +**NOTE (under review):** polkit registers a single authentication agent per +session and does not chain agents. When this agent declines, the decline +returns an error, which — depending on the desktop's agent registration — may +present as an authorization denial rather than a fallthrough to a password +dialog. The intended UX (non-eligible actions handled by the desktop's normal +password agent) is unverified pending live-desktop testing and may require a +design change. Behavior here is fail-closed: a non-eligible action is never +face-authorized. + +A decline never fails open to root, and never causes this agent itself to grant +authorization it should not — but see the caveat above on whether polkit +treats a decline as a fall-through to another agent or as an outright denial. ## Anti-Spoofing diff --git a/docs/security.md b/docs/security.md index 40ddb9e..4e33162 100644 --- a/docs/security.md +++ b/docs/security.md @@ -306,9 +306,16 @@ list deliberately (like widening a fingerprint reader's reach); an empty list disables face for all actions. When an action is not eligible, the agent **declines** (returns a D-Bus -`Failed` error) so polkit falls through to the password dialog handled by another -agent. This is a fall-through, never an outright denial — password auth always -remains available. +`Failed` error). + +> **NOTE (under review):** polkit registers a single authentication agent per +> session and does not chain agents. When this agent declines a non-allowlisted +> action it returns an error, which — depending on the desktop's agent +> registration — may present as an authorization denial rather than a +> fallthrough to a password dialog. The intended UX (non-eligible actions +> handled by the desktop's normal password agent) is unverified pending +> live-desktop testing and may require a design change. Behavior here is +> fail-closed: a non-eligible action is never face-authorized. **Fail closed on unresolved user.** When responding to the polkit authority, the agent resolves the target username to a uid. If the name does not resolve, the @@ -340,8 +347,10 @@ max_attempts = 5 # Max auth attempts per user window_secs = 60 # Rate limit window [polkit] -# Polkit actions eligible for face auth. Non-listed actions fall through to -# the password dialog. High-risk actions excluded by default. Empty = face off. +# Polkit actions eligible for face auth. Non-listed actions are declined by +# the agent (see "Polkit Agent Scoping" above — whether this presents as a +# password fallthrough or an authorization denial is unverified pending +# live-desktop testing). High-risk actions excluded by default. Empty = face off. face_eligible_actions = ["org.freedesktop.login1.lock-sessions"] ```