From c6c40dbced4b49c5c8b6ee58a2435ae03c36add4 Mon Sep 17 00:00:00 2001 From: Vladimir Kostelei Date: Fri, 25 Sep 2026 17:02:51 +0200 Subject: [PATCH] fix: fingerprint lid open (#3448) * lock: keep the fingerprint reader usable across suspend The reader stopped responding after the lid was closed and opened, after a long idle period, and after suspend. The shell held a PAM session for as long as the screen was locked, so a suspend always landed on a running verify. fprintd then suspends the device under the verify, pam_fprintd's ReleaseDevice fails with "device is still busy", and the claim is never released, so every later Claim fails until fprintd restarts. The reader is now released when the monitors blank and armed again when they come back, so a suspend has nothing left to wedge. The PAM profile drops to max-tries=1 and sets an explicit timeout. pam_fprintd only counts a wrong finger, so one finished session is now one wrong scan and maxFprintTries means the number of scans it names rather than five times that. A timeout expiry arrives as PamResult.Error, exactly like a device fault, so a session that ran at least as long as the configured timeout is treated as a planned renewal instead of spending the error budget and painting an error. A suspend that still lands while the screen is on wedges as before. Recovery now waits out fprintd's idle timeout instead of climbing to it, which brings the reader back about 37 seconds after resume instead of about 98. SessionService gains lidOpened from UPower, because logind can ignore the lid on AC and resume alone is not enough. The lock PAM state machine is fixed alongside: PamResult.Failed was dropped, a pending retry outlived the contexts that owned it, and an availability check reset the wrong-scan counter. * lock: time the fingerprint session from the verify, not the claim pam_fprintd's timeout runs from the verify, but the session age was measured from start(), which folds the claim into it. A slow claim plus a verify that failed before its own timeout could add up past the timeout and be taken for a planned renewal, skipping the backoff. A session that never reached a verify had the same problem. The age now starts at the message pam_fprintd sends when the verify begins, so a session that never verified can never qualify as an expiry, and a slow claim no longer ages a fault. The message is translated and quickshell clears it before completed, so only its timing is used, never its text. That message arrives after pam_fprintd has already armed its deadline, because VerifyStart is asynchronous, so the age read here runs short of the real timeout and a slack makes it up. The bias is deliberate. A fault misread as a timeout costs one extra retry, while a timeout misread as a fault parks the reader behind a backoff that grows to a minute, which on an idle lock screen would leave it unusable most of the time. Also corrects the comment that claimed a timeout is reported without a message. --------- (cherry picked from commit d7074a53ebc7ba4c16ff66f2174a4407b2dd98b7) --- .../qmlchecks/lockscreen_input_test.go | 49 +++++ quickshell/Modules/Lock/LockScreenContent.qml | 13 +- quickshell/Modules/Lock/LockSurface.qml | 13 ++ quickshell/Modules/Lock/Pam.qml | 206 +++++++++++++----- quickshell/Services/SessionService.qml | 43 +++- quickshell/assets/pam/fprint | 5 +- 6 files changed, 268 insertions(+), 61 deletions(-) diff --git a/core/internal/qmlchecks/lockscreen_input_test.go b/core/internal/qmlchecks/lockscreen_input_test.go index c7e2f168d..460613284 100644 --- a/core/internal/qmlchecks/lockscreen_input_test.go +++ b/core/internal/qmlchecks/lockscreen_input_test.go @@ -3,6 +3,7 @@ package qmlchecks import ( "os" "regexp" + "strconv" "strings" "testing" ) @@ -55,3 +56,51 @@ func TestLockScreenPamSupportsManagedAndSystemPolicies(t *testing.T) { t.Fatalf("DMS-managed policy must generate the sanitized user PAM stack on Nix-store installs") } } + +func TestLockScreenFprintProfileMatchesPamQml(t *testing.T) { + profile, err := os.ReadFile("../../../quickshell/assets/pam/fprint") + if err != nil { + t.Fatalf("read fprint PAM profile: %v", err) + } + + qml, err := os.ReadFile("../../../quickshell/Modules/Lock/Pam.qml") + if err != nil { + t.Fatalf("read lock screen PAM QML: %v", err) + } + + // Pam.qml tells a pam_fprintd timeout expiry apart from a device fault by + // how long the session ran, so a profile change that is not mirrored in + // sessionTimeoutMs makes real faults look like planned renewals. + profileTimeout := regexp.MustCompile(`pam_fprintd\.so[^\n]*\btimeout=(\d+)`).FindSubmatch(profile) + if profileTimeout == nil { + t.Fatalf("fprint profile must set an explicit pam_fprintd timeout") + } + qmlTimeout := regexp.MustCompile(`sessionTimeoutMs:\s*(\d+)`).FindSubmatch(qml) + if qmlTimeout == nil { + t.Fatalf("Pam.qml must declare sessionTimeoutMs") + } + + seconds, err := strconv.Atoi(string(profileTimeout[1])) + if err != nil { + t.Fatalf("parse profile timeout: %v", err) + } + millis, err := strconv.Atoi(string(qmlTimeout[1])) + if err != nil { + t.Fatalf("parse sessionTimeoutMs: %v", err) + } + if seconds*1000 != millis { + t.Fatalf("sessionTimeoutMs is %dms but the fprint profile sets timeout=%d", millis, seconds) + } + + // fprintd 1.94.0 and older ignore this option when it is longer than two + // digits and silently fall back to 30 seconds. + if len(profileTimeout[1]) > 2 { + t.Fatalf("pam_fprintd timeout=%s is too long for fprintd <= 1.94.0, which would ignore it", profileTimeout[1]) + } + + // One finished session must equal one wrong finger, otherwise + // SettingsData.maxFprintTries silently means a multiple of the scans it names. + if !regexp.MustCompile(`pam_fprintd\.so[^\n]*\bmax-tries=1\b`).Match(profile) { + t.Fatalf("fprint profile must use max-tries=1 so the shell counts wrong scans one for one") + } +} diff --git a/quickshell/Modules/Lock/LockScreenContent.qml b/quickshell/Modules/Lock/LockScreenContent.qml index 0ed1a9b1a..86e2747b0 100644 --- a/quickshell/Modules/Lock/LockScreenContent.qml +++ b/quickshell/Modules/Lock/LockScreenContent.qml @@ -82,6 +82,8 @@ Item { return I18n.tr("Too many attempts - locked out"); if (root.pamState === "fail") return I18n.tr("Incorrect password - try again"); + if (pam.fprint.status === "disabled") + return ""; if (pam.fprintState === "error") return I18n.tr("Fingerprint error"); if (pam.fprintState === "max") @@ -861,10 +863,15 @@ Item { name: { if (pam.u2fPending) return "passkey"; - if (pam.fprint.tries >= SettingsData.maxFprintTries) + switch (pam.fprint.status) { + case "max": + case "stopped": return "fingerprint_off"; - if (pam.fprint.active || pam.fprint.retrying) + case "active": return "fingerprint"; + case "retrying": + return "hourglass_empty"; + } if (pam.u2f.active) return "passkey"; return "lock"; @@ -1005,6 +1012,8 @@ Item { return; } + root.pam.retryFprintOnActivity(); + if (root.unlocking) { event.accepted = true; return; diff --git a/quickshell/Modules/Lock/LockSurface.qml b/quickshell/Modules/Lock/LockSurface.qml index f41a82666..4704683c3 100644 --- a/quickshell/Modules/Lock/LockSurface.qml +++ b/quickshell/Modules/Lock/LockSurface.qml @@ -16,7 +16,20 @@ FocusScope { signal passwordChanged(string newPassword) signal unlockRequested + HoverHandler { + enabled: root.pam.awaitingActivityRetry + onPointChanged: root.pam.retryFprintOnActivity() + } + + PointHandler { + onActiveChanged: { + if (active) + root.pam.retryFprintOnActivity(); + } + } + Keys.onPressed: event => { + root.pam.retryFprintOnActivity(); if (videoScreensaver.active && videoScreensaver.inputEnabled) { videoScreensaver.dismiss(); event.accepted = true; diff --git a/quickshell/Modules/Lock/Pam.qml b/quickshell/Modules/Lock/Pam.qml index 404c3d579..22b598d30 100644 --- a/quickshell/Modules/Lock/Pam.qml +++ b/quickshell/Modules/Lock/Pam.qml @@ -32,9 +32,8 @@ Scope { function resetAuthFlows(): void { passwd.abort(); - fprint.abort(); + fprint.stop(); u2f.abort(); - errorRetry.running = false; u2fErrorRetry.running = false; u2fPendingTimeout.running = false; passwdActiveTimeout.running = false; @@ -58,9 +57,8 @@ Scope { if (!root.unlockInProgress) { root.unlockInProgress = true; passwd.abort(); - fprint.abort(); + fprint.stop(); u2f.abort(); - errorRetry.running = false; u2fErrorRetry.running = false; u2fPendingTimeout.running = false; root.u2fPending = false; @@ -217,7 +215,7 @@ Scope { if (res === PamResult.Success) { if (!root.unlockInProgress) { - fprint.abort(); + fprint.stop(); root.proceedAfterPrimaryAuth(); } return; @@ -260,69 +258,147 @@ Scope { PamContext { id: fprint - property bool available: SettingsData.lockFingerprintReady - property int tries - property int errorTries - property bool retrying: false + readonly property bool available: SettingsData.lockFingerprintReady + property int tries: 0 + property int errorTries: 0 + property double attemptStartedAt: 0 + property bool completedDuringStart: false + readonly property int maxErrorTries: 200 + readonly property int daemonIdleExitMs: 30000 + // sessionTimeoutMs mirrors `timeout=` in assets/pam/fprint + readonly property int sessionTimeoutMs: 90000 + readonly property int verifyStartSlackMs: 10000 + readonly property bool retrying: errorRetry.running + readonly property int retryInterval: errorRetry.interval + readonly property bool allowed: available && SettingsData.enableFprint && root.lockSecured && !root.fprintSuppressedByPrimaryPam && !root.unlockInProgress && !root.u2fPending && !SessionService.preparingForSleep && !IdleService.monitorsOff && tries < SettingsData.maxFprintTries + readonly property string status: { + if (!available || !SettingsData.enableFprint || !root.lockSecured || root.fprintSuppressedByPrimaryPam) + return "disabled"; + if (tries >= SettingsData.maxFprintTries) + return "max"; + if (!allowed) + return "paused"; + if (active) + return "active"; + if (retrying) + return "retrying"; + return errorTries >= maxErrorTries ? "stopped" : "idle"; + } + + function stop(): void { + errorRetry.stop(); + attemptStartedAt = 0; + abort(); + } + + // pam_fprintd reports an expired timeout as PAM_AUTHINFO_UNAVAIL, the same + // result a device fault gives, and its "Verification timed out" message is + // translated, so age is the signal we can rely on. Its timeout runs from the + // verify rather than from the claim, which is why the age is measured from + // the message announcing the verify: a slow claim then cannot make a fault + // look old enough, and a session that never reached a verify never qualifies. + // That message lands after pam_fprintd has already armed its deadline, so the + // age read here runs short of the real timeout and the slack makes it up. The + // bias is deliberate: a timeout misread as a fault parks the reader behind the + // backoff, while a fault misread as a timeout costs one extra retry. + function attemptSettled(): bool { + return attemptStartedAt > 0 && Date.now() - attemptStartedAt >= sessionTimeoutMs - verifyStartSlackMs; + } function checkAvail(): void { - if (!available || !SettingsData.enableFprint || !root.lockSecured || root.fprintSuppressedByPrimaryPam) { - retrying = false; - abort(); + if (!allowed) { + stop(); + return; + } + if (!active && !retrying) + startAttempt(); + } + + function startAttempt(): void { + if (!allowed || errorTries >= maxErrorTries) { + stop(); return; } if (active) return; + errorRetry.stop(); + if (root.fprintState === "error") + root.fprintState = ""; + attemptStartedAt = 0; + completedDuringStart = false; + if (start()) + return; + if (!completedDuringStart) + scheduleErrorRetry(); + } - tries = 0; + function scheduleErrorRetry(): void { + stop(); + if (!allowed) + return; + errorTries++; + root.fprintState = "error"; + if (errorTries === 1) + root.flashMsg(); + fprintStateReset.restart(); + if (errorTries < maxErrorTries) + errorRetry.restart(); + } + + function recover(): void { + if (!allowed) + return; errorTries = 0; - retrying = false; - start(); + if (!active) + startAttempt(); + } + + onAllowedChanged: { + if (!allowed) + stop(); + else + Qt.callLater(checkAvail); } config: "fprint" configDirectory: Quickshell.shellDir + "/assets/pam" + onPamMessage: { + if (attemptStartedAt === 0) + attemptStartedAt = Date.now(); + } + onCompleted: res => { - if (!available) + completedDuringStart = true; + if (!allowed) return; switch (res) { case PamResult.Success: - retrying = false; - if (!root.unlockInProgress) { - passwd.abort(); - root.proceedAfterPrimaryAuth(); - } + stop(); + passwd.abort(); + root.proceedAfterPrimaryAuth(); return; case PamResult.Error: - errorTries++; - if (errorTries < 200) { - retrying = true; - abort(); - errorRetry.restart(); + if (attemptSettled()) { + stop(); + errorTries = 0; + startAttempt(); return; } - retrying = false; - abort(); + scheduleErrorRetry(); return; + case PamResult.Failed: case PamResult.MaxTries: - retrying = false; + stop(); tries++; - if (tries < SettingsData.maxFprintTries) { - root.fprintState = "fail"; - start(); - } else { - root.fprintState = "max"; - abort(); - } - break; - default: + errorTries = 0; + root.fprintState = tries < SettingsData.maxFprintTries ? "fail" : "max"; + root.flashMsg(); + fprintStateReset.restart(); + startAttempt(); return; } - - root.flashMsg(); - fprintStateReset.start(); } } @@ -426,20 +502,35 @@ Scope { Timer { id: errorRetry - interval: Math.min(1500 * Math.pow(2, Math.max(0, fprint.errorTries - 1)), 30000) - onTriggered: fprint.start() + // A device wedged by a suspend mid-verify only comes back when fprintd + // restarts, and fprintd only exits after its idle timeout with no client + // attached, so the second retry outlasts that instead of climbing to it. + interval: { + if (fprint.errorTries <= 1) + return 1500; + if (fprint.errorTries === 2) + return fprint.daemonIdleExitMs + 5000; + return 60000; + } + onTriggered: fprint.startAttempt() + } + + readonly property bool awaitingActivityRetry: fprint.errorTries > 0 && !fprint.retrying && !fprint.active + + function retryFprintOnActivity(): void { + if (root.awaitingActivityRetry) + fprint.recover(); } Connections { target: SessionService - // timers run on monotonic time, so suspend leaves the backoff remainder to burn after wake (#3171) function onSessionResumed() { - if (!fprint.available || !SettingsData.enableFprint || !root.lockSecured || root.fprintSuppressedByPrimaryPam) - return; - fprint.errorTries = 0; - if (errorRetry.running) - errorRetry.restart(); + fprint.recover(); + } + + function onLidOpened() { + fprint.recover(); } } @@ -491,7 +582,10 @@ Scope { id: fprintStateReset interval: 4000 - onTriggered: root.fprintState = "" + onTriggered: { + if (root.fprintState !== "max") + root.fprintState = ""; + } } onLockSecuredChanged: { @@ -508,6 +602,8 @@ Scope { root.attemptInfoMessages = []; root.lockoutAnnouncedThisAttempt = false; root.resetAuthFlows(); + fprint.tries = 0; + fprint.errorTries = 0; if (!SettingsData.lockPamExternallyManaged && !dankshellConfigWatcher.loaded && !userPamWatcher.loaded) ensureUserPamConfig(); // FileView cannot watch a path that does not exist yet; re-read so a @@ -521,11 +617,17 @@ Scope { target: SettingsData function onEnableFprintChanged(): void { - fprint.checkAvail(); + if (SettingsData.enableFprint) + fprint.recover(); + else + fprint.checkAvail(); } function onLockFingerprintReadyChanged(): void { - fprint.checkAvail(); + if (SettingsData.lockFingerprintReady) + fprint.recover(); + else + fprint.checkAvail(); } function onEnableU2fChanged(): void { diff --git a/quickshell/Services/SessionService.qml b/quickshell/Services/SessionService.qml index bd2f641b2..bd45b14a6 100644 --- a/quickshell/Services/SessionService.qml +++ b/quickshell/Services/SessionService.qml @@ -42,11 +42,14 @@ Singleton { signal sessionLocked signal sessionUnlocked signal sessionResumed + signal lidOpened signal loginctlStateChanged property bool stateInitialized: false property string prepareForSleepSubscriptionId: "" property bool prepareForSleepSubscriptionPending: false + property string lidSubscriptionId: "" + property bool lidSubscriptionPending: false property double lastResumeSignalTimestamp: 0 readonly property string socketPath: Quickshell.env("DMS_SOCKET") @@ -696,7 +699,7 @@ Singleton { if (DMSService.isConnected) { checkDMSCapabilities(); } else { - clearPrepareForSleepSubscriptionState(); + clearPowerSubscriptionState(); } } @@ -714,10 +717,11 @@ Singleton { } function onDbusSignalReceived(subscriptionId, data) { - if (subscriptionId !== prepareForSleepSubscriptionId) { - return; + if (subscriptionId === prepareForSleepSubscriptionId) { + handlePrepareForSleepSignal(data); + } else if (subscriptionId === lidSubscriptionId) { + handleLidPropertiesChanged(data); } - handlePrepareForSleepSignal(data); } } @@ -779,14 +783,41 @@ Singleton { if (DMSService.capabilities.includes("dbus")) { ensurePrepareForSleepSubscription(); + ensureLidSubscription(); } else { - clearPrepareForSleepSubscriptionState(); + clearPowerSubscriptionState(); } } - function clearPrepareForSleepSubscriptionState() { + function clearPowerSubscriptionState() { prepareForSleepSubscriptionId = ""; prepareForSleepSubscriptionPending = false; + lidSubscriptionId = ""; + lidSubscriptionPending = false; + } + + function ensureLidSubscription() { + if (!DMSService.isConnected || !DMSService.capabilities.includes("dbus")) + return; + if (lidSubscriptionId || lidSubscriptionPending) + return; + + lidSubscriptionPending = true; + DMSService.dbusSubscribe("system", "org.freedesktop.UPower", "/org/freedesktop/UPower", "org.freedesktop.DBus.Properties", "PropertiesChanged", response => { + lidSubscriptionPending = false; + if (response.error) { + log.warn("Failed to subscribe to lid changes:", response.error); + return; + } + lidSubscriptionId = response.result?.subscriptionId || ""; + }); + } + + function handleLidPropertiesChanged(data) { + if (data?.path !== "/org/freedesktop/UPower" || data.body?.[0] !== "org.freedesktop.UPower") + return; + if (data.body?.[1]?.LidIsClosed === false) + lidOpened(); } function ensurePrepareForSleepSubscription() { diff --git a/quickshell/assets/pam/fprint b/quickshell/assets/pam/fprint index b44bf7533..54b0da449 100644 --- a/quickshell/assets/pam/fprint +++ b/quickshell/assets/pam/fprint @@ -1,6 +1,9 @@ #%PAM-1.0 -auth required pam_fprintd.so max-tries=5 +# timeout bounds how long a stuck fprintd session can hold the reader. +# Modules/Lock/Pam.qml mirrors the value in sessionTimeoutMs to tell an expiry +# apart from a device fault; change both together. +auth required pam_fprintd.so max-tries=1 timeout=90 account required pam_permit.so password required pam_deny.so session required pam_permit.so