Merge remote-tracking branch 'origin/main' into fix/bearer-token-hashing
# Conflicts: # src/security/pairing.rs
This commit is contained in:
commit
7a03a01fbf
3 changed files with 122 additions and 25 deletions
|
|
@ -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?;
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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<u8> {
|
|||
.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<u8> {
|
||||
// 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<String> {
|
||||
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<Vec<u8>> {
|
||||
|
|
@ -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() {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue