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<SocketAddr>) 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) <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
tlongwell-block
2026-05-19 15:46:31 -04:00
co-authored by Dawn
parent 3ca6918078
commit 2d818b6efd
2 changed files with 90 additions and 5 deletions
+26
View File
@@ -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<String>,
/// 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<std::net::SocketAddr>,
}
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::<std::net::SocketAddr>().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,
})
}
}
+64 -5
View File
@@ -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.