sessiond: take lock truth from logind, not from our own history
Measured on device 40min after boot: device_state=locked, panel_on=false, while LockedHint=no. Screen dark, session unlocked. locked_ack was the only lock ingress and there was no unlock one outside the fallback PIN surface, so after the first unlock the machine believed it was locked forever. Every is_locked() rule then fired against an in-use phone — proximity blanked it, and blanks never asked for a lock because one was "already held". Doctrine §4: never hold state the protocol owns. New lockhint.rs watches LockedHint via gdbus monitor (§8's pattern, not polling). Resolve via User.Display, NOT GetSession(auto) — auto returns the CALLER's session and sessiond is a session-less user unit; over ssh it answered _310 while the graphical session was _31. set_session_locked() moves the base tier and voids lock-screen intent on unlock: pending blank cleared, dim restored. 46 tests, 8 new.
This commit is contained in:
Fimeg
parent
9ab1cfbccc
commit
df654e6bee
4 changed files with 389 additions and 0 deletions
|
|
@ -916,6 +916,46 @@ impl DeviceStateMachine {
|
|||
true
|
||||
}
|
||||
|
||||
/// Adopt logind's `LockedHint` as the lock truth.
|
||||
///
|
||||
/// This replaces the machine deciding for itself. It had no unlock ingress
|
||||
/// outside its own fallback surface, so after the first unlock it believed
|
||||
/// the session was locked forever, and every rule gated on
|
||||
/// `is_locked(state)` — the blank budget AND the proximity blank — fired
|
||||
/// against an unlocked, in-use phone. Doctrine §4: never hold state the
|
||||
/// protocol owns.
|
||||
///
|
||||
/// Locking moves the base tier to `Locked`; unlocking returns to `Active`
|
||||
/// from whichever locked sub-state we were in. Sub-state detail
|
||||
/// (`Observed`, doze tiers) is ours to track *within* locked, but whether
|
||||
/// we are locked at all is not.
|
||||
pub fn set_session_locked(&mut self, locked: bool) -> Vec<Action> {
|
||||
if locked == is_locked(self.state) {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
if locked {
|
||||
self.transition(DeviceState::Locked);
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
// Unlocked. Anything the machine decided on the strength of believing
|
||||
// it was locked is void: a pending blank was only legitimate because
|
||||
// the screen was a lock screen, and the dim warning belongs to that
|
||||
// same rule.
|
||||
info!("[device-state] session unlocked (logind) — clearing lock-screen intent");
|
||||
self.transition(DeviceState::Active);
|
||||
self.blank_requested = false;
|
||||
self.pending_blank = None;
|
||||
|
||||
let mut actions = Vec::new();
|
||||
if self.dimmed {
|
||||
self.dimmed = false;
|
||||
actions.push(Action::Restore);
|
||||
}
|
||||
actions
|
||||
}
|
||||
|
||||
/// Update sensor evidence and, if the current state is Locked,
|
||||
/// potentially transition to/from Observed.
|
||||
/// Emits forensic entries for every sensor input.
|
||||
|
|
@ -1115,6 +1155,62 @@ mod tests {
|
|||
(sm, t0)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn logind_unlock_releases_the_machine_from_locked() {
|
||||
// The bug this closes, measured on hardware 2026-07-26: the machine
|
||||
// sat in Locked for 40 minutes while LockedHint said no, because
|
||||
// locked_ack was the only lock ingress and there was no unlock one.
|
||||
let (mut sm, _t0) = locked_and_lit();
|
||||
assert!(is_locked(sm.state));
|
||||
|
||||
sm.set_session_locked(false);
|
||||
assert_eq!(sm.state, DeviceState::Active);
|
||||
assert!(!is_locked(sm.state));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn proximity_cannot_blank_after_a_logind_unlock() {
|
||||
// The symptom Casey reported: proximity blanking an unlocked phone.
|
||||
// The proximity rule was always correctly gated on is_locked — the
|
||||
// state was the thing lying.
|
||||
let (mut sm, t0) = locked_and_lit();
|
||||
sm.set_session_locked(false);
|
||||
|
||||
sm.sensor_evidence.proximity_near = true;
|
||||
sm.mark_evidence_seen(SensorSource::Proximity);
|
||||
assert!(
|
||||
sm.tick_at(t0 + Duration::from_secs(1)).is_empty(),
|
||||
"a hand over the sensor must not blank an unlocked phone"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unlocking_cancels_a_pending_blank_and_restores_the_dim() {
|
||||
let (mut sm, t0) = locked_and_lit();
|
||||
// Get into the dim pre-warning.
|
||||
let dim_at = LOCK_BLANK_AFTER - LOCK_DIM_GRACE;
|
||||
assert_eq!(
|
||||
sm.tick_at(t0 + dim_at + Duration::from_millis(10)),
|
||||
vec![Action::Dim]
|
||||
);
|
||||
assert!(sm.dimmed);
|
||||
|
||||
// Unlocking voids lock-screen intent and undims.
|
||||
assert_eq!(sm.set_session_locked(false), vec![Action::Restore]);
|
||||
assert!(!sm.dimmed);
|
||||
assert!(sm.pending_blank.is_none());
|
||||
assert!(!sm.blank_requested);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn relocking_puts_the_machine_back() {
|
||||
let (mut sm, _t0) = locked_and_lit();
|
||||
sm.set_session_locked(false);
|
||||
assert_eq!(sm.state, DeviceState::Active);
|
||||
sm.set_session_locked(true);
|
||||
assert!(is_locked(sm.state));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unlocked_blank_asks_for_the_lock_first() {
|
||||
let (mut sm, t0) = unlocked_and_lit();
|
||||
|
|
|
|||
271
src/sessiond/lockhint.rs
Normal file
271
src/sessiond/lockhint.rs
Normal file
|
|
@ -0,0 +1,271 @@
|
|||
//! logind's `LockedHint` — the machine's lock truth.
|
||||
//!
|
||||
//! The state machine used to decide for itself whether the session was locked,
|
||||
//! from its own transition history: `locked_ack` from the shell put it in
|
||||
//! `Locked` and nothing ever took it out. There was no unlock ingress at all
|
||||
//! except sessiond's own fallback PIN surface, which is the crash path, not
|
||||
//! the one anybody uses daily. So after the first unlock the machine believed
|
||||
//! the session was locked forever.
|
||||
//!
|
||||
//! Measured on hardware 2026-07-26, 40 minutes after a cold boot:
|
||||
//! `device_state: locked, locked: true, panel_on: false` while
|
||||
//! `loginctl show-session 1 -p LockedHint` said `LockedHint=no`. The screen was
|
||||
//! dark on an unlocked session, and both the blank-budget rule and the
|
||||
//! proximity rule had been firing against `is_locked(state) == true` the whole
|
||||
//! time — which is why proximity was blanking an unlocked phone and why a
|
||||
//! blank never asked for a lock first (it thought one was already held).
|
||||
//!
|
||||
//! Doctrine §4 named this exactly: "`locked` comes from `LockedHint` /
|
||||
//! `Lock()` / `Unlock()` — never a hand-tracked bool. A shadow copy can
|
||||
//! disagree with logind, and anything asking logind directly sees something
|
||||
//! different than the shell shows." The machine was keeping the shadow copy the
|
||||
//! doctrine forbids, and it disagreed precisely as predicted.
|
||||
//!
|
||||
//! So we read the authority instead. Whoever locks or unlocks — the shell's
|
||||
//! rich surface, sessiond's fallback surface, `loginctl lock-session`, a future
|
||||
//! greeter — logind is told, so we are told. The machine stops guessing.
|
||||
//!
|
||||
//! Transport is `gdbus monitor`, the pattern doctrine §8 names for the sibling
|
||||
//! problem ("a persistent `gdbus monitor --system` process, parsed
|
||||
//! line-by-line — **not** polling. Polling latency eats the delay budget
|
||||
//! directly"). sessiond has no D-Bus crate: `zbus` is behind the `secrets`
|
||||
//! feature and drags in a tokio runtime this thread-based daemon does not want.
|
||||
|
||||
use std::io::{BufRead, BufReader};
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
use std::process::{Command, Stdio};
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::{bail, Context, Result};
|
||||
use tracing::{info, warn};
|
||||
|
||||
/// Resolve the graphical session's REAL object path.
|
||||
///
|
||||
/// Two traps here, both measured.
|
||||
///
|
||||
/// **Not `auto`.** `GetSession("auto")` resolves to *the caller's* session, and
|
||||
/// sessiond is a systemd user unit — it runs under `user@1000.service`, outside
|
||||
/// any session scope. Asked over ssh it answers the ssh session
|
||||
/// (`session/_310` on 2026-07-26 while the phone's graphical session was `_31`);
|
||||
/// asked from a session-less unit it is at best a fallback. The user's
|
||||
/// `Display` session is the graphical one by definition, whoever asks.
|
||||
///
|
||||
/// **Not the `auto` path either.** `/org/freedesktop/login1/session/auto` is an
|
||||
/// alias, not an object: `PropertiesChanged` only ever fires on the concrete
|
||||
/// path, so monitoring the alias yields a subscription that is silently never
|
||||
/// delivered. That is the failure the shell already hit
|
||||
/// (`[session-events] no session path returned`).
|
||||
fn resolve_session_path() -> Result<String> {
|
||||
let uid = std::fs::metadata("/proc/self")
|
||||
.map(|m| m.uid())
|
||||
.context("reading our own uid")?;
|
||||
|
||||
// `(so) "1" "/org/freedesktop/login1/session/_31"` — the object path is the
|
||||
// second quoted field.
|
||||
let user_obj = format!("/org/freedesktop/login1/user/_{uid}");
|
||||
if let Ok(out) = Command::new("busctl")
|
||||
.args([
|
||||
"--system",
|
||||
"get-property",
|
||||
"org.freedesktop.login1",
|
||||
&user_obj,
|
||||
"org.freedesktop.login1.User",
|
||||
"Display",
|
||||
])
|
||||
.output()
|
||||
{
|
||||
if out.status.success() {
|
||||
let text = String::from_utf8_lossy(&out.stdout);
|
||||
if let Some(p) = text
|
||||
.split('"')
|
||||
.nth(3)
|
||||
.filter(|p| p.starts_with("/org/freedesktop/login1/session/"))
|
||||
{
|
||||
return Ok(p.to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
warn!("[lock-hint] no Display session for uid {uid}; falling back to GetSession(auto), which may resolve to the wrong session");
|
||||
let out = Command::new("busctl")
|
||||
.args([
|
||||
"--system",
|
||||
"call",
|
||||
"org.freedesktop.login1",
|
||||
"/org/freedesktop/login1",
|
||||
"org.freedesktop.login1.Manager",
|
||||
"GetSession",
|
||||
"s",
|
||||
"auto",
|
||||
])
|
||||
.output()
|
||||
.context("busctl GetSession")?;
|
||||
|
||||
if !out.status.success() {
|
||||
bail!(
|
||||
"busctl GetSession failed: {}",
|
||||
String::from_utf8_lossy(&out.stderr).trim()
|
||||
);
|
||||
}
|
||||
|
||||
// `o "/org/freedesktop/login1/session/_31"`
|
||||
let text = String::from_utf8_lossy(&out.stdout);
|
||||
let path = text
|
||||
.split('"')
|
||||
.nth(1)
|
||||
.map(str::to_string)
|
||||
.filter(|p| p.starts_with("/org/freedesktop/login1/session/"))
|
||||
.context("could not parse a session path out of busctl output")?;
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
/// One-shot read, so the machine starts from the truth rather than from a
|
||||
/// default that happens to be wrong until the first signal arrives.
|
||||
fn read_locked_hint(path: &str) -> Result<bool> {
|
||||
let out = Command::new("busctl")
|
||||
.args([
|
||||
"--system",
|
||||
"get-property",
|
||||
"org.freedesktop.login1",
|
||||
path,
|
||||
"org.freedesktop.login1.Session",
|
||||
"LockedHint",
|
||||
])
|
||||
.output()
|
||||
.context("busctl get-property LockedHint")?;
|
||||
|
||||
if !out.status.success() {
|
||||
bail!(
|
||||
"busctl get-property failed: {}",
|
||||
String::from_utf8_lossy(&out.stderr).trim()
|
||||
);
|
||||
}
|
||||
|
||||
// `b true`
|
||||
let text = String::from_utf8_lossy(&out.stdout);
|
||||
Ok(text.split_whitespace().nth(1) == Some("true"))
|
||||
}
|
||||
|
||||
/// Pull the LockedHint value out of a `gdbus monitor` PropertiesChanged line.
|
||||
///
|
||||
/// The line looks like:
|
||||
/// ```text
|
||||
/// /org/freedesktop/login1/session/_31: org.freedesktop.DBus.Properties.PropertiesChanged
|
||||
/// ('org.freedesktop.login1.Session', {'LockedHint': <true>}, @as [])
|
||||
/// ```
|
||||
/// Returns `None` for every other property change on the session, of which
|
||||
/// there are many (`IdleHint`, `Active`, `IdleSinceHint`…).
|
||||
pub(crate) fn parse_locked_hint(line: &str) -> Option<bool> {
|
||||
let idx = line.find("'LockedHint':")?;
|
||||
let rest = &line[idx..];
|
||||
let open = rest.find('<')?;
|
||||
let close = rest[open..].find('>')? + open;
|
||||
match rest[open + 1..close].trim() {
|
||||
"true" => Some(true),
|
||||
"false" => Some(false),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Watch `LockedHint` forever, reporting every change.
|
||||
///
|
||||
/// The callback fires once at startup with the current value, then on each
|
||||
/// transition. It is NOT called for repeats — the machine only needs edges.
|
||||
///
|
||||
/// A watcher that dies is loud and fatal to the guarantee, not silently
|
||||
/// retried into a state where the machine is guessing again: if the monitor
|
||||
/// exits we say so at `warn` and stop, leaving `is_locked` pinned at the last
|
||||
/// known truth rather than drifting.
|
||||
pub fn spawn(on_change: Arc<dyn Fn(bool) + Send + Sync>) {
|
||||
std::thread::spawn(move || {
|
||||
let path = match resolve_session_path() {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
warn!("[lock-hint] could not resolve the session path: {e:#} — the device state machine will fall back to its own lock tracking, which drifts");
|
||||
return;
|
||||
}
|
||||
};
|
||||
info!("[lock-hint] watching {path}");
|
||||
|
||||
let mut last = match read_locked_hint(&path) {
|
||||
Ok(v) => {
|
||||
info!("[lock-hint] initial LockedHint={v}");
|
||||
on_change(v);
|
||||
v
|
||||
}
|
||||
Err(e) => {
|
||||
warn!("[lock-hint] initial read failed: {e:#}");
|
||||
false
|
||||
}
|
||||
};
|
||||
|
||||
let mut child = match Command::new("gdbus")
|
||||
.args([
|
||||
"monitor",
|
||||
"--system",
|
||||
"--dest",
|
||||
"org.freedesktop.login1",
|
||||
"--object-path",
|
||||
&path,
|
||||
])
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
{
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
warn!("[lock-hint] could not start gdbus monitor: {e} — lock truth will not update");
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let Some(stdout) = child.stdout.take() else {
|
||||
warn!("[lock-hint] gdbus monitor produced no stdout");
|
||||
return;
|
||||
};
|
||||
|
||||
for line in BufReader::new(stdout).lines() {
|
||||
let Ok(line) = line else { break };
|
||||
if let Some(v) = parse_locked_hint(&line) {
|
||||
if v != last {
|
||||
last = v;
|
||||
info!("[lock-hint] LockedHint={v}");
|
||||
on_change(v);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
warn!("[lock-hint] gdbus monitor exited — lock truth is now STALE; the machine will keep its last known value");
|
||||
let _ = child.wait();
|
||||
});
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn parses_a_locked_hint_change() {
|
||||
let line = "/org/freedesktop/login1/session/_31: org.freedesktop.DBus.Properties.PropertiesChanged ('org.freedesktop.login1.Session', {'LockedHint': <true>}, @as [])";
|
||||
assert_eq!(parse_locked_hint(line), Some(true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_an_unlock() {
|
||||
let line = "/org/freedesktop/login1/session/_31: org.freedesktop.DBus.Properties.PropertiesChanged ('org.freedesktop.login1.Session', {'LockedHint': <false>}, @as [])";
|
||||
assert_eq!(parse_locked_hint(line), Some(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_other_properties() {
|
||||
// IdleHint churns constantly; treating it as a lock change would make
|
||||
// the machine flap between Active and Locked on every idle edge.
|
||||
let line = "/org/freedesktop/login1/session/_31: org.freedesktop.DBus.Properties.PropertiesChanged ('org.freedesktop.login1.Session', {'IdleHint': <true>}, @as [])";
|
||||
assert_eq!(parse_locked_hint(line), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_unrelated_lines() {
|
||||
assert_eq!(parse_locked_hint("something else entirely"), None);
|
||||
}
|
||||
}
|
||||
|
|
@ -15,5 +15,6 @@ pub mod device_state;
|
|||
pub mod draw;
|
||||
pub mod idle;
|
||||
pub mod lock;
|
||||
pub mod lockhint;
|
||||
pub mod protocol;
|
||||
pub mod server;
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ use tracing::{info, warn};
|
|||
use crate::sessiond::device_state::{Action, DeviceState, DeviceStateMachine, SensorEvidence};
|
||||
use crate::sessiond::idle;
|
||||
use crate::sessiond::lock::{self, LockController, Msg, SessionOutcome};
|
||||
use crate::sessiond::lockhint;
|
||||
use crate::sessiond::protocol::{
|
||||
InputTrigger, Phase, Request, SensorSource, SensorValue, LOCKED_ACK_TIMEOUT_SECS,
|
||||
MAX_REQUEST_BYTES,
|
||||
|
|
@ -114,6 +115,7 @@ pub fn run(pam_service: String, initial_lock: bool, socket_path: &Path) -> Resul
|
|||
}
|
||||
spawn_clock(&shared);
|
||||
spawn_idle_watcher(&shared);
|
||||
spawn_lock_hint_watcher(&shared);
|
||||
info!("sessiond serving on {}", socket_path.display());
|
||||
|
||||
for stream in listener.incoming() {
|
||||
|
|
@ -175,6 +177,25 @@ fn spawn_idle_watcher(shared: &Arc<Shared>) {
|
|||
}));
|
||||
}
|
||||
|
||||
/// Feed logind's lock truth into the machine.
|
||||
///
|
||||
/// Without this the machine tracked `locked` itself and had no unlock ingress,
|
||||
/// so after the first unlock it believed the session was locked forever — and
|
||||
/// then blanked an in-use phone on proximity, because the proximity rule is
|
||||
/// gated on `is_locked`. Doctrine §4: never hold state the protocol owns.
|
||||
fn spawn_lock_hint_watcher(shared: &Arc<Shared>) {
|
||||
let shared = Arc::clone(shared);
|
||||
lockhint::spawn(Arc::new(move |locked| {
|
||||
let actions = {
|
||||
let mut d = shared.lock();
|
||||
d.device_state.set_session_locked(locked)
|
||||
};
|
||||
for action in actions {
|
||||
execute(&shared, action);
|
||||
}
|
||||
}));
|
||||
}
|
||||
|
||||
/// Carry out one decision. Failures are recorded in the forensic trail — a
|
||||
/// blank that did not happen is a state divergence, not a log line to lose.
|
||||
fn execute(shared: &Arc<Shared>, action: Action) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue