fix: audit fixes — async restart_verify, locale caching, panic safety (v0.5.0)
- restart_verify() now async via spawn_future_local (was blocking main thread) - stop() uses 3s timeout instead of unbounded - load_strings() caches locale detection in OnceLock (was reading /etc/locale.conf on every call) - child_get() replaced with child_value().get() for graceful D-Bus type mismatch handling - Eliminate redundant password clone in auth path (direct move into spawn_blocking) - Add on_exhausted callback: hides fp_label after MAX_FP_ATTEMPTS - Set running=false before on_success callback (prevent double-unlock) - Add 4 unit tests for on_verify_status state machine - Document GLib-GString/CString zeroize limitation in CLAUDE.md
This commit is contained in:
+108
-31
@@ -29,6 +29,7 @@ pub struct FingerprintListener {
|
||||
failed_attempts: u32,
|
||||
on_success: Option<Box<dyn Fn() + 'static>>,
|
||||
on_failure: Option<Box<dyn Fn() + 'static>>,
|
||||
on_exhausted: Option<Box<dyn Fn() + 'static>>,
|
||||
}
|
||||
|
||||
impl FingerprintListener {
|
||||
@@ -42,6 +43,7 @@ impl FingerprintListener {
|
||||
failed_attempts: 0,
|
||||
on_success: None,
|
||||
on_failure: None,
|
||||
on_exhausted: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -77,7 +79,13 @@ impl FingerprintListener {
|
||||
};
|
||||
|
||||
// Extract device path from variant tuple
|
||||
let device_path: String = result.child_get::<String>(0);
|
||||
let device_path = match result.child_value(0).get::<String>() {
|
||||
Some(p) => p,
|
||||
None => {
|
||||
log::debug!("fprintd: unexpected GetDefaultDevice response type");
|
||||
return;
|
||||
}
|
||||
};
|
||||
if device_path.is_empty() {
|
||||
return;
|
||||
}
|
||||
@@ -115,8 +123,13 @@ impl FingerprintListener {
|
||||
{
|
||||
Ok(result) => {
|
||||
// Result is a tuple of (array of strings)
|
||||
let fingers: Vec<String> = result.child_get::<Vec<String>>(0);
|
||||
!fingers.is_empty()
|
||||
match result.child_value(0).get::<Vec<String>>() {
|
||||
Some(fingers) => !fingers.is_empty(),
|
||||
None => {
|
||||
log::debug!("fprintd: unexpected ListEnrolledFingers response type");
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(_) => false,
|
||||
}
|
||||
@@ -126,14 +139,16 @@ impl FingerprintListener {
|
||||
/// Claims the device and starts verification using async D-Bus calls.
|
||||
/// Connects the D-Bus g-signal handler internally. The `listener` parameter
|
||||
/// must be the same `Rc<RefCell<FingerprintListener>>` that owns `self`.
|
||||
pub async fn start_async<F, G>(
|
||||
pub async fn start_async<F, G, H>(
|
||||
listener: &Rc<RefCell<FingerprintListener>>,
|
||||
username: &str,
|
||||
on_success: F,
|
||||
on_failure: G,
|
||||
on_exhausted: H,
|
||||
) where
|
||||
F: Fn() + 'static,
|
||||
G: Fn() + 'static,
|
||||
H: Fn() + 'static,
|
||||
{
|
||||
let proxy = {
|
||||
let inner = listener.borrow();
|
||||
@@ -147,6 +162,7 @@ impl FingerprintListener {
|
||||
let mut inner = listener.borrow_mut();
|
||||
inner.on_success = Some(Box::new(on_success));
|
||||
inner.on_failure = Some(Box::new(on_failure));
|
||||
inner.on_exhausted = Some(Box::new(on_exhausted));
|
||||
}
|
||||
|
||||
// Claim the device
|
||||
@@ -217,6 +233,7 @@ impl FingerprintListener {
|
||||
}
|
||||
|
||||
if status == "verify-match" {
|
||||
self.running = false;
|
||||
if let Some(ref cb) = self.on_success {
|
||||
cb();
|
||||
}
|
||||
@@ -225,23 +242,26 @@ impl FingerprintListener {
|
||||
|
||||
if RETRY_STATUSES.contains(&status) {
|
||||
if done {
|
||||
self.restart_verify();
|
||||
self.restart_verify_async();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if status == "verify-no-match" {
|
||||
self.failed_attempts += 1;
|
||||
if let Some(ref cb) = self.on_failure {
|
||||
cb();
|
||||
}
|
||||
if self.failed_attempts >= MAX_FP_ATTEMPTS {
|
||||
log::warn!("Fingerprint max attempts ({MAX_FP_ATTEMPTS}) reached, stopping");
|
||||
if let Some(ref cb) = self.on_exhausted {
|
||||
cb();
|
||||
}
|
||||
self.stop();
|
||||
return;
|
||||
}
|
||||
if let Some(ref cb) = self.on_failure {
|
||||
cb();
|
||||
}
|
||||
if done {
|
||||
self.restart_verify();
|
||||
self.restart_verify_async();
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -249,31 +269,28 @@ impl FingerprintListener {
|
||||
log::debug!("Unhandled fprintd status: {status}");
|
||||
}
|
||||
|
||||
/// Restart fingerprint verification after a completed attempt.
|
||||
fn restart_verify(&self) {
|
||||
/// Restart fingerprint verification asynchronously after a completed attempt.
|
||||
fn restart_verify_async(&self) {
|
||||
if let Some(ref proxy) = self.device_proxy {
|
||||
// VerifyStop before VerifyStart to avoid D-Bus errors
|
||||
let _ = proxy.call_sync(
|
||||
"VerifyStop",
|
||||
None,
|
||||
gio::DBusCallFlags::NONE,
|
||||
-1,
|
||||
gio::Cancellable::NONE,
|
||||
);
|
||||
let args = glib::Variant::from((&"any",));
|
||||
if let Err(e) = proxy.call_sync(
|
||||
"VerifyStart",
|
||||
Some(&args),
|
||||
gio::DBusCallFlags::NONE,
|
||||
-1,
|
||||
gio::Cancellable::NONE,
|
||||
) {
|
||||
log::error!("Failed to restart fingerprint verification: {e}");
|
||||
}
|
||||
let proxy = proxy.clone();
|
||||
glib::spawn_future_local(async move {
|
||||
// VerifyStop before VerifyStart to avoid D-Bus errors
|
||||
let _ = proxy
|
||||
.call_future("VerifyStop", None, gio::DBusCallFlags::NONE, -1)
|
||||
.await;
|
||||
let args = glib::Variant::from((&"any",));
|
||||
if let Err(e) = proxy
|
||||
.call_future("VerifyStart", Some(&args), gio::DBusCallFlags::NONE, -1)
|
||||
.await
|
||||
{
|
||||
log::error!("Failed to restart fingerprint verification: {e}");
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// Stop listening and release the device.
|
||||
/// Uses a short timeout (3s) to avoid blocking the UI indefinitely.
|
||||
pub fn stop(&mut self) {
|
||||
if !self.running {
|
||||
return;
|
||||
@@ -288,14 +305,14 @@ impl FingerprintListener {
|
||||
"VerifyStop",
|
||||
None,
|
||||
gio::DBusCallFlags::NONE,
|
||||
-1,
|
||||
3000,
|
||||
gio::Cancellable::NONE,
|
||||
);
|
||||
let _ = proxy.call_sync(
|
||||
"Release",
|
||||
None,
|
||||
gio::DBusCallFlags::NONE,
|
||||
-1,
|
||||
3000,
|
||||
gio::Cancellable::NONE,
|
||||
);
|
||||
}
|
||||
@@ -319,4 +336,64 @@ mod tests {
|
||||
fn max_attempts_constant() {
|
||||
assert_eq!(MAX_FP_ATTEMPTS, 10);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_match_sets_running_false_and_calls_success() {
|
||||
use std::cell::Cell;
|
||||
let called = Rc::new(Cell::new(false));
|
||||
let called_clone = called.clone();
|
||||
let mut listener = FingerprintListener::new();
|
||||
listener.running = true;
|
||||
listener.on_success = Some(Box::new(move || { called_clone.set(true); }));
|
||||
|
||||
listener.on_verify_status("verify-match", false);
|
||||
assert!(called.get());
|
||||
assert!(!listener.running);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_no_match_calls_failure_and_stays_running() {
|
||||
use std::cell::Cell;
|
||||
let called = Rc::new(Cell::new(false));
|
||||
let called_clone = called.clone();
|
||||
let mut listener = FingerprintListener::new();
|
||||
listener.running = true;
|
||||
listener.on_failure = Some(Box::new(move || { called_clone.set(true); }));
|
||||
|
||||
listener.on_verify_status("verify-no-match", false);
|
||||
assert!(called.get());
|
||||
assert!(listener.running);
|
||||
assert_eq!(listener.failed_attempts, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn max_attempts_stops_listener_and_calls_exhausted() {
|
||||
use std::cell::Cell;
|
||||
let exhausted = Rc::new(Cell::new(false));
|
||||
let exhausted_clone = exhausted.clone();
|
||||
let mut listener = FingerprintListener::new();
|
||||
listener.running = true;
|
||||
listener.on_failure = Some(Box::new(|| {}));
|
||||
listener.on_exhausted = Some(Box::new(move || { exhausted_clone.set(true); }));
|
||||
|
||||
for _ in 0..MAX_FP_ATTEMPTS {
|
||||
listener.on_verify_status("verify-no-match", true);
|
||||
}
|
||||
assert!(!listener.running);
|
||||
assert!(exhausted.get());
|
||||
assert_eq!(listener.failed_attempts, MAX_FP_ATTEMPTS);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn not_running_ignores_signals() {
|
||||
use std::cell::Cell;
|
||||
let called = Rc::new(Cell::new(false));
|
||||
let called_clone = called.clone();
|
||||
let mut listener = FingerprintListener::new();
|
||||
listener.running = false;
|
||||
listener.on_success = Some(Box::new(move || { called_clone.set(true); }));
|
||||
|
||||
listener.on_verify_status("verify-match", false);
|
||||
assert!(!called.get());
|
||||
}
|
||||
}
|
||||
|
||||
+9
-2
@@ -4,9 +4,13 @@
|
||||
use std::env;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::sync::OnceLock;
|
||||
|
||||
const DEFAULT_LOCALE_CONF: &str = "/etc/locale.conf";
|
||||
|
||||
/// Cached locale prefix — detected once, reused for all subsequent calls.
|
||||
static CACHED_LOCALE: OnceLock<String> = OnceLock::new();
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct Strings {
|
||||
pub password_placeholder: &'static str,
|
||||
@@ -86,8 +90,11 @@ pub fn detect_locale() -> String {
|
||||
}
|
||||
|
||||
pub fn load_strings(locale: Option<&str>) -> &'static Strings {
|
||||
let locale = match locale { Some(l) => l.to_string(), None => detect_locale() };
|
||||
match locale.as_str() { "de" => &STRINGS_DE, _ => &STRINGS_EN }
|
||||
let locale = match locale {
|
||||
Some(l) => l,
|
||||
None => CACHED_LOCALE.get_or_init(detect_locale),
|
||||
};
|
||||
match locale { "de" => &STRINGS_DE, _ => &STRINGS_EN }
|
||||
}
|
||||
|
||||
pub fn faillock_warning(attempt_count: u32, max_attempts: u32, strings: &Strings) -> Option<String> {
|
||||
|
||||
+12
-3
@@ -239,9 +239,8 @@ pub fn create_lockscreen_window(
|
||||
password_entry,
|
||||
async move {
|
||||
let user = username.clone();
|
||||
let pass = Zeroizing::new((*password).clone());
|
||||
let result = gio::spawn_blocking(move || {
|
||||
auth::authenticate(&user, &pass)
|
||||
auth::authenticate(&user, &password)
|
||||
}).await;
|
||||
|
||||
match result {
|
||||
@@ -397,10 +396,20 @@ pub fn start_fingerprint(
|
||||
));
|
||||
};
|
||||
|
||||
let fp_label_exhausted = handles.fp_label.clone();
|
||||
let on_exhausted = move || {
|
||||
let label = fp_label_exhausted.clone();
|
||||
glib::idle_add_local_once(move || {
|
||||
label.set_visible(false);
|
||||
});
|
||||
};
|
||||
|
||||
let username = handles.username.clone();
|
||||
let fp_rc_clone = fp_rc.clone();
|
||||
glib::spawn_future_local(async move {
|
||||
FingerprintListener::start_async(&fp_rc_clone, &username, on_success, on_failure).await;
|
||||
FingerprintListener::start_async(
|
||||
&fp_rc_clone, &username, on_success, on_failure, on_exhausted,
|
||||
).await;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user