Merge pull request #24 from saorsa-labs/security-hardening-and-test-coverage

dirvine · web-flow · commit 3fd94171b568 · 2026-02-12T21:44:16.000Z
feat: security hardening and comprehensive test coverage
diff --git a/Cargo.toml b/Cargo.toml
@@ -142,6 +142,7 @@ quickcheck = "1.0"
 quickcheck_macros = "1.0"
 approx = "0.5"
 tokio-test = "0.4"
+semver = "1.0"
 
 # All features are now enabled by default - no feature flags needed
 
diff --git a/README.md b/README.md
@@ -34,7 +34,7 @@ Key design decisions are documented in [docs/adr/](docs/adr/):
 
 ## Features
 
-- **P2P NAT Traversal**: True peer-to-peer connectivity with automatic NAT traversal (ant-quic 0.10.0+)
+- **P2P NAT Traversal**: True peer-to-peer connectivity with automatic NAT traversal (ant-quic 0.21.x)
 - **DHT (Distributed Hash Table)**: Peer phonebook and routing with adaptive scoring and geographic awareness
 - **Placement System**: Intelligent shard placement with EigenTrust integration
 - **QUIC Transport**: High-performance networking with ant-quic
@@ -52,7 +52,7 @@ Add this to your `Cargo.toml`:
 
 ```toml
 [dependencies]
-saorsa-core = "0.5.0"
+saorsa-core = "0.11.0"
 ```
 
 ### Basic P2P Node
