fix(security): use OsRng for all security-critical key and token generation (#519)

* fix(security): use OsRng for all security-critical key and token generation

Replace rand::thread_rng() with rand::rngs::OsRng in all security-critical
code paths that generate cryptographic key material, bearer tokens, PKCE
verifiers, CSRF state parameters, and webhook secrets. thread_rng() uses a
userspace CSPRNG (ChaCha) seeded from OS entropy, which is fine for
non-security contexts but adds an unnecessary intermediate layer for
key material where direct OS entropy (OsRng) is the correct choice.

Files changed:
- src/secrets/keychain.rs: master encryption key generation
- src/secrets/crypto.rs: per-secret HKDF salt generation
- src/orchestrator/auth.rs: per-job bearer token generation
- src/channels/web/mod.rs: gateway auth token fallback
- src/cli/oauth_defaults.rs: OAuth PKCE verifier and CSRF state
- src/tools/mcp/auth.rs: MCP OAuth PKCE verifier
- src/extensions/manager.rs: auto-generated extension secrets
- src/setup/channels.rs: webhook secret generation

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(security): address PR review feedback for OsRng migration

- Remove shadowing inner `use rand::rngs::OsRng` in `generate_salt()`;
  use module-level `aes_gcm::aead::OsRng` import instead (same type,
  avoids divergence risk if rand_core versions drift)
- Fix missed callsites in `pairing/store.rs`: `random_code()` and
  `generate_unique_code()` now use `OsRng` for pairing auth codes
- Add regression tests for `generate_salt()`: correct length,
  non-zero output, uniqueness across calls

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>

src/channels/web/mod.rs

@@ -63,13 +63,11 @@ impl GatewayChannel {
     /// If no auth token is configured, generates a random one and prints it.
     pub fn new(config: GatewayConfig) -> Self {
         let auth_token = config.auth_token.clone().unwrap_or_else(|| {
-            use rand::Rng;
-            let token: String = rand::thread_rng()
-                .sample_iter(&rand::distributions::Alphanumeric)
-                .take(32)
-                .map(char::from)
-                .collect();
-            token
+            use rand::RngCore;
+            use rand::rngs::OsRng;
+            let mut bytes = [0u8; 32];
+            OsRng.fill_bytes(&mut bytes);
+            bytes.iter().map(|b| format!("{b:02x}")).collect()
         });
 
         let state = Arc::new(GatewayState {

src/cli/oauth_defaults.rs

@@ -353,7 +353,7 @@ pub fn build_oauth_url(
     // Generate PKCE verifier and challenge
     let (code_verifier, code_challenge) = if use_pkce {
         let mut verifier_bytes = [0u8; 32];
-        rand::thread_rng().fill_bytes(&mut verifier_bytes);
+        rand::rngs::OsRng.fill_bytes(&mut verifier_bytes);
         let verifier = URL_SAFE_NO_PAD.encode(verifier_bytes);
 
         let mut hasher = Sha256::new();
@@ -367,7 +367,7 @@ pub fn build_oauth_url(
 
     // Generate random state for CSRF protection
     let mut state_bytes = [0u8; 32];
-    rand::thread_rng().fill_bytes(&mut state_bytes);
+    rand::rngs::OsRng.fill_bytes(&mut state_bytes);
     let state = URL_SAFE_NO_PAD.encode(state_bytes);
 
     // Build authorization URL

src/extensions/manager.rs

@@ -2943,8 +2943,9 @@ impl ExtensionManager {
                             .unwrap_or(false);
                         if !already_provided && !already_stored {
                             use rand::RngCore;
+                            use rand::rngs::OsRng;
                             let mut bytes = vec![0u8; auto_gen.length];
-                            rand::thread_rng().fill_bytes(&mut bytes);
+                            OsRng.fill_bytes(&mut bytes);
                             let hex_value: String =
                                 bytes.iter().map(|b| format!("{b:02x}")).collect();
                             let params = CreateSecretParams::new(&secret_def.name, &hex_value)

src/orchestrator/auth.rs

@@ -14,7 +14,6 @@ use axum::extract::{Request, State};
 use axum::http::StatusCode;
 use axum::middleware::Next;
 use axum::response::Response;
-use rand::Rng;
 use serde::{Deserialize, Serialize};
 use subtle::ConstantTimeEq;
 use tokio::sync::RwLock;
@@ -98,8 +97,10 @@ impl Default for TokenStore {
 
 /// Generate a cryptographically random token (32 bytes, hex-encoded = 64 chars).
 fn generate_token() -> String {
+    use rand::RngCore;
+    use rand::rngs::OsRng;
     let mut bytes = [0u8; 32];
-    rand::thread_rng().fill(&mut bytes);
+    OsRng.fill_bytes(&mut bytes);
     // Hex-encode without pulling in a crate: fixed-size array, no allocation concern.
     bytes.iter().fold(String::with_capacity(64), |mut s, b| {
         use std::fmt::Write;

src/pairing/store.rs

@@ -10,6 +10,7 @@ use std::time::{SystemTime, UNIX_EPOCH};
 
 use fs4::FileExt;
 use rand::Rng;
+use rand::rngs::OsRng;
 use serde::{Deserialize, Serialize};
 
 use crate::bootstrap::ironclaw_base_dir;
@@ -147,7 +148,7 @@ fn is_expired(req: &PairingRequest, now_secs: u64) -> bool {
 }
 
 fn random_code() -> String {
-    let mut rng = rand::thread_rng();
+    let mut rng = OsRng;
     (0..PAIRING_CODE_LENGTH)
         .map(|_| {
             let idx = rng.gen_range(0..PAIRING_ALPHABET.len());
@@ -157,7 +158,7 @@ fn random_code() -> String {
 }
 
 fn generate_unique_code(existing: &HashSet<String>) -> String {
-    let mut rng = rand::thread_rng();
+    let mut rng = OsRng;
     for _ in 0..500 {
         let code = random_code();
         if !existing.contains(&code) {

src/secrets/crypto.rs

@@ -59,7 +59,7 @@ impl SecretsCrypto {
     /// Generate a random salt for a new secret.
     pub fn generate_salt() -> Vec<u8> {
         let mut salt = vec![0u8; SALT_SIZE];
-        rand::RngCore::fill_bytes(&mut rand::thread_rng(), &mut salt);
+        rand::RngCore::fill_bytes(&mut OsRng, &mut salt);
         salt
     }
 
@@ -247,4 +247,23 @@ mod tests {
         let decrypted = crypto.decrypt(&encrypted, &salt).unwrap();
         assert_eq!(decrypted.expose().as_bytes(), plaintext.as_slice());
     }
+
+    #[test]
+    fn test_generate_salt_correct_length() {
+        let salt = SecretsCrypto::generate_salt();
+        assert_eq!(salt.len(), super::SALT_SIZE);
+    }
+
+    #[test]
+    fn test_generate_salt_nonzero() {
+        let salt = SecretsCrypto::generate_salt();
+        assert!(salt.iter().any(|&b| b != 0), "salt should not be all zeros");
+    }
+
+    #[test]
+    fn test_generate_salt_unique() {
+        let s1 = SecretsCrypto::generate_salt();
+        let s2 = SecretsCrypto::generate_salt();
+        assert_ne!(s1, s2, "two generated salts should not be identical");
+    }
 }

src/secrets/keychain.rs

@@ -28,8 +28,9 @@ const MASTER_KEY_ACCOUNT: &str = "master_key";
 /// Generate a random 32-byte master key.
 pub fn generate_master_key() -> Vec<u8> {
     use rand::RngCore;
+    use rand::rngs::OsRng;
     let mut key = vec![0u8; 32];
-    rand::thread_rng().fill_bytes(&mut key);
+    OsRng.fill_bytes(&mut key);
     key
 }
 

src/setup/channels.rs

@@ -901,9 +901,9 @@ fn validate_cloudflare_token_format(token: &str) -> bool {
 /// Generate a random secret of specified length (in bytes).
 fn generate_secret_with_length(length: usize) -> String {
     use rand::RngCore;
-    let mut rng = rand::thread_rng();
+    use rand::rngs::OsRng;
     let mut bytes = vec![0u8; length];
-    rng.fill_bytes(&mut bytes);
+    OsRng.fill_bytes(&mut bytes);
     bytes.iter().map(|b| format!("{:02x}", b)).collect()
 }
 

src/tools/mcp/auth.rs

@@ -185,7 +185,7 @@ impl PkceChallenge {
     /// Generate a new PKCE challenge pair.
     pub fn generate() -> Self {
         let mut verifier_bytes = [0u8; 32];
-        rand::thread_rng().fill_bytes(&mut verifier_bytes);
+        rand::rngs::OsRng.fill_bytes(&mut verifier_bytes);
         let verifier = URL_SAFE_NO_PAD.encode(verifier_bytes);
 
         let mut hasher = Sha256::new();