Fix timing attack vulnerability in ApiKeyAuth::verify

services/api/Cargo.toml

@@ -29,4 +29,12 @@ sha2 = "0.10"
 hmac = "0.12"
 hex = "0.4"
 base64 = "0.22"
+subtle = "2.5"
+
+[dev-dependencies]
+criterion = { version = "0.5", features = ["html_reports"] }
+
+[[bench]]
+name = "api_key_auth"
+harness = false
 [workspace]

services/api/benches/api_key_auth.rs

@@ -0,0 +1,77 @@
+use criterion::{black_box, criterion_group, criterion_main, Criterion};
+use predictiq_api::security::ApiKeyAuth;
+
+fn bench_api_key_verify_constant_time(c: &mut Criterion) {
+    let keys = vec![
+        "aaaaaaaaaaaaaaaa".to_string(),
+        "baaaaaaaaaaaaaaaa".to_string(),
+        "caaaaaaaaaaaaaaaa".to_string(),
+        "daaaaaaaaaaaaaaaa".to_string(),
+        "target-key-123456".to_string(),
+    ];
+    let auth = ApiKeyAuth::new(keys);
+
+    let mut group = c.benchmark_group("api_key_verify_constant_time");
+
+    // Test early mismatch (first character different)
+    group.bench_function("early_mismatch", |b| {
+        b.iter(|| auth.verify(black_box("xaaaaaaaaaaaaaaaa")))
+    });
+
+    // Test late mismatch (last character different)
+    group.bench_function("late_mismatch", |b| {
+        b.iter(|| auth.verify(black_box("aaaaaaaaaaaaaaab")))
+    });
+
+    // Test exact match
+    group.bench_function("exact_match", |b| {
+        b.iter(|| auth.verify(black_box("target-key-123456")))
+    });
+
+    // Test wrong length (shorter)
+    group.bench_function("wrong_length_short", |b| {
+        b.iter(|| auth.verify(black_box("short")))
+    });
+
+    // Test wrong length (longer)
+    group.bench_function("wrong_length_long", |b| {
+        b.iter(|| auth.verify(black_box("this-is-a-very-long-key-that-does-not-match")))
+    });
+
+    // Test no keys scenario
+    let empty_auth = ApiKeyAuth::new(vec![]);
+    group.bench_function("no_keys", |b| {
+        b.iter(|| empty_auth.verify(black_box("any-key")))
+    });
+
+    group.finish();
+}
+
+fn bench_api_key_verify_scalability(c: &mut Criterion) {
+    let mut group = c.benchmark_group("api_key_verify_scalability");
+
+    // Test with different numbers of keys to ensure performance doesn't degrade
+    for &key_count in &[1, 10, 100, 1000] {
+        let keys: Vec<String> = (0..key_count)
+            .map(|i| format!("key-{:016}", i))
+            .collect();
+        let auth = ApiKeyAuth::new(keys);
+
+        group.bench_with_input(
+            format!("keys_{}", key_count),
+            &key_count,
+            |b, _| {
+                b.iter(|| auth.verify(black_box("non-existent-key")))
+            },
+        );
+    }
+
+    group.finish();
+}
+
+criterion_group!(
+    benches,
+    bench_api_key_verify_constant_time,
+    bench_api_key_verify_scalability
+);
+criterion_main!(benches);

services/api/src/api_key_auth_tests.rs

@@ -0,0 +1,56 @@
+#[cfg(test)]
+mod api_key_auth_tests {
+    use crate::security::ApiKeyAuth;
+
+    #[test]
+    fn test_api_key_auth_verify_valid_key() {
+        let auth = ApiKeyAuth::new(vec!["test-key-123".to_string(), "another-key".to_string()]);
+        assert!(auth.verify("test-key-123"));
+        assert!(auth.verify("another-key"));
+    }
+
+    #[test]
+    fn test_api_key_auth_verify_invalid_key() {
+        let auth = ApiKeyAuth::new(vec!["test-key-123".to_string()]);
+        assert!(!auth.verify("wrong-key"));
+        assert!(!auth.verify("test-key-12")); // Different length
+        assert!(!auth.verify("test-key-1234")); // Different length
+        assert!(!auth.verify(""));
+    }
+
+    #[test]
+    fn test_api_key_auth_verify_empty_keys() {
+        let auth = ApiKeyAuth::new(vec![]);
+        assert!(!auth.verify("any-key"));
+        assert!(!auth.verify(""));
+    }
+
+    #[test]
+    fn test_api_key_auth_verify_edge_cases() {
+        let auth = ApiKeyAuth::new(vec!["".to_string(), "a".to_string()]);
+        assert!(auth.verify("")); // Empty string key
+        assert!(auth.verify("a")); // Single character key
+        assert!(!auth.verify("b")); // Same length but different content
+    }
+
+    #[test]
+    fn test_api_key_auth_constant_time_behavior() {
+        // Test that verification time doesn't depend on how many keys match partially
+        let keys = vec![
+            "aaaaaaaaaaaaaaaa".to_string(),
+            "baaaaaaaaaaaaaaaa".to_string(),
+            "caaaaaaaaaaaaaaaa".to_string(),
+            "daaaaaaaaaaaaaaaa".to_string(),
+            "target-key-123456".to_string(),
+        ];
+        let auth = ApiKeyAuth::new(keys);
+
+        // These should all take roughly the same time regardless of where they differ
+        assert!(!auth.verify("aaaaaaaaaaaaaaab")); // Differs at last char of first key
+        assert!(!auth.verify("baaaaaaaaaaaaaaab")); // Differs at last char of second key
+        assert!(!auth.verify("caaaaaaaaaaaaaaab")); // Differs at last char of third key
+        assert!(!auth.verify("daaaaaaaaaaaaaaab")); // Differs at last char of fourth key
+        assert!(auth.verify("target-key-123456")); // Exact match
+        assert!(!auth.verify("target-key-123457")); // Differs at last char of matching key
+    }
+}

