diff --git a/src/gateway/mod.rs b/src/gateway/mod.rs index 5e7d28b..4290451 100644 --- a/src/gateway/mod.rs +++ b/src/gateway/mod.rs @@ -23,7 +23,9 @@ use axum::{ }; use std::net::SocketAddr; use std::sync::Arc; +use std::time::Duration; use tower_http::limit::RequestBodyLimitLayer; +use tower_http::timeout::TimeoutLayer; /// Maximum request body size (64KB) — prevents memory exhaustion pub const MAX_BODY_SIZE: usize = 65_536; @@ -163,8 +165,6 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { }; // Build router with middleware - // Note: Body limit layer prevents memory exhaustion from oversized requests - // Timeout is handled by tokio's TcpListener accept timeout and hyper's built-in timeouts let app = Router::new() .route("/health", get(handle_health)) .route("/pair", post(handle_pair)) @@ -172,7 +172,11 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { .route("/whatsapp", get(handle_whatsapp_verify)) .route("/whatsapp", post(handle_whatsapp_message)) .with_state(state) - .layer(RequestBodyLimitLayer::new(MAX_BODY_SIZE)); + .layer(RequestBodyLimitLayer::new(MAX_BODY_SIZE)) + .layer(TimeoutLayer::with_status_code( + StatusCode::REQUEST_TIMEOUT, + Duration::from_secs(REQUEST_TIMEOUT_SECS), + )); // Run the server axum::serve(listener, app).await?; diff --git a/src/security/pairing.rs b/src/security/pairing.rs index dd5f2eb..d7cb0e5 100644 --- a/src/security/pairing.rs +++ b/src/security/pairing.rs @@ -204,15 +204,27 @@ fn is_token_hash(value: &str) -> bool { value.len() == 64 && value.chars().all(|c| c.is_ascii_hexdigit()) } -/// Constant-time string comparison to prevent timing attacks on pairing code. +/// Constant-time string comparison to prevent timing attacks. +/// +/// Does not short-circuit on length mismatch — always iterates over the +/// longer input to avoid leaking length information via timing. pub fn constant_time_eq(a: &str, b: &str) -> bool { - if a.len() != b.len() { - return false; + let a = a.as_bytes(); + let b = b.as_bytes(); + + // Track length mismatch as a usize (non-zero = different lengths) + let len_diff = a.len() ^ b.len(); + + // XOR each byte, padding the shorter input with zeros. + // Iterates over max(a.len(), b.len()) to avoid timing differences. + let max_len = a.len().max(b.len()); + let mut byte_diff = 0u8; + for i in 0..max_len { + let x = *a.get(i).unwrap_or(&0); + let y = *b.get(i).unwrap_or(&0); + byte_diff |= x ^ y; } - a.bytes() - .zip(b.bytes()) - .fold(0u8, |acc, (x, y)| acc | (x ^ y)) - == 0 + (len_diff == 0) & (byte_diff == 0) } /// Check if a host string represents a non-localhost bind address. diff --git a/src/security/secrets.rs b/src/security/secrets.rs index 99cfe9e..2a26831 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -191,14 +191,34 @@ impl SecretStore { #[cfg(windows)] { // On Windows, use icacls to restrict permissions to current user only - let _ = std::process::Command::new("icacls") + let username = std::env::var("USERNAME").unwrap_or_default(); + let Some(grant_arg) = build_windows_icacls_grant_arg(&username) else { + tracing::warn!( + "USERNAME environment variable is empty; \ + cannot restrict key file permissions via icacls" + ); + return Ok(key); + }; + + match std::process::Command::new("icacls") .arg(&self.key_path) .args(["/inheritance:r", "/grant:r"]) - .arg(format!( - "{}:F", - std::env::var("USERNAME").unwrap_or_default() - )) - .output(); + .arg(grant_arg) + .output() + { + Ok(o) if !o.status.success() => { + tracing::warn!( + "Failed to set key file permissions via icacls (exit code {:?})", + o.status.code() + ); + } + Err(e) => { + tracing::warn!("Could not set key file permissions: {e}"); + } + _ => { + tracing::debug!("Key file permissions restricted via icacls"); + } + } } Ok(key) @@ -217,16 +237,12 @@ fn xor_cipher(data: &[u8], key: &[u8]) -> Vec { .collect() } -/// Generate a random key using system entropy (UUID v4 + process ID + timestamp). +/// Generate a random 256-bit key using the OS CSPRNG. +/// +/// Uses `OsRng` (via `getrandom`) directly, providing full 256-bit entropy +/// without the fixed version/variant bits that UUID v4 introduces. fn generate_random_key() -> Vec { - // Use two UUIDs (32 random bytes) as our key material - let u1 = uuid::Uuid::new_v4(); - let u2 = uuid::Uuid::new_v4(); - let mut key = Vec::with_capacity(KEY_LEN); - key.extend_from_slice(u1.as_bytes()); - key.extend_from_slice(u2.as_bytes()); - key.truncate(KEY_LEN); - key + ChaCha20Poly1305::generate_key(&mut OsRng).to_vec() } /// Hex-encode bytes to a lowercase hex string. @@ -239,6 +255,16 @@ fn hex_encode(data: &[u8]) -> String { s } +/// Build the `/grant` argument for `icacls` using a normalized username. +/// Returns `None` when the username is empty or whitespace-only. +fn build_windows_icacls_grant_arg(username: &str) -> Option { + let normalized = username.trim(); + if normalized.is_empty() { + return None; + } + Some(format!("{normalized}:F")) +} + /// Hex-decode a hex string to bytes. #[allow(clippy::manual_is_multiple_of)] fn hex_decode(hex: &str) -> Result> { @@ -733,6 +759,28 @@ mod tests { assert!(hex_decode("zzzz").is_err()); } + #[test] + fn windows_icacls_grant_arg_rejects_empty_username() { + assert_eq!(build_windows_icacls_grant_arg(""), None); + assert_eq!(build_windows_icacls_grant_arg(" \t\n"), None); + } + + #[test] + fn windows_icacls_grant_arg_trims_username() { + assert_eq!( + build_windows_icacls_grant_arg(" alice "), + Some("alice:F".to_string()) + ); + } + + #[test] + fn windows_icacls_grant_arg_preserves_valid_characters() { + assert_eq!( + build_windows_icacls_grant_arg("DOMAIN\\svc-user"), + Some("DOMAIN\\svc-user:F".to_string()) + ); + } + #[test] fn generate_random_key_correct_length() { let key = generate_random_key(); @@ -752,6 +800,39 @@ mod tests { assert_ne!(k1, k2, "Two random keys should differ"); } + #[test] + fn generate_random_key_has_no_uuid_fixed_bits() { + // UUID v4 has fixed bits at positions 6 (version = 0b0100xxxx) and + // 8 (variant = 0b10xxxxxx). A direct CSPRNG key should not consistently + // have these patterns across multiple samples. + let mut version_match = 0; + let mut variant_match = 0; + let samples = 100; + for _ in 0..samples { + let key = generate_random_key(); + // In UUID v4, byte 6 always has top nibble = 0x4 + if key[6] & 0xf0 == 0x40 { + version_match += 1; + } + // In UUID v4, byte 8 always has top 2 bits = 0b10 + if key[8] & 0xc0 == 0x80 { + variant_match += 1; + } + } + // With true randomness, each pattern should appear ~1/16 and ~1/4 of + // the time. UUID would hit 100/100 on both. Allow generous margin. + assert!( + version_match < 30, + "byte[6] matched UUID v4 version nibble {version_match}/100 times — \ + likely still using UUID-based key generation" + ); + assert!( + variant_match < 50, + "byte[8] matched UUID v4 variant bits {variant_match}/100 times — \ + likely still using UUID-based key generation" + ); + } + #[cfg(unix)] #[test] fn key_file_has_restricted_permissions() {