From f6f33a13aba2f09c7bfa97167442a554221f9b69 Mon Sep 17 00:00:00 2001 From: nevaforget Date: Tue, 31 Mar 2026 11:08:40 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20audit=20fixes=20=E2=80=94=20power=20time?= =?UTF-8?q?out,=20timing=20mitigation,=20release=20profile,=20GREETD=5FSOC?= =?UTF-8?q?K=20cache=20(v0.7.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add 30s timeout with SIGKILL to power actions (adapted from moonset) - Add 500ms minimum login response time against timing enumeration - Cache GREETD_SOCK in GreeterState at startup - Add [profile.release] with LTO, codegen-units=1, strip - Add compressed="true" to GResource CSS/SVG entries - Add SYNC comments to duplicated blur/background functions - Add nix dependency for signal handling in power timeout --- Cargo.lock | 21 +++++++- Cargo.toml | 8 ++- DECISIONS.md | 7 +++ resources/resources.gresource.xml | 4 +- src/greeter.rs | 22 ++++++-- src/power.rs | 89 ++++++++++++++++++++++++------- 6 files changed, 125 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index acfe53b..ce6c60a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -59,6 +59,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" + [[package]] name = "equivalent" version = "1.0.2" @@ -569,7 +575,7 @@ dependencies = [ [[package]] name = "moongreet" -version = "0.7.0" +version = "0.7.1" dependencies = [ "gdk-pixbuf", "gdk4", @@ -580,6 +586,7 @@ dependencies = [ "gtk4", "gtk4-layer-shell", "log", + "nix", "serde", "serde_json", "systemd-journal-logger", @@ -588,6 +595,18 @@ dependencies = [ "zeroize", ] +[[package]] +name = "nix" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" +dependencies = [ + "bitflags", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "once_cell" version = "1.21.4" diff --git a/Cargo.toml b/Cargo.toml index 2fcda62..d27d51b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moongreet" -version = "0.7.0" +version = "0.7.1" edition = "2024" description = "A greetd greeter for Wayland with GTK4 and Layer Shell" license = "MIT" @@ -16,6 +16,7 @@ toml = "0.8" serde = { version = "1", features = ["derive"] } serde_json = "1" graphene-rs = { version = "0.22", package = "graphene-rs" } +nix = { version = "0.29", features = ["signal"] } zeroize = { version = "1", features = ["std"] } log = "0.4" systemd-journal-logger = "2.2" @@ -23,5 +24,10 @@ systemd-journal-logger = "2.2" [dev-dependencies] tempfile = "3" +[profile.release] +lto = "thin" +codegen-units = 1 +strip = true + [build-dependencies] glib-build-tools = "0.22" diff --git a/DECISIONS.md b/DECISIONS.md index f43e25b..12e4bec 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,5 +1,12 @@ # Decisions +## 2026-03-31 – Fourth audit: power timeout, timing mitigation, release profile, GREETD_SOCK caching + +- **Who**: Ragnar, Dom +- **Why**: Fourth triple audit found moongreet power.rs had no timeout on loginctl (greeter could freeze), username enumeration via timing differential, GREETD_SOCK re-read on every login, missing release profile, and missing GResource compression. +- **Tradeoffs**: 500ms minimum login response time adds slight delay on fast auth but prevents timing-based username enumeration. Power timeout (30s + SIGKILL) matches moonset pattern — aggressive but prevents greeter freeze. +- **How**: (1) power.rs adapted from moonset with 30s timeout + SIGKILL (nix dependency added). (2) 500ms min response floor in attempt_login via Instant + glib::timeout_future. (3) GREETD_SOCK cached in GreeterState at startup. (4) `[profile.release]` with LTO, codegen-units=1, strip. (5) `compressed="true"` on GResource entries. (6) SYNC comments on duplicated blur/background functions. + ## 2026-03-30 – Full audit fix: security, quality, performance (v0.6.2) - **Who**: Ragnar, Dom diff --git a/resources/resources.gresource.xml b/resources/resources.gresource.xml index 46067e6..c5dea62 100644 --- a/resources/resources.gresource.xml +++ b/resources/resources.gresource.xml @@ -1,7 +1,7 @@ - style.css - default-avatar.svg + style.css + default-avatar.svg diff --git a/src/greeter.rs b/src/greeter.rs index 196ffab..b6b1390 100644 --- a/src/greeter.rs +++ b/src/greeter.rs @@ -133,6 +133,10 @@ pub fn load_background_texture(bg_path: &Path) -> Option { // -- GPU blur via GskBlurNode ------------------------------------------------- +// SYNC: MAX_BLUR_DIMENSION, render_blurred_texture, and create_background_picture +// are duplicated in moonlock/src/lockscreen.rs and moonset/src/panel.rs. +// Changes here must be mirrored to the other two projects. + /// Maximum texture dimension before downscaling for blur. /// Keeps GPU work reasonable on 4K+ displays. const MAX_BLUR_DIMENSION: f32 = 1920.0; @@ -240,6 +244,7 @@ struct GreeterState { default_avatar_texture: Option, failed_attempts: HashMap, greetd_sock: Arc>>, + greetd_sock_path: Option, login_cancelled: Arc, fingerprint_available: bool, /// Incremented on each user switch to discard stale async results. @@ -281,12 +286,16 @@ pub fn create_greeter_window( log::debug!("GTK theme: {theme}"); } + // Cache GREETD_SOCK at startup — it never changes during runtime + let greetd_sock_path = std::env::var("GREETD_SOCK").ok().filter(|p| !p.is_empty()); + let state = Rc::new(RefCell::new(GreeterState { selected_user: None, avatar_cache: HashMap::new(), default_avatar_texture: None, failed_attempts: HashMap::new(), greetd_sock: Arc::new(Mutex::new(None)), + greetd_sock_path, login_cancelled: Arc::new(std::sync::atomic::AtomicBool::new(false)), fingerprint_available: false, user_switch_generation: 0, @@ -960,9 +969,9 @@ fn attempt_login( session_dropdown: >k::DropDown, ) { log::debug!("Login attempt for user: {}", user.username); - let sock_path = match std::env::var("GREETD_SOCK") { - Ok(p) if !p.is_empty() => p, - _ => { + let sock_path = match state.borrow().greetd_sock_path.clone() { + Some(p) => p, + None => { show_error(error_label, password_entry, strings.greetd_sock_not_set); return; } @@ -1009,6 +1018,8 @@ fn attempt_login( state, async move { let session_name_clone = session_name.clone(); + // Minimum response time to prevent username enumeration via timing + let login_start = std::time::Instant::now(); let result = gio::spawn_blocking(move || { login_worker( &username, @@ -1022,6 +1033,11 @@ fn attempt_login( ) }) .await; + let elapsed = login_start.elapsed(); + let min_response = std::time::Duration::from_millis(500); + if elapsed < min_response { + glib::timeout_future(min_response - elapsed).await; + } match result { Ok(Ok(LoginResult::Success { username })) => { diff --git a/src/power.rs b/src/power.rs index 8ed862c..2f361ee 100644 --- a/src/power.rs +++ b/src/power.rs @@ -2,11 +2,18 @@ // ABOUTME: Wrappers around system commands for the greeter UI. use std::fmt; -use std::process::Command; +use std::io::Read; +use std::process::{Command, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; +use std::time::Duration; + +const POWER_TIMEOUT: Duration = Duration::from_secs(30); #[derive(Debug)] pub enum PowerError { CommandFailed { action: &'static str, message: String }, + Timeout { action: &'static str }, } impl fmt::Display for PowerError { @@ -15,41 +22,79 @@ impl fmt::Display for PowerError { PowerError::CommandFailed { action, message } => { write!(f, "{action} failed: {message}") } + PowerError::Timeout { action } => { + write!(f, "{action} timed out") + } } } } impl std::error::Error for PowerError {} -/// Run a command and return a PowerError on failure. +/// Run a command with timeout and return a PowerError on failure. +/// +/// Uses blocking `child.wait()` with a separate timeout thread that sends +/// SIGKILL after POWER_TIMEOUT. This runs inside `gio::spawn_blocking`, +/// so blocking is expected. fn run_command(action: &'static str, program: &str, args: &[&str]) -> Result<(), PowerError> { log::debug!("Power action: {action} ({program} {args:?})"); - let child = Command::new(program) + let mut child = Command::new(program) .args(args) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) .spawn() .map_err(|e| PowerError::CommandFailed { action, message: e.to_string(), })?; - let output = child - .wait_with_output() - .map_err(|e| PowerError::CommandFailed { - action, - message: e.to_string(), - })?; + let child_pid = nix::unistd::Pid::from_raw(child.id() as i32); + let done = Arc::new(AtomicBool::new(false)); + let done_clone = done.clone(); - if output.status.success() { - log::debug!("Power action {action} completed successfully"); + let timeout_thread = std::thread::spawn(move || { + let interval = Duration::from_millis(100); + let mut elapsed = Duration::ZERO; + while elapsed < POWER_TIMEOUT { + std::thread::sleep(interval); + if done_clone.load(Ordering::Relaxed) { + return; + } + elapsed += interval; + } + // ESRCH if the process already exited — harmless + let _ = nix::sys::signal::kill(child_pid, nix::sys::signal::Signal::SIGKILL); + }); + + let status = child.wait().map_err(|e| PowerError::CommandFailed { + action, + message: e.to_string(), + })?; + + done.store(true, Ordering::Relaxed); + let _ = timeout_thread.join(); + + if status.success() { + log::debug!("Power action {action} completed"); + Ok(()) } else { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(PowerError::CommandFailed { - action, - message: format!("exit code {}: {}", output.status, stderr.trim()), - }); - } + #[cfg(unix)] + { + use std::os::unix::process::ExitStatusExt; + if status.signal() == Some(9) { + return Err(PowerError::Timeout { action }); + } + } - Ok(()) + let mut stderr_buf = String::new(); + if let Some(mut stderr) = child.stderr.take() { + let _ = stderr.read_to_string(&mut stderr_buf); + } + Err(PowerError::CommandFailed { + action, + message: format!("exit code {}: {}", status, stderr_buf.trim()), + }) + } } /// Reboot the system via loginctl. @@ -75,6 +120,12 @@ mod tests { assert_eq!(err.to_string(), "reboot failed: No such file or directory"); } + #[test] + fn power_error_timeout_display() { + let err = PowerError::Timeout { action: "shutdown" }; + assert_eq!(err.to_string(), "shutdown timed out"); + } + #[test] fn run_command_returns_error_for_missing_binary() { let result = run_command("test", "nonexistent-binary-xyz", &[]); @@ -99,7 +150,7 @@ mod tests { #[test] fn run_command_passes_args() { - let result = run_command("test", "true", &["--ignored-arg"]); + let result = run_command("test", "echo", &["hello", "world"]); assert!(result.is_ok()); } }