fix: security and correctness audit fixes (v0.4.1)

PAM conv callback: check msg_style (password only for ECHO_OFF),
handle strdup OOM with proper cleanup, null-check PAM handle.

Fingerprint: self-wire D-Bus g-signal in start() via Rc<RefCell<>>
and connect_local — VerifyStatus signals are now actually dispatched.
VerifyStop before VerifyStart in restart_verify.

Lockscreen: password entry stays active after faillock threshold
(PAM decides lockout, not UI), use Zeroizing<String> from GTK entry.

Release builds exit(1) without ext-session-lock-v1 support.

Config: fingerprint_enabled as Option<bool> so empty user config
does not override system config.

Dead code: remove unused i18n strings and fingerprint accessors,
parameterize faillock_warning max_attempts.
This commit is contained in:
nevaforget 2026-03-28 00:06:27 +01:00
parent 64f032cd9a
commit 17f8930ff7
8 changed files with 201 additions and 70 deletions

View File

@ -37,20 +37,23 @@ LD_PRELOAD=/usr/lib/libgtk4-layer-shell.so ./target/release/moonlock
## Architektur
- `auth.rs` — PAM-Authentifizierung via Raw FFI (unsafe extern "C" conv callback, Zeroizing<Vec<u8>>)
- `fingerprint.rs` — fprintd D-Bus Listener (gio::DBusProxy, VerifyStatus Signal-Handling)
- `auth.rs` — PAM-Authentifizierung via Raw FFI (unsafe extern "C" conv callback, msg_style-aware, Zeroizing<Vec<u8>>)
- `fingerprint.rs` — fprintd D-Bus Listener (Rc<RefCell<FingerprintListener>>, self-wiring g-signal via connect_local)
- `users.rs` — Aktuellen User via nix getuid, Avatar-Loading mit Symlink-Rejection
- `power.rs` — Reboot/Shutdown via /usr/bin/loginctl
- `i18n.rs` — Locale-Erkennung und String-Tabellen (DE/EN)
- `config.rs` — TOML-Config (background_path, fingerprint_enabled) + Wallpaper-Fallback
- `lockscreen.rs` — GTK4 UI, PAM-Auth via gio::spawn_blocking, Fingerprint-Indikator, Power-Confirm
- `main.rs` — Entry Point, Panic-Hook, Root-Check, ext-session-lock-v1, Multi-Monitor
- `i18n.rs` — Locale-Erkennung und String-Tabellen (DE/EN), faillock_warning mit konfigurierbarem max_attempts
- `config.rs` — TOML-Config (background_path, fingerprint_enabled als Option<bool>) + Wallpaper-Fallback
- `lockscreen.rs` — GTK4 UI, PAM-Auth via gio::spawn_blocking, Fingerprint-Indikator, Zeroizing<String> für Passwort, Power-Confirm
- `main.rs` — Entry Point, Panic-Hook, Root-Check, ext-session-lock-v1 (Pflicht in Release), Multi-Monitor
## Sicherheit
- ext-session-lock-v1 garantiert: Compositor sperrt alle Surfaces bei lock()
- Release-Build: Ohne ext-session-lock-v1 wird `exit(1)` aufgerufen — kein Fenster-Fallback
- Panic-Hook: Bei Crash wird geloggt, aber NIEMALS unlock() aufgerufen — Screen bleibt schwarz
- Passwort: Zeroizing<Vec<u8>> für sicheres Wiping nach PAM-Callback
- PAM-Callback: msg_style-aware (Passwort nur bei PAM_PROMPT_ECHO_OFF), strdup-OOM-sicher
- Passwort: Zeroizing<String> ab GTK-Entry-Extraktion, Zeroizing<Vec<u8>> im PAM-FFI-Layer
- Root-Check: Exit mit Fehler wenn als root gestartet
- Faillock: UI-Warnung nach 3 Fehlversuchen, aber PAM entscheidet über Lockout (Entry bleibt aktiv)
- Kein Schließen per Escape/Alt-F4 — nur durch erfolgreiche PAM-Auth oder Fingerprint
- GResource-Bundle: CSS/Assets in der Binary kompiliert

View File

@ -1,6 +1,6 @@
[package]
name = "moonlock"
version = "0.4.0"
version = "0.4.1"
edition = "2024"
description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support"
license = "MIT"

View File

