diff --git a/CLAUDE.md b/CLAUDE.md index 143d14f..df0a8f2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,6 +52,7 @@ LD_PRELOAD=/usr/lib/libgtk4-layer-shell.so ./target/release/moonlock - PAM callback: msg_style-aware (password only on PAM_PROMPT_ECHO_OFF), strdup-OOM-safe, num_msg guard against negative values - fprintd: the D-Bus signal sender is validated against fprintd's unique bus name (anti-spoofing) - Password: Zeroizing from GTK entry extraction, Zeroizing in the PAM FFI layer (known limitation: the GLib GString and the strdup copy in PAM are not zeroized — an inherent GTK/libc limitation) +- Core dumps: disabled in release builds via `PR_SET_DUMPABLE(0)` (`main.rs`, right after `setup_logging()`). The non-zeroizable password copies above can therefore never reach an on-disk core dump, and non-root `ptrace` of the running lockscreen is blocked. Debug builds stay dumpable so the dev-mode lockscreen remains debuggable. - Fingerprint unlock: calls unlock_callback directly after a verify match (no additional PAM acct check, because the PAM stack contains only `auth include login` — like swaylock); FP lockout is implemented via MAX_FP_ATTEMPTS in fingerprint.rs - PAM stack: only `auth include login` (standard pattern like swaylock/gtklock); no `account`/`session` stack, because the lockscreen does not manage a session lifecycle and `pam_unix(account)` requires setuid root - The wallpaper is loaded before lock() — connect_monitor fires during lock() and needs the texture; local JPEG loading is fast enough diff --git a/Cargo.lock b/Cargo.lock index fbedeab..b35858a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,7 +575,7 @@ dependencies = [ [[package]] name = "moonlock" -version = "0.6.19" +version = "0.6.20" dependencies = [ "gdk-pixbuf", "gdk4", diff --git a/Cargo.toml b/Cargo.toml index ed0be4b..2fb8fd5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moonlock" -version = "0.6.19" +version = "0.6.20" edition = "2024" description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support" license = "MIT" diff --git a/DECISIONS.md b/DECISIONS.md index 1aa40c4..646e397 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -2,6 +2,13 @@ Architectural and design decisions for Moonlock, in reverse chronological order. +## 2026-07-08 – Unlock SIGSEGV root-caused to a GTK 4.22 regression in gtk4-layer-shell (not our code); harden against core dumps + +- **Who**: ClaudeCode, Dom +- **Why**: `moonlock` was leaving a SIGSEGV core dump on essentially every unlock — 394 dumps since **2026-04-08 10:24**, and **zero** before that, i.e. 24 minutes after the system upgrade `gtk4 1:4.20.4 → 1:4.22.2` that morning. All backtraces are identical: `gtk_session_lock_instance_unlock` → `clear_lock_state` → `gtk_lock_surface_unmap_window` → `gtk_window_destroy` → `GtkApplication::window-removed` (a `VOID__OBJECT` marshal) → `gdk_surface_get_display: assertion 'GDK_IS_SURFACE (surface)' failed`. This is the **same symptom** as the v0.6.9–v0.6.17 crash hunt, which misdiagnosed it as our own `app.quit()` double-destroy and fixed a real-but-secondary double-destroy in v0.6.17 (quit in `::unlocked`, never destroy windows ourselves). The residual crash persisted because the true cause is upstream and was never correlated with the GTK 4.22 bump. Root cause: `gtk4-layer-shell` 1.3.0 `gtk_lock_surface_unmap_window()` unrealizes the lock window **before** destroying it; on GTK 4.22+, `gtk_window_destroy()` removes the window from its `GtkApplication`, and the `window-removed` handler dereferences the already-freed `GdkSurface`. Fixed upstream in [PR #125](https://github.com/wmww/gtk4-layer-shell/pull/125) (merge `cbfdd73`, 2026-06-15): destroy first, unrealize only if still realized. The fix is in `main` only — the latest release is 1.3.0 (Oct 2024), and Arch `extra` ships `1.3.0-1` without it. `moonset`/`moongreet` (plain layer-shell, no session-lock) never crash, confirming the fault is isolated to the session-lock teardown path. +- **Tradeoffs**: (1) **Not security-relevant for access control** — `unlock()` sends the Wayland unlock (`session_lock_unlock`) *before* the crashing window teardown, so the crash is post-unlock cleanup on an already-authenticated path; and any lock-client death *without* a sent unlock keeps the session locked per ext-session-lock-v1 (enforced by the compositor/niri — the Rust panic hook does not apply, SIGSEGV bypasses it). Reframed from an initial over-cautious "session unlocks itself" read. (2) **Did not change our window handling.** Both upstream examples use app-associated windows (`session-lock.c`: `gtk_application_window_new(app)`; `session-lock.py`: `Gtk.Window(application=app)`) and upstream fixed the *library* while leaving the examples unchanged — so the pattern is endorsed and our `ApplicationWindow(application=app)` is correct. Reworking it to dodge the bug was rejected as working around an already-fixed upstream bug by deviating from the supported pattern. (3) **Did not override the system `gtk4-layer-shell`.** It is a shared library (waybar, moonset, moongreet all link it); shadowing it system-wide via `[moonarch]` to fix a moonlock-only unlock path is disproportionate and forks an established `extra` package (maintenance + stale-override risk). Delivery of the real fix is left to the upstream/Arch channel. (4) The core-dump hardening is **release-gated** (matching the file's existing release-only hardening — `exit(1)` without session-lock, `MOONLOCK_DEBUG` gating) so dev builds stay debuggable. +- **How**: (1) `main.rs`: new `disable_core_dumps()` (`#[cfg(not(debug_assertions))]`) calling `libc::prctl(PR_SET_DUMPABLE, 0, …)`, invoked in `main()` right after `setup_logging()` and before any secret enters memory. Suppresses on-disk dumps of the non-zeroizable password copies (GLib GString, PAM strdup — documented limitation) and blocks non-root `ptrace`. `CLAUDE.md` Security section updated. Verified: `cargo build --release` compiles the release-gated path. (2) Deleted the 132 existing moonlock core dumps (~1.2 GB, `root:root 0640`) from `/var/lib/systemd/coredump/`. (3) **Open item, not done here**: deliver PR #125 to the system — report to the Arch `extra` maintainer for a `1.3.0-2` backport. Until then the crash persists (now without a dump); no fix for it lives in this repo, because the fix is not ours to make. + ## 2026-06-17 – Harden avatar load against symlink TOCTOU via a shared O_NOFOLLOW loader (v0.6.19) - **Who**: ClaudeCode, Dom diff --git a/src/main.rs b/src/main.rs index 0bc65ab..56cb5f8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -275,6 +275,26 @@ fn setup_logging() { log::set_max_level(level); } +/// Disable core dumps for this process (release builds only). +/// +/// moonlock holds the user's password in memory during authentication. Our own +/// copies use `Zeroizing`, but the GLib GString from the entry and the strdup +/// copy inside the PAM FFI layer are not zeroizable — on a crash they could be +/// written to a core dump on disk. PR_SET_DUMPABLE(0) suppresses the dump and +/// additionally blocks non-root ptrace of the running lockscreen. +/// Debug builds stay dumpable so the dev-mode lockscreen remains debuggable. +#[cfg(not(debug_assertions))] +fn disable_core_dumps() { + // SAFETY: prctl(PR_SET_DUMPABLE, 0) takes scalar args and has no memory effects. + let rc = unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) }; + if rc != 0 { + log::warn!( + "Failed to disable core dumps (PR_SET_DUMPABLE): {}", + std::io::Error::last_os_error() + ); + } +} + fn install_panic_hook() { // Install a panic hook BEFORE starting the app. // On panic, we log but NEVER unlock. The compositor's ext-session-lock-v1 @@ -290,6 +310,10 @@ fn main() { install_panic_hook(); setup_logging(); + // Suppress core dumps before any secret enters memory (release only). + #[cfg(not(debug_assertions))] + disable_core_dumps(); + // Root check — moonlock should not run as root if nix::unistd::getuid().is_root() { log::error!("Moonlock should not run as root");