fix: security hardening — replace fake HMAC, add path traversal protection, OTA auth (ADR-050)

Sprint 1 security fixes from quality engineering analysis (issue #170):

- Replace XOR-fold fake HMAC with real HMAC-SHA256 (hmac + sha2 crates) in secure_tdm.rs
- Add path traversal sanitization on DELETE /api/v1/models/:id and /api/v1/recording/:id
- Default bind address changed from 0.0.0.0 to 127.0.0.1 (configurable via --bind-addr / SENSING_BIND_ADDR)
- Add PSK authentication to ESP32 OTA firmware upload endpoint (ota_update.c)
- Flip WASM signature verification to default-on (CONFIG_WASM_SKIP_SIGNATURE opt-out vs opt-in)
- Add 6 new security tests: HMAC key/message sensitivity, determinism, wrong-key rejection, bit-flip detection, enforcing mode
- Add clap env feature for environment variable configuration

All 106 hardware crate tests pass. Sensing server compiles clean.

Closes #170

Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
ruv
2026-03-06 13:11:04 -05:00
parent c45690ed4e
commit 47223a98be
8 changed files with 313 additions and 20 deletions
+2
View File
@@ -4579,10 +4579,12 @@ dependencies = [
"chrono",
"clap",
"criterion",
"hmac",
"midstreamer-quic",
"midstreamer-scheduler",
"serde",
"serde_json",
"sha2",
"thiserror 1.0.69",
"tokio",
"tracing",
+1 -1
View File
@@ -101,7 +101,7 @@ csv = "1.3"
indicatif = "0.17"
# CLI
clap = { version = "4.4", features = ["derive"] }
clap = { version = "4.4", features = ["derive", "env"] }
# Testing
criterion = { version = "0.5", features = ["html_reports"] }
@@ -24,6 +24,9 @@ linux-wifi = []
[dependencies]
# CLI argument parsing (for bin/aggregator)
clap = { version = "4.4", features = ["derive"] }
# Cryptographic HMAC (ADR-050: replace fake XOR-fold HMAC)
hmac = "0.12"
sha2 = "0.10"
# Byte parsing
byteorder = "1.5"
# Time
@@ -33,9 +33,13 @@ use super::quic_transport::{
QuicTransportHandle, QuicTransportError, SecurityMode,
};
use super::tdm::{SyncBeacon, TdmCoordinator, TdmSchedule, TdmSlotCompleted};
use hmac::{Hmac, Mac};
use sha2::Sha256;
use std::collections::VecDeque;
use std::fmt;
type HmacSha256 = Hmac<Sha256>;
// ---------------------------------------------------------------------------
// Constants
// ---------------------------------------------------------------------------
@@ -245,19 +249,17 @@ impl AuthenticatedBeacon {
})
}
/// Compute the expected HMAC tag for this beacon using the given key.
/// Compute the HMAC-SHA256 tag for this beacon, truncated to 8 bytes.
///
/// Uses a simplified HMAC approximation for testing. In production,
/// this calls mbedtls HMAC-SHA256 via the ESP-IDF hardware accelerator
/// or the `sha2` crate on aggregator nodes.
/// Uses the `hmac` + `sha2` crates for cryptographically secure
/// message authentication (ADR-050, Sprint 1).
pub fn compute_tag(payload_and_nonce: &[u8], key: &[u8; 16]) -> [u8; HMAC_TAG_SIZE] {
// Simplified HMAC: XOR key into payload hash. In production, use
// real HMAC-SHA256 from sha2 crate. This is sufficient for
// testing the protocol structure.
let mut mac = HmacSha256::new_from_slice(key)
.expect("HMAC-SHA256 accepts any key length");
mac.update(payload_and_nonce);
let result = mac.finalize().into_bytes();
let mut tag = [0u8; HMAC_TAG_SIZE];
for (i, byte) in payload_and_nonce.iter().enumerate() {
tag[i % HMAC_TAG_SIZE] ^= byte ^ key[i % 16];
}
tag.copy_from_slice(&result[..HMAC_TAG_SIZE]);
tag
}
@@ -975,6 +977,97 @@ mod tests {
assert_eq!(SecLevel::Enforcing as u8, 2);
}
// ---- Security tests (ADR-050) ----
#[test]
fn test_hmac_different_keys_produce_different_tags() {
let msg = b"test payload with nonce";
let key1: [u8; 16] = [0x01; 16];
let key2: [u8; 16] = [0x02; 16];
let tag1 = AuthenticatedBeacon::compute_tag(msg, &key1);
let tag2 = AuthenticatedBeacon::compute_tag(msg, &key2);
assert_ne!(tag1, tag2, "Different keys must produce different HMAC tags");
}
#[test]
fn test_hmac_different_messages_produce_different_tags() {
let key: [u8; 16] = DEFAULT_TEST_KEY;
let tag1 = AuthenticatedBeacon::compute_tag(b"message one", &key);
let tag2 = AuthenticatedBeacon::compute_tag(b"message two", &key);
assert_ne!(tag1, tag2, "Different messages must produce different HMAC tags");
}
#[test]
fn test_hmac_is_deterministic() {
let key: [u8; 16] = DEFAULT_TEST_KEY;
let msg = b"determinism test";
let tag1 = AuthenticatedBeacon::compute_tag(msg, &key);
let tag2 = AuthenticatedBeacon::compute_tag(msg, &key);
assert_eq!(tag1, tag2, "Same key + message must produce identical tags");
}
#[test]
fn test_wrong_key_fails_verification() {
let beacon = SyncBeacon {
cycle_id: 42,
cycle_period: Duration::from_millis(50),
drift_correction_us: 0,
generated_at: std::time::Instant::now(),
};
let correct_key: [u8; 16] = DEFAULT_TEST_KEY;
let wrong_key: [u8; 16] = [0xFF; 16];
let nonce = 1u32;
let mut msg = [0u8; 20];
msg[..16].copy_from_slice(&beacon.to_bytes());
msg[16..20].copy_from_slice(&nonce.to_le_bytes());
let tag = AuthenticatedBeacon::compute_tag(&msg, &correct_key);
let auth = AuthenticatedBeacon { beacon, nonce, hmac_tag: tag };
assert!(auth.verify(&wrong_key).is_err(), "Wrong key must fail verification");
}
#[test]
fn test_single_bit_flip_in_payload_fails_verification() {
let beacon = SyncBeacon {
cycle_id: 42,
cycle_period: Duration::from_millis(50),
drift_correction_us: 0,
generated_at: std::time::Instant::now(),
};
let key: [u8; 16] = DEFAULT_TEST_KEY;
let nonce = 1u32;
let mut msg = [0u8; 20];
msg[..16].copy_from_slice(&beacon.to_bytes());
msg[16..20].copy_from_slice(&nonce.to_le_bytes());
let tag = AuthenticatedBeacon::compute_tag(&msg, &key);
let auth = AuthenticatedBeacon { beacon, nonce, hmac_tag: tag };
let mut wire = auth.to_bytes();
// Flip one bit in the beacon payload
wire[0] ^= 0x01;
let tampered = AuthenticatedBeacon::from_bytes(&wire).unwrap();
assert!(tampered.verify(&key).is_err(), "Single bit flip must fail verification");
}
#[test]
fn test_enforcing_mode_rejects_unauthenticated() {
let mut cfg = manual_config();
cfg.sec_level = SecLevel::Enforcing;
let mut coord = SecureTdmCoordinator::new(test_schedule(), cfg).unwrap();
// Raw 16-byte beacon without HMAC
let raw = SyncBeacon {
cycle_id: 1,
cycle_period: Duration::from_millis(50),
drift_correction_us: 0,
generated_at: std::time::Instant::now(),
}.to_bytes();
assert!(coord.verify_beacon(&raw).is_err());
}
// ---- Error display tests ----
#[test]
@@ -77,6 +77,10 @@ struct Args {
#[arg(long, default_value = "100")]
tick_ms: u64,
/// Bind address (default 127.0.0.1; set to 0.0.0.0 for network access)
#[arg(long, default_value = "127.0.0.1", env = "SENSING_BIND_ADDR")]
bind_addr: String,
/// Data source: auto, wifi, esp32, simulate
#[arg(long, default_value = "auto")]
source: String,
@@ -2112,7 +2116,15 @@ async fn delete_model(
State(state): State<SharedState>,
Path(id): Path<String>,
) -> Json<serde_json::Value> {
let path = PathBuf::from("data/models").join(format!("{}.rvf", id));
// ADR-050: Sanitize path to prevent directory traversal
let safe_id = std::path::Path::new(&id)
.file_name()
.and_then(|f| f.to_str())
.unwrap_or("");
if safe_id.is_empty() || safe_id != id {
return Json(serde_json::json!({ "error": "invalid model id", "success": false }));
}
let path = PathBuf::from("data/models").join(format!("{}.rvf", safe_id));
if path.exists() {
if let Err(e) = std::fs::remove_file(&path) {
warn!("Failed to delete model file {:?}: {}", path, e);
@@ -2363,7 +2375,15 @@ async fn delete_recording(
State(state): State<SharedState>,
Path(id): Path<String>,
) -> Json<serde_json::Value> {
let path = PathBuf::from("data/recordings").join(format!("{}.jsonl", id));
// ADR-050: Sanitize path to prevent directory traversal
let safe_id = std::path::Path::new(&id)
.file_name()
.and_then(|f| f.to_str())
.unwrap_or("");
if safe_id.is_empty() || safe_id != id {
return Json(serde_json::json!({ "error": "invalid recording id", "success": false }));
}
let path = PathBuf::from("data/recordings").join(format!("{}.jsonl", safe_id));
if path.exists() {
if let Err(e) = std::fs::remove_file(&path) {
warn!("Failed to delete recording {:?}: {}", path, e);
@@ -3604,6 +3624,10 @@ async fn main() {
}
}
// ADR-050: Parse bind address once, use for all listeners
let bind_ip: std::net::IpAddr = args.bind_addr.parse()
.expect("Invalid --bind-addr (use 127.0.0.1 or 0.0.0.0)");
// WebSocket server on dedicated port (8765)
let ws_state = state.clone();
let ws_app = Router::new()
@@ -3611,7 +3635,7 @@ async fn main() {
.route("/health", get(health))
.with_state(ws_state);
let ws_addr = SocketAddr::from(([0, 0, 0, 0], args.ws_port));
let ws_addr = SocketAddr::from((bind_ip, args.ws_port));
let ws_listener = tokio::net::TcpListener::bind(ws_addr).await
.expect("Failed to bind WebSocket port");
info!("WebSocket server listening on {ws_addr}");
@@ -3686,7 +3710,7 @@ async fn main() {
))
.with_state(state.clone());
let http_addr = SocketAddr::from(([0, 0, 0, 0], args.http_port));
let http_addr = SocketAddr::from((bind_ip, args.http_port));
let http_listener = tokio::net::TcpListener::bind(http_addr).await
.expect("Failed to bind HTTP port");
info!("HTTP server listening on {http_addr}");