From 2d818b6efdec652bb6fe16ba3df88e95e7a87b4d Mon Sep 17 00:00:00 2001 From: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Date: Tue, 19 May 2026 15:46:31 -0400 Subject: [PATCH] iroh-relay: review fixups from Mari + Max Mari (trust): - Add 64 KiB pre-decode length cap on the bearer token. NIP-98 events are well under a kilobyte; rejecting oversized inputs before allocating the base64 decode buffer prevents an admission request from coercing the relay into multi-megabyte allocations. New const MAX_BEARER_LEN. - New verify_bearer_rejects_oversized_token test. - New verify_bearer_rejects_internal_whitespace test: pins the fact that base64 0.22's general_purpose engines reject mid-token whitespace (no MIME mode), which is what we want. Max (review): - Soften the module-level 'patched-fork hooks' docs so they don't imply the per-client max-lifetime hook is already wired, and add an explicit TODO(patched-iroh-relay) marker at the future insertion site in spawn. - Add SPROUT_IROH_RELAY_BIND_ADDR to Config (iroh_relay_bind_addr: Option) now, so the main.rs wiring follow-up can read it without a separate config churn. Server::spawn owns its own listener, so this is independent of the Sprout HTTP bind_addr. sprout-relay --lib: 206 -> 208 tests pass (+2). workspace clippy -D warnings: clean. workspace cargo fmt --check: clean. Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com> Co-authored-by: Dawn (sprout agent) --- crates/sprout-relay/src/config.rs | 26 ++++++++++ crates/sprout-relay/src/iroh_relay.rs | 69 +++++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 5 deletions(-) diff --git a/crates/sprout-relay/src/config.rs b/crates/sprout-relay/src/config.rs index 0c26188a7..a2cedb07b 100644 --- a/crates/sprout-relay/src/config.rs +++ b/crates/sprout-relay/src/config.rs @@ -133,6 +133,16 @@ pub struct Config { /// `https://relay.example.com/iroh` (a path prefix is supported and /// preserved by the NIP-98 canonicaliser). pub iroh_relay_public_url: Option, + + /// Optional local socket address the embedded iroh-relay binds to. + /// + /// `iroh_relay::server::Server::spawn` owns its own listener, so this is + /// independent of [`Self::bind_addr`] (the Sprout HTTP/WS port). When + /// unset, [`crate::iroh_relay::spawn`] is *not* started by `fn main` — + /// even if `iroh_relay_public_url` is configured, since advertising a + /// URL without a listener would be a deploy footgun. Set via + /// `SPROUT_IROH_RELAY_BIND_ADDR`, e.g. `0.0.0.0:3478`. + pub iroh_relay_bind_addr: Option, } impl Config { @@ -340,6 +350,21 @@ impl Config { .map(|s| s.trim().to_string()) .filter(|s| !s.is_empty()); + // Mesh-LLM iroh-relay local bind address (optional; independent of + // the Sprout HTTP listener since Server::spawn owns its own socket). + let iroh_relay_bind_addr = match std::env::var("SPROUT_IROH_RELAY_BIND_ADDR") + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + { + Some(s) => Some(s.parse::().map_err(|e| { + ConfigError::InvalidValue(format!( + "SPROUT_IROH_RELAY_BIND_ADDR={s:?} is not a valid socket address: {e}" + )) + })?), + None => None, + }; + // Web UI static file serving let web_dir = std::env::var("SPROUT_WEB_DIR") .ok() @@ -400,6 +425,7 @@ impl Config { git_hook_hmac_secret, web_dir, iroh_relay_public_url, + iroh_relay_bind_addr, }) } } diff --git a/crates/sprout-relay/src/iroh_relay.rs b/crates/sprout-relay/src/iroh_relay.rs index f1521df96..49a0a5cd1 100644 --- a/crates/sprout-relay/src/iroh_relay.rs +++ b/crates/sprout-relay/src/iroh_relay.rs @@ -30,13 +30,14 @@ //! Schnorr verify and 1-2 DB reads per connection, which is negligible //! versus the QUIC + model traffic that follows. //! -//! # Patched-fork hooks +//! # Patched-fork hooks (forward-looking) //! //! Upstream iroh-relay rc.0 does not expose a per-client maximum-lifetime -//! hook. A locally-patched fork (`upstream PR C` in the plan) adds one so we -//! can force re-auth every N minutes. We isolate that wiring behind -//! [`cfg(feature = "patched-iroh-relay")`] so the unpatched crate compiles -//! cleanly. +//! hook. The mesh-LLM plan (v6.1, upstream PR C) will add one so we can +//! force re-auth every N minutes. The `patched-iroh-relay` Cargo feature +//! and the `TODO(patched-iroh-relay)` marker in [`spawn`] are reserved +//! insertion points for that wiring — **the hook is not implemented yet**. +//! Unpatched rc.0 stays compile-clean either way. use std::net::SocketAddr; use std::sync::Arc; @@ -60,6 +61,14 @@ pub const IROH_RELAY_PATH: &str = "/relay"; /// HTTP method bound into NIP-98 events for iroh-relay connection auth. const NIP98_METHOD: &str = "GET"; +/// Maximum size of a bearer token (raw, pre-base64-decode) we'll even try to +/// process. A well-formed NIP-98 event JSON is well under a kilobyte; the +/// base64 expansion of that is ~1.4 KiB. We allow up to 64 KiB so generous +/// signers don't trip over `payload` tag hashes etc., but reject anything +/// larger before allocating decode buffers — admission requests must not be +/// able to coerce the relay into multi-megabyte allocations. +const MAX_BEARER_LEN: usize = 64 * 1024; + /// Handle returned by [`spawn`] — dropping it stops the server. pub struct IrohRelayHandle { /// The bound HTTP address (resolved if the caller passed port 0). @@ -107,6 +116,11 @@ pub async fn spawn( let mut relay = RelayConfig::new(bind_addr); relay.access = access; + // TODO(patched-iroh-relay): once upstream PR C lands, set the per-client + // maximum-lifetime hook here (gated on `#[cfg(feature = "patched-iroh-relay")]`) + // so we force re-auth every N minutes. Until then the connection lifetime + // is whatever iroh-relay's defaults are. + let mut cfg = ServerConfig::default(); cfg.relay = Some(relay); @@ -217,6 +231,16 @@ fn verify_bearer( _ => return Err("missing or empty bearer token".to_string()), }; + // Pre-decode length cap (Mari's review note). NIP-98 events are tiny; an + // attacker shouldn't be able to coerce the relay into allocating a + // multi-megabyte decode buffer before any signature check runs. + if token.len() > MAX_BEARER_LEN { + return Err(format!( + "bearer token exceeds {MAX_BEARER_LEN}-byte limit ({} bytes)", + token.len() + )); + } + let json = decode_bearer(token).ok_or_else(|| "bearer token is not valid base64".to_string())?; @@ -308,6 +332,41 @@ mod tests { assert!(matches!(result, Err(ref e) if e.contains("base64"))); } + #[test] + fn verify_bearer_rejects_oversized_token() { + // 64 KiB + 1 byte. Must be rejected by the length cap *before* any + // decode allocation happens, so an attacker can't coerce a giant + // base64 buffer. + let url = canonical(); + let huge = "A".repeat(MAX_BEARER_LEN + 1); + let result = verify_bearer(&url, Some(&huge)); + assert!( + matches!(result, Err(ref e) if e.contains("exceeds")), + "expected length-cap denial, got {result:?}", + ); + } + + #[test] + fn verify_bearer_rejects_internal_whitespace() { + // base64 0.22's `general_purpose` engines reject internal whitespace + // (no MIME mode). A valid token with a space spliced into the middle + // must therefore fail decode, not be silently accepted as if the + // whitespace were ignored. + let keys = Keys::generate(); + let url = canonical(); + let json = signed_event_json(&keys, &url, NIP98_METHOD); + let mut token = bearer(&json); + // Splice a space into the middle of an otherwise valid token. + let mid = token.len() / 2; + token.insert(mid, ' '); + + let result = verify_bearer(&url, Some(&token)); + assert!( + matches!(result, Err(ref e) if e.contains("base64")), + "expected base64 denial on internal whitespace, got {result:?}", + ); + } + #[test] fn verify_bearer_rejects_wrong_method() { // NIP-98 event signed for POST but the iroh-relay handshake is GET.