From 4a82094f4787c110a434e8086b3dbfd3dc17cd41 Mon Sep 17 00:00:00 2001 From: keirsalterego Date: Tue, 26 May 2026 14:33:35 +0530 Subject: [PATCH 1/2] security(proxy): refuse non-loopback plaintext bind without ALLOW_INSECURE F7 from the 2026-05-26 CSO review: serving /execute and /audit/export over plain HTTP on a non-loopback address exposes containment commands and tenant audit history in cleartext (signed for integrity, not encrypted). The proxy now refuses to start in that case unless ALLOW_INSECURE=true is set explicitly, matching the safe-by-default posture of DRY_RUN. A warning was not enough to stop a misconfigured deploy from serving cleartext to the internet. Validation: cargo fmt --check + clippy -D warnings clean, 19 tests pass. --- src/main.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index b332e13..8016943 100644 --- a/src/main.rs +++ b/src/main.rs @@ -529,24 +529,28 @@ async fn main() { } (None, None) => { // Plain HTTP. Intended for deployment behind a TLS-terminating - // reverse proxy (Cloudflare Tunnel, Caddy, nginx). NOT for - // direct internet exposure. Warn loudly if we're binding a - // non-loopback address in cleartext without an explicit - // acknowledgement — that exposes signed-but-cleartext containment - // traffic if there is no proxy in front. (Warn, not panic: plain - // HTTP on 0.0.0.0 behind a reverse proxy is a supported setup.) + // reverse proxy (Cloudflare Tunnel, Caddy, nginx). NOT for direct + // internet exposure: /execute carries hostnames + action types and + // /audit/export returns a tenant's full containment history, all in + // cleartext over plain HTTP (signed for integrity, NOT encrypted). + // + // Fail CLOSED on a non-loopback plaintext bind unless the operator + // explicitly acknowledges with ALLOW_INSECURE=true. A warning was + // not enough — a misconfigured deploy would happily serve cleartext + // containment traffic to the internet (CSO finding #7). Insecure + // modes must be opted into, never reached by omission. let allow_insecure = parse_bool_env("ALLOW_INSECURE", false); let is_loopback = bind_addr .parse::() .map(|a| a.ip().is_loopback()) .unwrap_or(false); if !is_loopback && !allow_insecure { - warn!( - addr = %bind_addr, - "binding plain HTTP to a non-loopback address: containment \ - traffic is cleartext unless a TLS-terminating reverse proxy \ - sits in front. Set TLS_CERT_PATH/TLS_KEY_PATH, bind 127.0.0.1, \ - or set ALLOW_INSECURE=true to acknowledge." + panic!( + "refusing to bind plain HTTP to a non-loopback address ({bind_addr}): \ + containment + audit traffic would be cleartext. Set \ + TLS_CERT_PATH/TLS_KEY_PATH for direct TLS, bind 127.0.0.1 behind a \ + TLS-terminating reverse proxy, or set ALLOW_INSECURE=true to \ + acknowledge the risk explicitly." ); } info!(addr = %bind_addr, tls = false, dry_run, "vyrox proxy starting (plain HTTP)"); From 9a105061ea030a7ae146e53ba9c2f5b12f2dfbd1 Mon Sep 17 00:00:00 2001 From: keirsalterego Date: Fri, 29 May 2026 11:39:40 +0530 Subject: [PATCH 2/2] security(proxy): enforce nonce-store hard cap to bound memory claim_or_replay ran only TTL-based eviction (evict_expired) at the cap, so a burst of more than MAX_RECORDS unique request_ids inside the RETENTION window left every record younger than the cutoff, freed nothing, and grew the map without bound (OOM). The MAX_RECORDS comment promised a hard cap that did not exist. evict_to_cap now drops the oldest entries down to 90% of the cap when TTL eviction is insufficient. Keys are collected before removal so no DashMap iteration and write guard overlap. Regression test inserts MAX_RECORDS+1000 unique fresh claims and asserts len() stays <= MAX_RECORDS. --- src/nonce.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/src/nonce.rs b/src/nonce.rs index 549d9c3..660ab2a 100644 --- a/src/nonce.rs +++ b/src/nonce.rs @@ -144,12 +144,21 @@ impl NonceStore { /// /// See [`Outcome`]. pub fn claim_or_replay(&self, request_id: &str) -> Outcome { - // First, run eviction opportunistically. We do this on the read - // path so memory is reclaimed even if no background task runs. - // Eviction is fast (O(N) worst case) but only triggered when the - // table is at or above the soft cap to amortize the cost. + // Run eviction opportunistically on the read path so memory is + // reclaimed even if no background task runs. Only triggered at/above + // the cap to amortize the cost. if self.inner.len() >= MAX_RECORDS { + // First drop anything past its TTL. self.evict_expired(); + // TTL eviction alone does NOT bound memory: an adversary (or a + // genuine storm) sending > MAX_RECORDS unique request_ids inside + // the RETENTION window leaves every record younger than the + // cutoff, so `evict_expired` frees nothing and the map grows + // without bound. Enforce the hard cap by dropping the oldest + // entries. This is the bounded-memory guarantee the cap promises. + if self.inner.len() >= MAX_RECORDS { + self.evict_to_cap(); + } } // `entry().or_insert_with(...)` atomically inserts the InFlight @@ -229,6 +238,36 @@ impl NonceStore { .retain(|_, record| record.created_at.elapsed() < cutoff); } + /// Enforce the hard cap by dropping the oldest entries. + /// + /// Called only when the map is still at/above `MAX_RECORDS` after TTL + /// eviction — i.e. a burst of unique request_ids all younger than + /// `RETENTION_SECONDS`. We bring the map down to 90% of the cap so the + /// O(N log N) sort amortizes over many subsequent claims instead of + /// running on every insert at the boundary. + /// + /// Collect keys first (fully draining the iterator) before removing, so + /// we never hold a `DashMap` iteration guard and a write guard on the + /// same shard at once. + fn evict_to_cap(&self) { + let len = self.inner.len(); + let target = MAX_RECORDS / 10 * 9; // 90% of the cap + let to_remove = len.saturating_sub(target); + if to_remove == 0 { + return; + } + + let mut entries: Vec<(String, Instant)> = self + .inner + .iter() + .map(|e| (e.key().clone(), e.value().created_at)) + .collect(); + entries.sort_by_key(|(_, created)| *created); // oldest first + for (key, _) in entries.into_iter().take(to_remove) { + self.inner.remove(&key); + } + } + /// Test-only helper to inspect map size. #[cfg(test)] pub fn len(&self) -> usize { @@ -312,4 +351,22 @@ mod tests { .count(); assert_eq!(fresh_wins, 1, "exactly one claim must win FreshClaim"); } + + #[test] + fn unique_id_burst_is_bounded_by_hard_cap() { + // A burst of more than MAX_RECORDS unique request_ids, all younger + // than RETENTION_SECONDS, must NOT grow the map without bound. TTL + // eviction frees nothing here (everything is fresh), so the hard-cap + // eviction is what keeps memory bounded. Regression for the OOM gap. + let store = NonceStore::new(); + for i in 0..(MAX_RECORDS + 1_000) { + let _ = store.claim_or_replay(&format!("burst-{i}")); + } + assert!( + store.len() <= MAX_RECORDS, + "nonce store exceeded the hard cap: {} > {}", + store.len(), + MAX_RECORDS + ); + } }