From 3e610bdb4b2a155896cf60da000c874fdf3037ec Mon Sep 17 00:00:00 2001 From: nevaforget Date: Fri, 24 Apr 2026 14:05:17 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20audit=20LOW=20fixes=20=E2=80=94=20docs,?= =?UTF-8?q?=20rustdoc,=20scope,=20debug=20gate,=20lto=20fat=20(v0.6.12)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- CLAUDE.md | 2 +- Cargo.lock | 2 +- Cargo.toml | 4 ++-- DECISIONS.md | 7 +++++++ README.md | 4 ++-- src/auth.rs | 14 ++++++++++++-- src/lockscreen.rs | 2 +- src/main.rs | 6 ++++++ 8 files changed, 32 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9ad4565..1b10d43 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,7 +40,7 @@ LD_PRELOAD=/usr/lib/libgtk4-layer-shell.so ./target/release/moonlock - `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) + Wallpaper-Fallback + Symlink-Rejection via symlink_metadata + Parse-Error-Logging +- `config.rs` — TOML-Config (background_path, background_blur clamped [0,200], fingerprint_enabled als Option) + 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 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() diff --git a/Cargo.lock b/Cargo.lock index 1c9e10d..6b2a38d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,7 +575,7 @@ dependencies = [ [[package]] name = "moonlock" -version = "0.6.11" +version = "0.6.12" dependencies = [ "gdk-pixbuf", "gdk4", diff --git a/Cargo.toml b/Cargo.toml index 738124f..1a07e09 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moonlock" -version = "0.6.11" +version = "0.6.12" 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 diff --git a/DECISIONS.md b/DECISIONS.md index c782a7a..05d9999 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -2,6 +2,13 @@ Architectural and design decisions for Moonlock, in reverse chronological order. +## 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 diff --git a/README.md b/README.md index 07088e4..a9f25d8 100644 --- a/README.md +++ b/README.md @@ -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 0–100) +- **GPU blur** — Background blur via GskBlurNode (downscale to max 1920px, configurable 0–200) - **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.0–100.0, optional +background_blur = 40.0 # 0.0–200.0, optional fingerprint_enabled = true ``` diff --git a/src/auth.rs b/src/auth.rs index 3dc078f..fcc5dc4 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -191,7 +191,12 @@ 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. let auth_ret = unsafe { pam_authenticate(handle, 0) }; let acct_ret = if auth_ret == PAM_SUCCESS { // Safety: handle is valid, check account restrictions @@ -211,7 +216,12 @@ pub fn authenticate(username: &str, password: &str) -> bool { /// 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 { +/// +/// **Precondition**: `username` must be the authenticated system user, derived +/// via `users::get_current_user()` (which reads `getuid()`). Calling this with +/// an attacker-controlled username is unsafe — `pam_acct_mgmt` returns SUCCESS +/// for any valid unlocked account, giving a trivial unlock bypass. +pub(crate) fn check_account(username: &str) -> bool { let service = match CString::new("moonlock") { Ok(c) => c, Err(_) => return false, diff --git a/src/lockscreen.rs b/src/lockscreen.rs index 3199ab8..eefd5ff 100644 --- a/src/lockscreen.rs +++ b/src/lockscreen.rs @@ -592,11 +592,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; diff --git a/src/main.rs b/src/main.rs index 87df818..602ecbf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -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); }