@@ -108,7 +108,7 @@ tokio::spawn(async move {
 
 ### Core Components
 
-1. **Network Layer**: QUIC-based P2P networking with automatic NAT traversal (ant-quic 0.10.0+)
+1. **Network Layer**: QUIC-based P2P networking with automatic NAT traversal (ant-quic 0.21.x)
 2. **DHT**: S/Kademlia-based peer phonebook with adaptive routing and geographic awareness
 3. **Placement System**: Intelligent shard placement with weighted selection algorithms
 4. **Identity**: Post‑quantum cryptographic identities with ML‑DSA‑65 signatures (no PoW; no embedded four‑word address)
@@ -387,7 +387,7 @@ For commercial licensing, contact: david@saorsalabs.com
 - `tracing` - Logging
 
 ### Networking
-- `ant-quic` (0.10.0+) - QUIC transport with P2P NAT traversal
+- `ant-quic` (0.21.x) - QUIC transport with P2P NAT traversal
 - `four-word-networking` - Human-readable addresses
 
 ### Cryptography
diff --git a/docs/API.md b/docs/API.md
@@ -484,7 +484,7 @@ Enable optional features in `Cargo.toml`:
 
 ```toml
 [dependencies]
-saorsa-core = { version = "0.5", features = ["metrics"] }
+saorsa-core = { version = "0.11", features = ["metrics"] }
 ```
 
 | Feature | Description |
@@ -515,8 +515,9 @@ spawn(async move {
 
 | saorsa-core | ant-quic | Rust | Features |
 |-------------|----------|------|----------|
-| 0.5.x | 0.14.x | 1.75+ | Full PQC, unified config |
-| 0.4.x | 0.10.x | 1.70+ | NAT traversal |
+| 0.11.x | 0.21.x | 1.75+ | Full PQC, placement system, threshold crypto |
+| 0.10.x | 0.20.x | 1.75+ | Full PQC, unified config |
+| 0.5.x | 0.14.x | 1.75+ | Legacy stable |
 
 ---
 
diff --git a/docs/adr/ADR-002-delegated-transport.md b/docs/adr/ADR-002-delegated-transport.md
@@ -132,9 +132,10 @@ We track ant-quic versions explicitly and test against specific releases:
 
 | saorsa-core | ant-quic | Features |
 |-------------|----------|----------|
+| 0.11.x      | 0.21.x   | Full PQC, placement system, threshold crypto |
+| 0.10.x      | 0.20.x   | Full PQC, unified config |
+| 0.5.x       | 0.14.x   | Unified config, PQC integration |
 | 0.3.x       | 0.10.x   | Basic QUIC, NAT traversal |
-| 0.4.x       | 0.14.x   | Unified config, PQC integration |
-| Future      | 0.15+    | Enhanced relay support |
 
 ## Consequences
 
diff --git a/docs/examples/saorsa-node-trust-integration.md b/docs/examples/saorsa-node-trust-integration.md
@@ -9,7 +9,7 @@ Add saorsa-core dependency in your `Cargo.toml` with the `adaptive-ml` feature e
 
 ```toml
 [dependencies]
-saorsa-core = { version = "0.10", features = ["adaptive-ml"] }
+saorsa-core = { version = "0.11.0", features = ["adaptive-ml"] }
 ```
 
 Note: The `adaptive-ml` feature is required for trust API methods (`report_peer_success`,
diff --git a/docs/trust-signals-api.md b/docs/trust-signals-api.md
@@ -17,7 +17,7 @@ The trust API requires the `adaptive-ml` feature to be enabled:
 
 ```toml
 [dependencies]
-saorsa-core = { version = "0.10", features = ["adaptive-ml"] }
+saorsa-core = { version = "0.11.0", features = ["adaptive-ml"] }
 ```
 
 ## Quick Start
diff --git a/src/adaptive/hyperbolic_greedy.rs b/src/adaptive/hyperbolic_greedy.rs
@@ -617,12 +617,10 @@ fn deterministic_distance(peer1: &PeerId, peer2: &PeerId) -> f64 {
 /// It measures distances between peers and optimizes coordinates using gradient descent.
 pub async fn embed_snapshot(peers: &[PeerId]) -> Result<Embedding> {
     // Create a temporary router for embedding
-    let local_id = if !peers.is_empty() {
-        peers[0].clone()
-    } else {
-        // Generate a random PeerId
-        format!("peer_{}", rand::random::<u64>())
-    };
+    let local_id = peers
+        .first()
+        .cloned()
+        .unwrap_or_else(|| format!("peer_{}", rand::random::<u64>()));
 
     let router = HyperbolicGreedyRouter::new(local_id);
     router.embed_snapshot(peers).await
diff --git a/src/adaptive/storage.rs b/src/adaptive/storage.rs
@@ -447,7 +447,11 @@ impl ChunkManager {
             return Err(anyhow::anyhow!("No chunks provided"));
         }
 
-        let total_chunks = sorted_chunks[0].metadata.total_chunks;
+        let total_chunks = sorted_chunks
+            .first()
+            .ok_or_else(|| anyhow::anyhow!("No chunks provided for reconstruction"))?
+            .metadata
+            .total_chunks;
         if sorted_chunks.len() != total_chunks as usize {
             return Err(anyhow::anyhow!(
                 "Missing chunks: have {}, need {}",
diff --git a/src/adaptive/transport.rs b/src/adaptive/transport.rs
@@ -18,14 +18,19 @@
 
 use super::*;
 use async_trait::async_trait;
+use lru::LruCache;
 use std::collections::HashMap;
 use std::net::SocketAddr;
+use std::num::NonZeroUsize;
 use std::sync::Arc;
 use tokio::io::{AsyncRead, AsyncWrite};
 use tokio::net::{TcpListener, TcpStream};
 use tokio::sync::RwLock;
 use tracing;
 
+/// Maximum number of cached TCP connections to prevent memory exhaustion
+const MAX_TCP_CONNECTIONS: usize = 1_000;
+
 /// Transport protocol types
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum TransportProtocol {
@@ -83,8 +88,8 @@ pub trait TransportConnection: AsyncRead + AsyncWrite + Send + Sync + Unpin {
 
 /// TCP transport implementation
 pub struct TcpTransport {
-    /// Connection pool for reuse
-    connections: Arc<RwLock<HashMap<SocketAddr, Arc<RwLock<TcpStream>>>>>,
+    /// Connection pool for reuse (bounded LRU to prevent memory DoS)
+    connections: Arc<RwLock<LruCache<SocketAddr, Arc<RwLock<TcpStream>>>>>,
 }
 
 impl Default for TcpTransport {
@@ -96,7 +101,9 @@ impl Default for TcpTransport {
 impl TcpTransport {
     pub fn new() -> Self {
         Self {
-            connections: Arc::new(RwLock::new(HashMap::new())),
+            connections: Arc::new(RwLock::new(LruCache::new(
+                NonZeroUsize::new(MAX_TCP_CONNECTIONS).unwrap_or(NonZeroUsize::MIN),
+            ))),
         }
     }
 }
@@ -112,10 +119,10 @@ impl Transport for TcpTransport {
     }
 
     async fn connect(&self, addr: SocketAddr) -> Result<Box<dyn TransportConnection>> {
-        // Check connection pool first
+        // Check connection pool first (use peek to avoid mutating LRU order with read lock)
         {
             let connections = self.connections.read().await;
-            if let Some(_conn) = connections.get(&addr) {
+            if let Some(_conn) = connections.peek(&addr) {
                 // TODO: Check if connection is still alive
                 // For now, always create new connection
             }
@@ -390,7 +397,11 @@ impl TransportManager {
 
         // For now, just try the first listener
         // TODO: Implement proper multiplexing
-        listeners[0].accept().await
+        listeners
+            .first()
+            .ok_or_else(|| AdaptiveNetworkError::Other("No listeners available".to_string()))?
+            .accept()
+            .await
     }
 }
 
diff --git a/src/bootstrap/manager.rs b/src/bootstrap/manager.rs
@@ -191,7 +191,14 @@ impl BootstrapManager {
             )));
         }
 
-        let ip = addresses[0].ip();
+        let ip = addresses
+            .first()
+            .ok_or_else(|| {
+                P2PError::Bootstrap(BootstrapError::InvalidData(
+                    "No addresses provided".to_string().into(),
+                ))
+            })?
+            .ip();
 
         // Rate limiting check
         self.rate_limiter.check_join_allowed(&ip).map_err(|e| {
diff --git a/src/dht/core_engine.rs b/src/dht/core_engine.rs
@@ -15,10 +15,12 @@ use crate::dht::routing_maintenance::{
 use crate::dht::trust_peer_selector::{TrustAwarePeerSelector, TrustSelectionConfig};
 use crate::network::NetworkSender;
 use crate::security::{IPDiversityConfig, IPDiversityEnforcer};
-use anyhow::{Result, anyhow};
+use anyhow::{Context, Result, anyhow};
+use lru::LruCache;
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::net::{IpAddr, SocketAddr};
+use std::num::NonZeroUsize;
 use std::sync::Arc;
 use std::time::{Duration, SystemTime};
 use tokio::sync::{RwLock, oneshot};
@@ -165,7 +167,7 @@ pub struct KademliaRoutingTable {
 impl KademliaRoutingTable {
     fn new(node_id: NodeId, k_value: usize) -> Self {
         let mut buckets = Vec::new();
-        for _ in 0..256 {
+        for _ in 0..KADEMLIA_BUCKET_COUNT {
             buckets.push(KBucket::new(k_value));
         }
 
@@ -215,7 +217,7 @@ impl KademliaRoutingTable {
             }
 
             // Early exit: if we have enough candidates, we can stop expanding
-            if candidates.len() >= count * 2 {
+            if candidates.len() >= count * CANDIDATE_EXPANSION_FACTOR {
                 break;
             }
         }
@@ -478,6 +480,20 @@ const MAX_DHT_VALUE_SIZE: usize = 512;
 /// Prevents amplification attacks by limiting response size
 const MAX_FIND_NODE_COUNT: usize = 20;
 
+/// Maximum pending DHT requests before evicting oldest (prevents memory DoS)
+const MAX_PENDING_DHT_REQUESTS: usize = 10_000;
+
+/// Number of K-buckets in Kademlia routing table (one per bit in 256-bit key space)
+const KADEMLIA_BUCKET_COUNT: usize = 256;
+
+/// Candidate expansion factor for find_closest_nodes optimization
+/// Collect 2x requested count before early exit to ensure good selection
+const CANDIDATE_EXPANSION_FACTOR: usize = 2;
+
+/// DHT routing table maintenance interval in seconds
+/// Periodic refresh of buckets and eviction of stale nodes
+const MAINTENANCE_INTERVAL_SECS: u64 = 60;
+
 /// DHT request wrapper with request ID for correlation
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct DhtRequestWrapper {
@@ -516,7 +532,8 @@ pub struct DhtCoreEngine {
     /// Transport handle for sending messages to remote peers
     transport: Option<Arc<dyn NetworkSender>>,
     /// Pending requests waiting for responses (request_id -> response sender)
-    pending_requests: Arc<RwLock<HashMap<String, oneshot::Sender<DhtResponse>>>>,
+    /// LRU cache with max 10k entries to prevent memory DoS
+    pending_requests: Arc<RwLock<LruCache<String, oneshot::Sender<DhtResponse>>>>,
 
     // Trust-weighted peer selection
     /// Optional trust-aware peer selector for combining distance with trust scores
@@ -584,7 +601,10 @@ impl DhtCoreEngine {
             eviction_manager,
             geographic_diversity_enforcer,
             transport: None,
-            pending_requests: Arc::new(RwLock::new(HashMap::new())),
+            pending_requests: Arc::new(RwLock::new(LruCache::new(
+                NonZeroUsize::new(MAX_PENDING_DHT_REQUESTS)
+                    .context("MAX_PENDING_DHT_REQUESTS must be non-zero")?,
+            ))),
             trust_peer_selector: None,
             shutdown: CancellationToken::new(),
         })
@@ -710,7 +730,8 @@ impl DhtCoreEngine {
         let shutdown = self.shutdown.clone();
 
         tokio::spawn(async move {
-            let mut interval = tokio::time::interval(Duration::from_secs(60));
+            let mut interval =
+                tokio::time::interval(Duration::from_secs(MAINTENANCE_INTERVAL_SECS));
             loop {
                 tokio::select! {
                     _ = interval.tick() => {}
@@ -977,10 +998,16 @@ impl DhtCoreEngine {
         // Create response channel
         let (tx, rx) = oneshot::channel();
 
-        // Register pending request
+        // Register pending request - reject if at capacity to avoid evicting in-flight requests
         {
             let mut pending = self.pending_requests.write().await;
-            pending.insert(request_id.clone(), tx);
+            if pending.len() >= MAX_PENDING_DHT_REQUESTS {
+                return Err(anyhow!(
+                    "DHT request capacity exceeded ({} pending requests). Too many concurrent requests.",
+                    MAX_PENDING_DHT_REQUESTS
+                ));
+            }
+            pending.put(request_id.clone(), tx);
         }
 
         // Create the DHT message
@@ -1001,7 +1028,7 @@ impl DhtCoreEngine {
             Err(e) => {
                 // Clean up pending request
                 let mut pending = self.pending_requests.write().await;
-                pending.remove(&request_id);
+                pending.pop(&request_id);
                 return Err(anyhow!(
                     "Failed to serialize DHT request for key {}: {e}",
                     hex::encode(key.as_bytes())
@@ -1019,7 +1046,7 @@ impl DhtCoreEngine {
         {
             // Clean up pending request
             let mut pending = self.pending_requests.write().await;
-            pending.remove(&request_id);
+            pending.pop(&request_id);
             tracing::debug!(peer_id = %peer_id, error = %e, "Failed to send DHT request");
             return Err(anyhow!("Failed to send DHT request to peer {peer_id}: {e}"));
         }
@@ -1048,7 +1075,7 @@ impl DhtCoreEngine {
             Err(_timeout) => {
                 // Timeout - clean up pending request
                 let mut pending = self.pending_requests.write().await;
-                pending.remove(&request_id);
+                pending.pop(&request_id);
                 tracing::debug!(peer_id = %peer_id, "DHT request timed out");
                 Ok(None)
             }
@@ -1061,7 +1088,7 @@ impl DhtCoreEngine {
     /// message is received. It routes the response to the waiting caller.
     pub async fn handle_response(&self, response_wrapper: DhtResponseWrapper) {
         let mut pending = self.pending_requests.write().await;
-        if let Some(tx) = pending.remove(&response_wrapper.id) {
+        if let Some(tx) = pending.pop(&response_wrapper.id) {
             // Send response - log if receiver dropped (timeout or cancelled request)
             if tx.send(response_wrapper.response).is_err() {
                 tracing::debug!(
diff --git a/src/identity/four_words.rs b/src/identity/four_words.rs
@@ -7,6 +7,7 @@ use crate::error::IdentityError;
 use crate::{P2PError, Result};
 use serde::{Deserialize, Serialize};
 use std::fmt;
+use tracing;
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub struct FourWordAddress(pub String);
@@ -67,7 +68,10 @@ impl FourWordAddress {
         let bytes = node_id.to_bytes();
         match Self::from_bytes(&bytes[..6]) {
             Ok(addr) => addr,
-            Err(_) => FourWordAddress(hex::encode(&bytes[..4])),
+            Err(e) => {
+                tracing::warn!("Four-word encoding failed, falling back to hex: {}", e);
+                FourWordAddress(hex::encode(&bytes[..4]))
+            }
         }
     }
 
diff --git a/src/rate_limit.rs b/src/rate_limit.rs
diff --git a/src/secure_memory.rs b/src/secure_memory.rs
diff --git a/src/security.rs b/src/security.rs
diff --git a/src/transport/ant_quic_adapter.rs b/src/transport/ant_quic_adapter.rs
diff --git a/tests/placement_integration_test.rs b/tests/placement_integration_test.rs
diff --git a/tests/upgrade_integration_test.rs b/tests/upgrade_integration_test.rs