@ -7,6 +7,11 @@ use zeroize::Zeroizing;
// PAM return codes
const PAM_SUCCESS: i32 = 0;
const PAM_BUF_ERR: i32 = 5;
// PAM message styles
const PAM_PROMPT_ECHO_OFF: libc::c_int = 1;
const PAM_PROMPT_ECHO_ON: libc::c_int = 2;
/// PAM message structure (pam_message).
#[repr(C)]
@ -60,7 +65,7 @@ unsafe extern "C" {
/// which PAM will free. The appdata_ptr must point to a valid CString (the password).
unsafe extern "C" fn pam_conv_callback(
num_msg: libc::c_int,
_msg: *mut *const PamMessage,
msg: *mut *const PamMessage,
resp: *mut *mut PamResponse,
appdata_ptr: *mut libc::c_void,
) -> libc::c_int {
@ -83,10 +88,48 @@ unsafe extern "C" fn pam_conv_callback(
}
for i in 0..num_msg as isize {
// Safety: strdup allocates with malloc — PAM will free() the resp strings.
// We dereference password which is valid for the lifetime of authenticate().
let resp_ptr = resp_array.offset(i);
(*resp_ptr).resp = libc::strdup((*password).as_ptr());
// Safety: msg is an array of pointers provided by PAM
let pam_msg = *msg.offset(i);
let msg_style = (*pam_msg).msg_style;
match msg_style {
PAM_PROMPT_ECHO_OFF => {
// Password prompt — provide the password via strdup
let dup = libc::strdup((*password).as_ptr());
if dup.is_null() {
// strdup failed (OOM) — free all previously allocated strings
for j in 0..i {
let prev = resp_array.offset(j);
if !(*prev).resp.is_null() {
libc::free((*prev).resp as *mut libc::c_void);
}
}
libc::free(resp_array as *mut libc::c_void);
return PAM_BUF_ERR;
}
(*resp_ptr).resp = dup;
}
PAM_PROMPT_ECHO_ON => {
// Visible prompt — provide empty string, never the password
let empty = libc::strdup(b"\0".as_ptr() as *const libc::c_char);
if empty.is_null() {
for j in 0..i {
let prev = resp_array.offset(j);
if !(*prev).resp.is_null() {
libc::free((*prev).resp as *mut libc::c_void);
}
}
libc::free(resp_array as *mut libc::c_void);
return PAM_BUF_ERR;
}
(*resp_ptr).resp = empty;
}
_ => {
// PAM_ERROR_MSG, PAM_TEXT_INFO, or unknown — no response expected
(*resp_ptr).resp = ptr::null_mut();
}
}
(*resp_ptr).resp_retcode = 0;
}
@ -140,7 +183,11 @@ pub fn authenticate(username: &str, password: &str) -> bool {
return false;
}
// Safety: handle is valid after successful pam_start
if handle.is_null() {
return false;
}
// Safety: handle is valid and non-null after successful pam_start
let auth_ret = unsafe { pam_authenticate(handle, 0) };
let acct_ret = if auth_ret == PAM_SUCCESS {
// Safety: handle is valid, check account restrictions

View File

@ -16,24 +16,39 @@ fn default_config_paths() -> Vec<PathBuf> {
paths
}
/// Raw deserialization struct — fingerprint_enabled is optional so that
/// an empty user config does not override the system config's value.
#[derive(Debug, Clone, Default, Deserialize)]
struct RawConfig {
pub background_path: Option<String>,
pub fingerprint_enabled: Option<bool>,
}
/// Resolved configuration with concrete values.
#[derive(Debug, Clone)]
pub struct Config {
pub background_path: Option<String>,
#[serde(default = "default_fingerprint")]
pub fingerprint_enabled: bool,
}
fn default_fingerprint() -> bool { true }
impl Default for Config {
fn default() -> Self {
Config {
background_path: None,
fingerprint_enabled: true,
}
}
}
pub fn load_config(config_paths: Option<&[PathBuf]>) -> Config {
let default_paths = default_config_paths();
let paths = config_paths.unwrap_or(&default_paths);
let mut merged = Config { fingerprint_enabled: true, ..Config::default() };
let mut merged = Config::default();
for path in paths {
if let Ok(content) = fs::read_to_string(path) {
if let Ok(parsed) = toml::from_str::<Config>(&content) {
if let Ok(parsed) = toml::from_str::<RawConfig>(&content) {
if parsed.background_path.is_some() { merged.background_path = parsed.background_path; }
merged.fingerprint_enabled = parsed.fingerprint_enabled;
if let Some(fp) = parsed.fingerprint_enabled { merged.fingerprint_enabled = fp; }
}
}
}
@ -57,7 +72,7 @@ pub fn resolve_background_path_with(config: &Config, moonarch_wallpaper: &Path)
mod tests {
use super::*;
#[test] fn default_config() { let c = Config::default(); assert!(c.background_path.is_none()); assert!(!c.fingerprint_enabled); }
#[test] fn default_config() { let c = Config::default(); assert!(c.background_path.is_none()); assert!(c.fingerprint_enabled); }
#[test] fn load_default_fingerprint_true() {
let dir = tempfile::tempdir().unwrap();
let conf = dir.path().join("moonlock.toml");
@ -79,6 +94,15 @@ mod tests {
let c = Config { background_path: Some(wp.to_str().unwrap().to_string()), fingerprint_enabled: true };
assert_eq!(resolve_background_path_with(&c, Path::new("/nonexistent")), wp);
}
#[test] fn empty_user_config_preserves_system_fingerprint() {
let dir = tempfile::tempdir().unwrap();
let sys_conf = dir.path().join("system.toml");
let usr_conf = dir.path().join("user.toml");
fs::write(&sys_conf, "fingerprint_enabled = false\n").unwrap();
fs::write(&usr_conf, "").unwrap();
let c = load_config(Some(&[sys_conf, usr_conf]));
assert!(!c.fingerprint_enabled);
}
#[test] fn resolve_gresource_fallback() {
let c = Config::default();
let r = resolve_background_path_with(&c, Path::new("/nonexistent"));

View File

@ -3,6 +3,8 @@
use gio::prelude::*;
use gtk4::gio;
use std::cell::RefCell;
use std::rc::Rc;
const FPRINTD_BUS_NAME: &str = "net.reactivated.Fprint";
const FPRINTD_MANAGER_PATH: &str = "/net/reactivated/Fprint/Manager";
@ -126,24 +128,31 @@ impl FingerprintListener {
}
}
/// Whether the listener is currently running.
pub fn is_running(&self) -> bool {
self.running
}
/// Start listening for fingerprint verification.
pub fn start<F, G>(&mut self, username: &str, on_success: F, on_failure: G)
where
/// Connects the D-Bus g-signal handler internally. The `listener` parameter
/// must be the same `Rc<RefCell<FingerprintListener>>` that owns `self`.
pub fn start<F, G>(
listener: &Rc<RefCell<FingerprintListener>>,
username: &str,
on_success: F,
on_failure: G,
) where
F: Fn() + 'static,
G: Fn() + 'static,
{
let proxy = match &self.device_proxy {
Some(p) => p,
None => return,
let proxy = {
let inner = listener.borrow();
match inner.device_proxy.clone() {
Some(p) => p,
None => return,
}
};
self.on_success = Some(Box::new(on_success));
self.on_failure = Some(Box::new(on_failure));
{
let mut inner = listener.borrow_mut();
inner.on_success = Some(Box::new(on_success));
inner.on_failure = Some(Box::new(on_failure));
}
// Claim the device
let args = glib::Variant::from((&username,));
@ -178,12 +187,42 @@ impl FingerprintListener {
return;
}
self.running = true;
// Connect the g-signal handler on the proxy to dispatch VerifyStatus
let listener_weak = Rc::downgrade(listener);
let signal_id = proxy.connect_local("g-signal", false, move |values| {
// g-signal arguments: (proxy, sender_name, signal_name, parameters)
let signal_name: String = match values[2].get() {
Ok(v) => v,
Err(_) => return None,
};
if signal_name.as_str() != "VerifyStatus" {
return None;
}
// Note: Signal handling is set up by the caller via connect_g_signal()
// because FingerprintListener is not an Rc and the g-signal callback
// needs to reference mutable state. The caller (lockscreen.rs) will
// connect the proxy's "g-signal" and call on_verify_status().
let params = match values[3].get::<glib::Variant>() {
Ok(v) => v,
Err(_) => return None,
};
let status = params
.child_value(0)
.get::<String>()
.unwrap_or_default();
let done = params
.child_value(1)
.get::<bool>()
.unwrap_or(false);
if let Some(listener_rc) = listener_weak.upgrade() {
listener_rc.borrow_mut().on_verify_status(&status, done);
}
None
});
let mut inner = listener.borrow_mut();
inner.signal_id = Some(signal_id);
inner.running = true;
}
/// Process a VerifyStatus signal from fprintd.
@ -228,6 +267,14 @@ impl FingerprintListener {
/// Restart fingerprint verification after a completed attempt.
fn restart_verify(&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",
@ -269,15 +316,6 @@ impl FingerprintListener {
}
}
/// Get a reference to the device proxy for signal connection.
pub fn device_proxy(&self) -> Option<&gio::DBusProxy> {
self.device_proxy.as_ref()
}
/// Store the signal handler ID for cleanup.
pub fn set_signal_id(&mut self, id: glib::SignalHandlerId) {
self.signal_id = Some(id);
}
}
#[cfg(test)]

View File

@ -10,13 +10,11 @@ const DEFAULT_LOCALE_CONF: &str = "/etc/locale.conf";
#[derive(Debug, Clone)]
pub struct Strings {
pub password_placeholder: &'static str,
pub unlock_button: &'static str,
pub reboot_tooltip: &'static str,
pub shutdown_tooltip: &'static str,
pub fingerprint_prompt: &'static str,
pub fingerprint_success: &'static str,
pub fingerprint_failed: &'static str,
pub auth_failed: &'static str,
pub wrong_password: &'static str,
pub reboot_failed: &'static str,
pub shutdown_failed: &'static str,
@ -30,13 +28,11 @@ pub struct Strings {
const STRINGS_DE: Strings = Strings {
password_placeholder: "Passwort",
unlock_button: "Entsperren",
reboot_tooltip: "Neustart",
shutdown_tooltip: "Herunterfahren",
fingerprint_prompt: "Fingerabdruck auflegen zum Entsperren",
fingerprint_success: "Fingerabdruck erkannt",
fingerprint_failed: "Fingerabdruck nicht erkannt",
auth_failed: "Authentifizierung fehlgeschlagen",
wrong_password: "Falsches Passwort",
reboot_failed: "Neustart fehlgeschlagen",
shutdown_failed: "Herunterfahren fehlgeschlagen",
@ -50,13 +46,11 @@ const STRINGS_DE: Strings = Strings {
const STRINGS_EN: Strings = Strings {
password_placeholder: "Password",
unlock_button: "Unlock",
reboot_tooltip: "Reboot",
shutdown_tooltip: "Shut down",
fingerprint_prompt: "Place finger on reader to unlock",
fingerprint_success: "Fingerprint recognized",
fingerprint_failed: "Fingerprint not recognized",
auth_failed: "Authentication failed",
wrong_password: "Wrong password",
reboot_failed: "Reboot failed",
shutdown_failed: "Shutdown failed",
@ -96,10 +90,9 @@ pub fn load_strings(locale: Option<&str>) -> &'static Strings {
match locale.as_str() { "de" => &STRINGS_DE, _ => &STRINGS_EN }
}
pub fn faillock_warning(attempt_count: u32, strings: &Strings) -> Option<String> {
const MAX: u32 = 3;
if attempt_count >= MAX { return Some(strings.faillock_locked.to_string()); }
let remaining = MAX - attempt_count;
pub fn faillock_warning(attempt_count: u32, max_attempts: u32, strings: &Strings) -> Option<String> {
if attempt_count >= max_attempts { return Some(strings.faillock_locked.to_string()); }
let remaining = max_attempts - attempt_count;
if remaining == 1 { return Some(strings.faillock_attempts_remaining.replace("{n}", &remaining.to_string())); }
None
}
@ -133,8 +126,8 @@ mod tests {
assert!(!s.fingerprint_failed.is_empty());
}
#[test] fn faillock_zero() { assert!(faillock_warning(0, load_strings(Some("en"))).is_none()); }
#[test] fn faillock_one() { assert!(faillock_warning(1, load_strings(Some("en"))).is_none()); }
#[test] fn faillock_two() { assert!(faillock_warning(2, load_strings(Some("en"))).is_some()); }
#[test] fn faillock_three() { assert_eq!(faillock_warning(3, load_strings(Some("en"))).unwrap(), "Account may be locked"); }
#[test] fn faillock_zero() { assert!(faillock_warning(0, 3, load_strings(Some("en"))).is_none()); }
#[test] fn faillock_one() { assert!(faillock_warning(1, 3, load_strings(Some("en"))).is_none()); }
#[test] fn faillock_two() { assert!(faillock_warning(2, 3, load_strings(Some("en"))).is_some()); }
#[test] fn faillock_three() { assert_eq!(faillock_warning(3, 3, load_strings(Some("en"))).unwrap(), "Account may be locked"); }
}

View File

@ -10,6 +10,8 @@ use std::cell::RefCell;
use std::path::Path;
use std::rc::Rc;
use zeroize::Zeroizing;
use crate::auth;
use crate::config::Config;
use crate::fingerprint::FingerprintListener;
@ -24,6 +26,7 @@ const FAILLOCK_MAX_ATTEMPTS: u32 = 3;
struct LockscreenState {
failed_attempts: u32,
fp_listener: FingerprintListener,
fp_listener_rc: Option<Rc<RefCell<FingerprintListener>>>,
}
/// Create a lockscreen window for a single monitor.
@ -54,6 +57,7 @@ pub fn create_lockscreen_window(
let state = Rc::new(RefCell::new(LockscreenState {
failed_attempts: 0,
fp_listener,
fp_listener_rc: None,
}));
// Root overlay for background + centered content
@ -205,7 +209,7 @@ pub fn create_lockscreen_window(
#[weak]
password_entry,
move |entry| {
let password = entry.text().to_string();
let password = Zeroizing::new(entry.text().to_string());
if password.is_empty() {
return;
}
@ -223,14 +227,18 @@ pub fn create_lockscreen_window(
password_entry,
async move {
let user = username.clone();
let pass = password.clone();
let pass = Zeroizing::new((*password).clone());
let result = gio::spawn_blocking(move || {
auth::authenticate(&user, &pass)
}).await;
match result {
Ok(true) => {
state.borrow_mut().fp_listener.stop();
let s = state.borrow();
if let Some(ref fp_rc) = s.fp_listener_rc {
fp_rc.borrow_mut().stop();
}
drop(s);
unlock_cb();
}
_ => {
@ -241,13 +249,15 @@ pub fn create_lockscreen_window(
password_entry.set_text("");
if count >= FAILLOCK_MAX_ATTEMPTS {
// Show warning but keep entry active — PAM decides lockout
error_label.set_text(strings.faillock_locked);
error_label.set_visible(true);
password_entry.set_sensitive(false);
password_entry.set_sensitive(true);
password_entry.grab_focus();
} else {
password_entry.set_sensitive(true);
password_entry.grab_focus();
if let Some(warning) = faillock_warning(count, strings) {
if let Some(warning) = faillock_warning(count, FAILLOCK_MAX_ATTEMPTS, strings) {
error_label.set_text(&warning);
} else {
error_label.set_text(strings.wrong_password);
@ -325,10 +335,17 @@ pub fn create_lockscreen_window(
));
};
state
.borrow_mut()
.fp_listener
.start(&user.username, on_success, on_failure);
// Extract the fp_listener into its own Rc<RefCell<>> for signal self-wiring
let fp_rc = {
let mut s = state.borrow_mut();
let listener = std::mem::replace(&mut s.fp_listener, FingerprintListener::new());
Rc::new(RefCell::new(listener))
};
FingerprintListener::start(&fp_rc, &user.username, on_success, on_failure);
// Store back the Rc reference for stop() on unlock
state.borrow_mut().fp_listener_rc = Some(fp_rc);
}
// Fade-in on map

View File

@ -43,8 +43,16 @@ fn activate(app: &gtk::Application) {
if gtk4_session_lock::is_supported() {
activate_with_session_lock(app, &display, &bg_path, &config);
} else {
log::warn!("ext-session-lock-v1 not supported — running in development mode");
activate_without_lock(app, &bg_path, &config);
#[cfg(debug_assertions)]
{
log::warn!("ext-session-lock-v1 not supported — running in development mode");
activate_without_lock(app, &bg_path, &config);
}
#[cfg(not(debug_assertions))]
{
log::error!("ext-session-lock-v1 not supported — refusing to run without session lock");
std::process::exit(1);
}
}
}
@ -90,6 +98,7 @@ fn activate_with_session_lock(
}
}
#[cfg(debug_assertions)]
fn activate_without_lock(
app: &gtk::Application,
bg_path: &PathBuf,