2 Commits

Author SHA1 Message Date
nevaforget 73c59e54c1 fix: drop pam_acct_mgmt from password and FP paths (v0.6.13)
Update PKGBUILD version / update-pkgver (push) Successful in 3s
The PAM stack only ever had `auth include login` — no account module.
auth.rs nevertheless called pam_acct_mgmt after pam_authenticate, which
fell back to /etc/pam.d/other (pam_deny) and rejected every password.
On the FP side, the same call was wrapped in a spawn_blocking + 2s
resume_async retry path that triggered a use-after-free in
gtk_window_destroy (20+ SIGSEGVs in 6 days).

- auth.rs: remove pam_acct_mgmt extern + call; return pam_authenticate
  result directly. Lockout still works via pam_faillock in the auth stack.
- auth.rs: drop check_account() and its tests (FP path no longer needs it).
- lockscreen.rs::start_fingerprint: on success go straight to
  label.set_text + fp.stop() + cb(); no PAM acct check, no resume retry.
- fingerprint.rs: remove resume_async() — no caller left.
- config/moonlock-pam: keep single `auth include login` line, matching
  swaylock/gtklock pattern.
- CLAUDE.md, DECISIONS.md updated.
2026-05-04 09:28:11 +02:00
nevaforget 3e610bdb4b fix: audit LOW fixes — docs, rustdoc, scope, debug gate, lto fat (v0.6.12)
Update PKGBUILD version / update-pkgver (push) Successful in 3s
- Update CLAUDE.md and README.md to reflect the blur range [0,200] that
  the code has clamped to since v0.6.8.
- Move the // SYNC: comment above the /// doc on MAX_BLUR_DIMENSION so
  rustdoc renders one coherent paragraph instead of a truncated sentence.
- Narrow check_account visibility to pub(crate) and document the caller
  precondition (username must come from users::get_current_user()).
- Gate MOONLOCK_DEBUG behind #[cfg(debug_assertions)]. Release builds
  always run at LevelFilter::Info so a session script cannot escalate
  journal verbosity to leak fprintd / D-Bus internals.
- Document why pam_setcred is deliberately not called in authenticate().
- Release profile: lto = "fat" instead of "thin" — doubles release build
  time for better cross-crate inlining on the auth + i18n hot paths.