services/api/src/blockchain.rs

@@ -1,4 +1,5 @@
 use std::{
+    collections::HashSet,
     sync::{
         atomic::{AtomicBool, Ordering},
         Arc,
@@ -697,14 +698,14 @@ impl BlockchainClient {
 
     pub async fn run_transaction_monitor(self: Arc<Self>) {
         loop {
-            let hashes = self
+            let hashes: Vec<String> = self
                 .monitor
                 .watched_txs
                 .read()
                 .await
                 .iter()
                 .cloned()
-                .collect::<Vec<_>>();
+                .collect();
 
             for hash in hashes {
                 if let Ok(status) = self.transaction_status_cached(&hash).await {

services/api/src/main.rs

@@ -10,6 +10,9 @@ mod rate_limit;
 mod security;
 mod validation;
 
+#[cfg(test)]
+mod api_key_auth_tests;
+
 use std::sync::Arc;
 
 use axum::{

services/api/src/security.rs

@@ -6,14 +6,14 @@ use std::{
 };
 
 use axum::{
-    body::Body,
     extract::{ConnectInfo, Request, State},
     http::{HeaderMap, HeaderValue, StatusCode},
     middleware::Next,
     response::{IntoResponse, Response},
     Json,
 };
 use serde::Serialize;
+use subtle::ConstantTimeEq;
 use tokio::sync::RwLock;
 
 /// Rate limiter configuration
@@ -257,7 +257,19 @@ impl ApiKeyAuth {
     }
 
     pub fn verify(&self, key: &str) -> bool {
-        self.valid_keys.iter().any(|k| k == key)
+        // Use constant-time comparison to prevent timing attacks
+        for valid_key in self.valid_keys.iter() {
+            // First check length equality (this is safe to compare normally)
+            if valid_key.len() != key.len() {
+                continue;
+            }
+
+            // Use constant-time byte comparison for the actual key content
+            if valid_key.as_bytes().ct_eq(key.as_bytes()).into() {
+                return true;
+            }
+        }
+        false
     }
 }
 
@@ -446,4 +458,56 @@ mod tests {
 
         assert_eq!(extract_client_ip(&headers, None), "2001:db8::1");
     }
+
+    #[test]
+    fn test_api_key_auth_verify_valid_key() {
+        let auth = ApiKeyAuth::new(vec!["test-key-123".to_string(), "another-key".to_string()]);
+        assert!(auth.verify("test-key-123"));
+        assert!(auth.verify("another-key"));
+    }
+
+    #[test]
+    fn test_api_key_auth_verify_invalid_key() {
+        let auth = ApiKeyAuth::new(vec!["test-key-123".to_string()]);
+        assert!(!auth.verify("wrong-key"));
+        assert!(!auth.verify("test-key-12")); // Different length
+        assert!(!auth.verify("test-key-1234")); // Different length
+        assert!(!auth.verify(""));
+    }
+
+    #[test]
+    fn test_api_key_auth_verify_empty_keys() {
+        let auth = ApiKeyAuth::new(vec![]);
+        assert!(!auth.verify("any-key"));
+        assert!(!auth.verify(""));
+    }
+
+    #[test]
+    fn test_api_key_auth_verify_edge_cases() {
+        let auth = ApiKeyAuth::new(vec!["".to_string(), "a".to_string()]);
+        assert!(auth.verify("")); // Empty string key
+        assert!(auth.verify("a")); // Single character key
+        assert!(!auth.verify("b")); // Same length but different content
+    }
+
+    #[test]
+    fn test_api_key_auth_constant_time_behavior() {
+        // Test that verification time doesn't depend on how many keys match partially
+        let keys = vec![
+            "aaaaaaaaaaaaaaaa".to_string(),
+            "baaaaaaaaaaaaaaaa".to_string(),
+            "caaaaaaaaaaaaaaaa".to_string(),
+            "daaaaaaaaaaaaaaaa".to_string(),
+            "target-key-123456".to_string(),
+        ];
+        let auth = ApiKeyAuth::new(keys);
+
+        // These should all take roughly the same time regardless of where they differ
+        assert!(!auth.verify("aaaaaaaaaaaaaaab")); // Differs at last char of first key
+        assert!(!auth.verify("baaaaaaaaaaaaaaab")); // Differs at last char of second key
+        assert!(!auth.verify("caaaaaaaaaaaaaaab")); // Differs at last char of third key
+        assert!(!auth.verify("daaaaaaaaaaaaaaab")); // Differs at last char of fourth key
+        assert!(auth.verify("target-key-123456")); // Exact match
+        assert!(!auth.verify("target-key-123457")); // Differs at last char of matching key
+    }
 }