From 15a58eb7da878de0f8b178f0fe49a0ec1ef81529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20R=2E=20Escobar?= Date: Sat, 14 Feb 2026 13:29:58 +0100 Subject: [PATCH 1/2] fix: use CSPRNG for pairing code generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace DefaultHasher + SystemTime + process::id() with UUID v4 (backed by getrandom/urandom CSPRNG) for pairing code generation. The previous implementation used predictable entropy sources (system time to ~1s precision and process ID) with a non-cryptographic hash (SipHash), making the 6-digit code brute-forceable. The new implementation extracts 4 random bytes from a UUID v4 (which uses the OS CSPRNG) and derives the 6-digit code from those. No new dependencies added — reuses existing uuid crate. Adds a test verifying non-deterministic output. Ref: CWE-330 (Use of Insufficiently Random Values) Co-Authored-By: Claude Opus 4.6 --- src/security/pairing.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/security/pairing.rs b/src/security/pairing.rs index 08c99af..814f951 100644 --- a/src/security/pairing.rs +++ b/src/security/pairing.rs @@ -145,16 +145,14 @@ impl PairingGuard { } } -/// Generate a 6-digit numeric pairing code. +/// Generate a 6-digit numeric pairing code using cryptographically secure randomness. fn generate_code() -> String { - use std::collections::hash_map::DefaultHasher; - use std::hash::{Hash, Hasher}; - use std::time::SystemTime; - - let mut hasher = DefaultHasher::new(); - SystemTime::now().hash(&mut hasher); - std::process::id().hash(&mut hasher); - let raw = hasher.finish(); + // UUID v4 uses getrandom (backed by /dev/urandom on Linux, BCryptGenRandom + // on Windows) — a CSPRNG. We extract 4 bytes from it for a uniform random + // number in [0, 1_000_000). + let uuid = uuid::Uuid::new_v4(); + let bytes = uuid.as_bytes(); + let raw = u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]); format!("{:06}", raw % 1_000_000) } @@ -314,6 +312,15 @@ mod tests { assert!(code.chars().all(|c| c.is_ascii_digit())); } + #[test] + fn generate_code_is_not_deterministic() { + // Two codes generated in the same process should differ (with overwhelming + // probability — collision chance is 1 in 1,000,000). + let c1 = generate_code(); + let c2 = generate_code(); + assert_ne!(c1, c2, "Two consecutive codes should differ (CSPRNG)"); + } + #[test] fn generate_token_has_prefix() { let token = generate_token(); From 1c8fe792384a9206b97a9ff59b9ba1ab3e7d7035 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20R=2E=20Escobar?= Date: Sat, 14 Feb 2026 13:48:36 +0100 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20?= =?UTF-8?q?rejection=20sampling=20and=20robust=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use rejection sampling to eliminate modulo bias in generate_code(). Values above the largest multiple of 1_000_000 in u32 are discarded and re-drawn (~0.02% rejection rate). - Make generate_code_is_not_deterministic test robust against the 1-in-10^6 collision chance by trying 10 pairs instead of one. Co-Authored-By: Claude Opus 4.6 --- src/security/pairing.rs | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/security/pairing.rs b/src/security/pairing.rs index 814f951..5f55603 100644 --- a/src/security/pairing.rs +++ b/src/security/pairing.rs @@ -150,10 +150,23 @@ fn generate_code() -> String { // UUID v4 uses getrandom (backed by /dev/urandom on Linux, BCryptGenRandom // on Windows) — a CSPRNG. We extract 4 bytes from it for a uniform random // number in [0, 1_000_000). - let uuid = uuid::Uuid::new_v4(); - let bytes = uuid.as_bytes(); - let raw = u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]); - format!("{:06}", raw % 1_000_000) + // + // Rejection sampling eliminates modulo bias: values above the largest + // multiple of 1_000_000 that fits in u32 are discarded and re-drawn. + // The rejection probability is ~0.02%, so this loop almost always exits + // on the first iteration. + const UPPER_BOUND: u32 = 1_000_000; + const REJECT_THRESHOLD: u32 = (u32::MAX / UPPER_BOUND) * UPPER_BOUND; + + loop { + let uuid = uuid::Uuid::new_v4(); + let bytes = uuid.as_bytes(); + let raw = u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]); + + if raw < REJECT_THRESHOLD { + return format!("{:06}", raw % UPPER_BOUND); + } + } } /// Generate a cryptographically-adequate bearer token (hex-encoded). @@ -314,11 +327,15 @@ mod tests { #[test] fn generate_code_is_not_deterministic() { - // Two codes generated in the same process should differ (with overwhelming - // probability — collision chance is 1 in 1,000,000). - let c1 = generate_code(); - let c2 = generate_code(); - assert_ne!(c1, c2, "Two consecutive codes should differ (CSPRNG)"); + // Two codes should differ with overwhelming probability. We try + // multiple pairs so a single 1-in-10^6 collision doesn't cause + // a flaky CI failure. All 10 pairs colliding is ~1-in-10^60. + for _ in 0..10 { + if generate_code() != generate_code() { + return; // Pass: found a non-matching pair. + } + } + panic!("Generated 10 pairs of codes and all were collisions — CSPRNG failure"); } #[test]