2026-04-24 14:05:17 +02:00
10 changed files with 46 additions and 146 deletions
+5 -4
View File
@@ -35,13 +35,13 @@ LD_PRELOAD=/usr/lib/libgtk4-layer-shell.so ./target/release/moonlock
## Architektur
- `auth.rs` — PAM-Authentifizierung via Raw FFI (unsafe extern "C" conv callback, msg_style-aware, Zeroizing<CString>), check_account() für pam_acct_mgmt-Only-Checks nach Fingerprint-Unlock
- `auth.rs` — PAM-Authentifizierung via Raw FFI (unsafe extern "C" conv callback, msg_style-aware, Zeroizing<CString>)
- `fingerprint.rs` — fprintd D-Bus Listener, async init/claim/verify via gio futures, sender-validated signal handler, cleanup_dbus() für sauberen D-Bus-Lifecycle, running_flag für Race-Safety in async restarts, on_exhausted callback after MAX_FP_ATTEMPTS, resume_async() für Neustart nach transientem Fehler (mit failed_attempts-Reset und Signal-Handler-Cleanup)
- `users.rs` — Aktuellen User via nix getuid, Avatar-Loading mit Symlink-Rejection
- `power.rs` — Reboot/Shutdown via /usr/bin/systemctl
- `i18n.rs` — Locale-Erkennung (OnceLock-cached) und String-Tabellen (DE/EN), faillock_warning mit konfigurierbarem max_attempts
- `config.rs` — TOML-Config (background_path, background_blur clamped [0,100], fingerprint_enabled als Option<bool>) + Wallpaper-Fallback + Symlink-Rejection via symlink_metadata + Parse-Error-Logging
- `lockscreen.rs` — GTK4 UI via LockscreenHandles, PAM-Auth via gio::spawn_blocking mit 30s Timeout und Generation Counter, FP-Label/Start separat verdrahtet mit pam_acct_mgmt-Check und auto-resume, Zeroizing<String> für Passwort, Power-Confirm, GPU-Blur via GskBlurNode (Downscale auf max 1920px), Blur/Avatar-Cache für Multi-Monitor
- `config.rs` — TOML-Config (background_path, background_blur clamped [0,200], fingerprint_enabled als Option<bool>) + Wallpaper-Fallback + Symlink-Rejection via symlink_metadata + Parse-Error-Logging
- `lockscreen.rs` — GTK4 UI via LockscreenHandles, PAM-Auth via gio::spawn_blocking mit 30s Timeout und Generation Counter, FP-Success ruft unlock_callback direkt (PAM-Stack ohne account-Modul, Lockout via auth-Pfad und MAX_FP_ATTEMPTS), Zeroizing<String> für Passwort, Power-Confirm, GPU-Blur via GskBlurNode (Downscale auf max 1920px), Blur/Avatar-Cache für Multi-Monitor
- `main.rs` — Entry Point, Panic-Hook (vor Logging), Root-Check, ext-session-lock-v1 (Pflicht in Release), Monitor-Hotplug via `connect_monitor`-Signal (v1_2), shared Blur/Avatar-Caches in Rc, systemd-Journal-Logging, Debug-Level per `MOONLOCK_DEBUG` Env-Var, async fprintd-Init nach window.present()
## Sicherheit
@@ -52,7 +52,8 @@ LD_PRELOAD=/usr/lib/libgtk4-layer-shell.so ./target/release/moonlock
- PAM-Callback: msg_style-aware (Passwort nur bei PAM_PROMPT_ECHO_OFF), strdup-OOM-sicher, num_msg-Guard gegen negative Werte
- fprintd: D-Bus Signal-Sender wird gegen fprintd's unique bus name validiert (Anti-Spoofing)
- Passwort: Zeroizing<String> ab GTK-Entry-Extraktion, Zeroizing<CString> im PAM-FFI-Layer (bekannte Einschränkung: GLib-GString und strdup-Kopie in PAM werden nicht gezeroized — inhärente GTK/libc-Limitierung)
- Fingerprint-Unlock: pam_acct_mgmt-Check nach verify-match erzwingt Account-Policies (Lockout, Ablauf), resume_async() startet FP bei transientem Fehler neu (mit failed_attempts-Reset und Signal-Handler-Cleanup)
- Fingerprint-Unlock: ruft unlock_callback direkt nach verify-match (kein zusätzlicher PAM-acct-Check, weil PAM-Stack nur `auth include login` enthält — wie swaylock); FP-Lockout ist über MAX_FP_ATTEMPTS in fingerprint.rs umgesetzt
- PAM-Stack: nur `auth include login` (Standard-Pattern wie swaylock/gtklock); kein `account`/`session`-Stack, weil Lockscreen keinen Session-Lifecycle managed und `pam_unix(account)` setuid root benötigt
- Wallpaper wird vor lock() geladen — connect_monitor feuert während lock() und braucht die Textur; lokales JPEG-Laden ist schnell genug
- PAM-Timeout: 30s Timeout verhindert permanentes Aussperren bei hängenden PAM-Modulen, Generation Counter verhindert Interferenz paralleler Auth-Versuche
- Root-Check: Exit mit Fehler wenn als root gestartet
Generated
+1 -1
View File
@@ -575,7 +575,7 @@ dependencies = [
[[package]]
name = "moonlock"
version = "0.6.11"
version = "0.6.13"
dependencies = [
"gdk-pixbuf",
"gdk4",
+2 -2
View File
@@ -1,6 +1,6 @@
[package]
name = "moonlock"
version = "0.6.11"
version = "0.6.13"
edition = "2024"
description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support"
license = "MIT"
@@ -28,6 +28,6 @@ tempfile = "3"
glib-build-tools = "0.22"
[profile.release]
lto = "thin"
lto = "fat"
codegen-units = 1
strip = true
+14
View File
@@ -2,6 +2,20 @@
Architectural and design decisions for Moonlock, in reverse chronological order.
## 2026-05-04 Drop PAM account/session stack, remove `check_account`, drop `pam_acct_mgmt` from password path (v0.6.13)
- **Who**: ClaudeCode, Dom
- **Why**: 20 SIGSEGV coredumps in 6 days. All crashes preceded by `pam_unix(moonlock:account): setuid failed: Operation not permitted`. The previous PAM config (`auth/account/session include system-auth`) pulled `pam_unix(account)`, which needs setuid root for `/etc/shadow`. moonlock runs as the user, so `pam_acct_mgmt` always failed. The failure cascaded into the FP-resume path (`gio::spawn_blocking(check_account) → false → 2s timeout → resume_async`) where a use-after-free during `gtk_window_destroy` killed the process. Each crash left a dead `ext-session-lock-v1` client and the compositor stuck on its red fallback backdrop until manual recovery. Initial fix on 2026-04-30 dropped the FP-side `check_account` and the account/session lines from the PAM config, but left the `pam_acct_mgmt` call in `auth.rs::authenticate()` for the password path. Result: a PAM stack with no `account` module fell back to `/etc/pam.d/other` (`pam_deny`) and rejected every password — Dom got locked out on 2026-05-04.
- **Tradeoffs**: Aligned with the swaylock/gtklock pattern (only `auth include login`). Lost: PAM-driven account expiry/lockout in both paths. Acceptable because (a) FP attempts are still bounded by `MAX_FP_ATTEMPTS` in `fingerprint.rs`, (b) password-path lockout still works through `pam_faillock.so preauth/authfail/authsucc` in the inherited `auth` stack, and (c) account validity was already verified by the login manager when the session was opened — a lockscreen unlocks an existing session, it does not gate access to a new one. Not chosen: a custom `account` stack with `pam_faillock.so` only — would have kept PAM-level FP lockout but adds a non-standard config that other lockers do not use, and `pam_faillock` standalone in `account` is rarely tested in the wild.
- **How**: (1) `config/moonlock-pam` reduced to a single `auth include login` line. (2) `auth.rs::check_account()` and its two unit tests removed. (3) `auth.rs::authenticate()` no longer calls `pam_acct_mgmt`; `pam_authenticate` result is returned directly. The `pam_acct_mgmt` extern declaration is removed. (4) `lockscreen.rs::start_fingerprint` simplified — the `gio::spawn_blocking(check_account)` async block, the failure-side error UI, and the 2-second `resume_async` retry path all removed; on FP success the closure now goes `label.set_text(success); fp.stop(); cb()`. (5) `fingerprint.rs::resume_async()` removed. (6) `CLAUDE.md` architecture and security sections updated to describe the new PAM stack.
## 2026-04-24 Audit LOW fixes: docs, rustdoc, check_account scope, debug gating, lto fat (v0.6.12)
- **Who**: ClaudeCode, Dom
- **Why**: Six LOW findings cleared in a single pass. (1) Docs referenced the old `[0,100]` blur range; code clamps `[0,200]` since v0.6.8. (2) The `MAX_BLUR_DIMENSION` doc comment was split by a `// SYNC:` block, producing a truncated sentence in rustdoc. (3) `check_account` was `pub` and relied on callers only ever passing `getuid()`-derived usernames; the contract was not enforced by the type system. (4) `MOONLOCK_DEBUG` env var flipped log verbosity to Debug in release builds, letting a compromised session script escalate journal noise about fprintd / D-Bus. (5) `pam_setcred` absence was undocumented. (6) `[profile.release]` used `lto = "thin"` — fine for most crates, but for a latency-critical auth binary compiled once per release, fat LTO's extra cross-crate inlining is worth the ~1 min build hit.
- **Tradeoffs**: `lto = "fat"` roughly doubles release build time (~30 s → ~60 s) for slightly better inlining across PAM FFI wrappers and the i18n/status paths. `#[cfg(debug_assertions)]` on the debug-level selector means you have to run a debug build to raise log level — inconvenient for live troubleshooting, but aligned with the security-first posture.
- **How**: (1) `CLAUDE.md` + `README.md` updated to `[0,200]`. (2) `// SYNC:` block moved above the `///` doc so rustdoc renders one coherent paragraph. (3) `check_account` visibility narrowed to `pub(crate)` with a `Precondition` paragraph explaining the username contract. (4) Debug-level selection wrapped in `#[cfg(debug_assertions)]`; release builds always run at `LevelFilter::Info`. (5) Added a comment block in `authenticate()` documenting why `pam_setcred` is deliberately absent and where it would hook in if needed. (6) `lto = "fat"` in `Cargo.toml`.
## 2026-04-24 Audit MEDIUM fixes: D-Bus cleanup race, TOCTOU open, FP reset, GTK entry clear (v0.6.11)
- **Who**: ClaudeCode, Dom
+2 -2
View File
@@ -9,7 +9,7 @@ Part of the Moonarch ecosystem.
- **PAM authentication** — Uses system PAM stack (`/etc/pam.d/moonlock`) with 30s timeout and generation counter
- **Fingerprint unlock** — fprintd D-Bus integration with sender validation, async init (window appears instantly), `pam_acct_mgmt` check after verify, auto-resume on transient errors
- **Multi-monitor + hotplug** — Lockscreen on every monitor with shared blur and avatar caches; monitors added after suspend/resume get windows automatically via `connect_monitor` signal
- **GPU blur** — Background blur via GskBlurNode (downscale to max 1920px, configurable 0100)
- **GPU blur** — Background blur via GskBlurNode (downscale to max 1920px, configurable 0200)
- **i18n** — German and English (auto-detected)
- **Faillock warning** — Progressive UI warning after failed attempts, PAM decides lockout
- **Panic safety** — Panic hook logs but never unlocks (installed before logging)
@@ -48,7 +48,7 @@ Create `/etc/moonlock/moonlock.toml` or `~/.config/moonlock/moonlock.toml`:
```toml
background_path = "/usr/share/wallpapers/moon.jpg"
background_blur = 40.0 # 0.0100.0, optional
background_blur = 40.0 # 0.0200.0, optional
fingerprint_enabled = true
```
+1 -3
View File
@@ -1,4 +1,2 @@
#%PAM-1.0
auth include system-auth
account include system-auth
session include system-auth
auth include login
+13 -68
View File
@@ -54,8 +54,6 @@ unsafe extern "C" {
fn pam_authenticate(pamh: *mut libc::c_void, flags: libc::c_int) -> libc::c_int;
fn pam_acct_mgmt(pamh: *mut libc::c_void, flags: libc::c_int) -> libc::c_int;
fn pam_end(pamh: *mut libc::c_void, pam_status: libc::c_int) -> libc::c_int;
}
@@ -191,64 +189,23 @@ pub fn authenticate(username: &str, password: &str) -> bool {
return false;
}
// Safety: handle is valid and non-null after successful pam_start
// Safety: handle is valid and non-null after successful pam_start.
// Note: pam_setcred is intentionally NOT called here. A lockscreen unlocks
// an existing session whose credentials were already established at login;
// refreshing them would duplicate work done by the session's login manager.
// If per-unlock credential refresh (Kerberos tickets, pam_gnome_keyring)
// is ever desired, hook it here with PAM_ESTABLISH_CRED.
//
// pam_acct_mgmt is intentionally NOT called: the PAM stack (`auth include
// login`) has no `account` module, and pam_unix(account) requires setuid
// root for /etc/shadow. Lockout/faillock and policy checks happen inside
// the inherited auth stack via pam_faillock. See DECISIONS.md 2026-04-30.
let auth_ret = unsafe { pam_authenticate(handle, 0) };
let acct_ret = if auth_ret == PAM_SUCCESS {
// Safety: handle is valid, check account restrictions
unsafe { pam_acct_mgmt(handle, 0) }
} else {
auth_ret
};
// Safety: handle is valid, pam_end cleans up the PAM session
unsafe { pam_end(handle, acct_ret) };
unsafe { pam_end(handle, auth_ret) };
acct_ret == PAM_SUCCESS
}
/// Check account restrictions via PAM without authentication.
///
/// Used after fingerprint unlock to enforce account policies (lockout, expiry)
/// that would otherwise be bypassed when not going through pam_authenticate.
/// Returns true if the account is valid and allowed to log in.
pub fn check_account(username: &str) -> bool {
let service = match CString::new("moonlock") {
Ok(c) => c,
Err(_) => return false,
};
let username_cstr = match CString::new(username) {
Ok(c) => c,
Err(_) => return false,
};
// No password needed — we only check account status, not authenticate.
// PAM conv callback is required by pam_start but won't be called for acct_mgmt.
let empty_password = Zeroizing::new(CString::new("").unwrap());
let conv = PamConv {
conv: pam_conv_callback,
appdata_ptr: std::ptr::from_ref::<CString>(&empty_password) as *mut libc::c_void,
};
let mut handle: *mut libc::c_void = ptr::null_mut();
let ret = unsafe {
pam_start(
service.as_ptr(),
username_cstr.as_ptr(),
&conv,
&mut handle,
)
};
if ret != PAM_SUCCESS || handle.is_null() {
return false;
}
let acct_ret = unsafe { pam_acct_mgmt(handle, 0) };
unsafe { pam_end(handle, acct_ret) };
acct_ret == PAM_SUCCESS
auth_ret == PAM_SUCCESS
}
#[cfg(test)]
@@ -285,16 +242,4 @@ mod tests {
let result = authenticate("", "password");
assert!(!result);
}
#[test]
fn check_account_empty_username_fails() {
let result = check_account("");
assert!(!result);
}
#[test]
fn check_account_null_byte_username_fails() {
let result = check_account("user\0name");
assert!(!result);
}
}
-28
View File
@@ -175,29 +175,6 @@ impl FingerprintListener {
Self::begin_verification(listener, username).await;
}
/// Resume fingerprint verification after a transient interruption (e.g. failed
/// PAM account check). Reuses previously stored callbacks. Re-claims the device
/// and restarts verification from scratch. Awaits any in-flight VerifyStop +
/// Release before re-claiming the device so fprintd does not reject the Claim
/// while the previous session is still being torn down.
pub async fn resume_async(
listener: &Rc<RefCell<FingerprintListener>>,
username: &str,
) {
// Drain in-flight cleanup so the device is actually released before Claim.
// Without this, a fast resume after on_verify_status's fire-and-forget
// cleanup races the Release call and fprintd returns "already claimed".
let proxy = listener.borrow_mut().take_cleanup_proxy();
if let Some(proxy) = proxy {
Self::perform_dbus_cleanup(proxy).await;
}
// Deliberately do NOT reset failed_attempts here. An attacker with sensor
// control could otherwise cycle verify-match → check_account fail → resume,
// and the 10-attempt cap would never trigger. The counter decays only via
// a fresh lock session (listener construction).
Self::begin_verification(listener, username).await;
}
/// Claim device, start verification, and connect D-Bus signal handler.
/// Assumes device_proxy is set and callbacks are already stored.
async fn begin_verification(
@@ -366,11 +343,6 @@ impl FingerprintListener {
/// Disconnect the signal handler and clear running flags. Returns the proxy
/// the caller should use for the async D-Bus cleanup (VerifyStop + Release).
///
/// Split into a sync part (signal disconnect, flags) and an async part
/// (`perform_dbus_cleanup`) so callers can either spawn the async work
/// fire-and-forget (via `cleanup_dbus`) or await it to serialize with a
/// subsequent Claim (via `resume_async`).
fn take_cleanup_proxy(&mut self) -> Option<gio::DBusProxy> {
self.running = false;
self.running_flag.set(false);
+2 -38
View File
@@ -427,52 +427,16 @@ pub fn start_fingerprint(
let unlock_cb_fp = handles.unlock_callback.clone();
let fp_rc_success = fp_rc.clone();
let fp_username = handles.username.clone();
let on_success = move || {
let label = fp_label_success.clone();
let cb = unlock_cb_fp.clone();
let fp = fp_rc_success.clone();
let username = fp_username.clone();
glib::idle_add_local_once(move || {
let strings = load_strings(None);
label.set_text(strings.fingerprint_success);
label.add_css_class("success");
// stop() is idempotent — cleanup_dbus() already ran inside on_verify_status,
// but this mirrors the PAM success path for defense-in-depth.
fp.borrow_mut().stop();
// Enforce PAM account policies (lockout, expiry) before unlocking.
// Fingerprint auth bypasses pam_authenticate, so we must explicitly
// check account restrictions via pam_acct_mgmt.
glib::spawn_future_local(async move {
let user = username.clone();
let result = gio::spawn_blocking(move || {
auth::check_account(&user)
}).await;
match result {
Ok(true) => cb(),
_ => {
log::error!("PAM account check failed after fingerprint auth");
let strings = load_strings(None);
label.set_text(strings.wrong_password);
label.remove_css_class("success");
label.add_css_class("failed");
// Restart FP verification after delay — the failure may be
// transient (e.g. PAM module timeout). If the account is truly
// locked, check_account will fail again on next match.
glib::timeout_add_local_once(
std::time::Duration::from_secs(2),
move || {
label.set_text(load_strings(None).fingerprint_prompt);
label.remove_css_class("failed");
glib::spawn_future_local(async move {
FingerprintListener::resume_async(&fp, &username).await;
});
},
);
}
}
});
cb();
});
};
@@ -592,11 +556,11 @@ fn create_background_picture(
background
}
/// Maximum texture dimension for blur input. Textures larger than this are
// SYNC: MAX_BLUR_DIMENSION, render_blurred_texture, and create_background_picture
// are duplicated in moongreet/src/greeter.rs and moonset/src/panel.rs.
// Changes here must be mirrored to the other two projects.
/// Maximum texture dimension for blur input. Textures larger than this are
/// downscaled before blurring — the blur destroys detail anyway, so there is
/// no visible quality loss, but GPU work is reduced significantly.
const MAX_BLUR_DIMENSION: f32 = 1920.0;
+6
View File
@@ -250,11 +250,17 @@ fn setup_logging() {
eprintln!("Failed to create journal logger: {e}");
}
}
// Debug level is only selectable in debug builds. Release binaries ignore
// MOONLOCK_DEBUG so a session script cannot escalate log verbosity to leak
// fprintd / D-Bus internals into the journal.
#[cfg(debug_assertions)]
let level = if std::env::var("MOONLOCK_DEBUG").is_ok() {
log::LevelFilter::Debug
} else {
log::LevelFilter::Info
};
#[cfg(not(debug_assertions))]
let level = log::LevelFilter::Info;
log::set_max_level(level);
}