diff --git a/crates/buzz-auth/src/lib.rs b/crates/buzz-auth/src/lib.rs index 4e801e20c..4f971b0ba 100644 --- a/crates/buzz-auth/src/lib.rs +++ b/crates/buzz-auth/src/lib.rs @@ -34,7 +34,9 @@ pub use access::{check_read_access, check_write_access, require_scope, ChannelAc pub use error::AuthError; pub use nip42::{generate_challenge, verify_nip42_event}; pub use nip98::verify_nip98_event; -pub use nip98_replay::{nip98_replay_key, Nip98ReplayGuard, DEFAULT_REPLAY_TTL_SECS}; +pub use nip98_replay::{ + nip98_replay_key, Nip98ReplayGuard, DEFAULT_REPLAY_TTL_SECS, MAX_REPLAY_TTL_SECS, +}; pub use rate_limit::{ ip_rate_limit_key, rate_limit_key, LimitType, RateLimitConfig, RateLimitResult, RateLimiter, }; diff --git a/crates/buzz-auth/src/nip98_replay.rs b/crates/buzz-auth/src/nip98_replay.rs index 429d380e1..0b91065c9 100644 --- a/crates/buzz-auth/src/nip98_replay.rs +++ b/crates/buzz-auth/src/nip98_replay.rs @@ -42,6 +42,18 @@ use crate::error::AuthError; /// for safety margin; they MUST NOT use a smaller one. pub const DEFAULT_REPLAY_TTL_SECS: u64 = 120; +/// Ceiling for the replay-prevention window, in seconds. +/// +/// Any TTL beyond an hour is implausible for NIP-98 replay protection: the +/// verifier only accepts events within ±60s, so a same-id replay is only +/// physically possible inside that window plus clock skew. A 1-hour cap is +/// 30× the natural maximum and still keeps Redis values well inside +/// `i64::MAX` seconds (which Redis `EX` requires). Anything larger reaching +/// this code is a config/caller bug; implementations MUST clamp down to it +/// rather than admit values that risk Redis `EX` parse failures or +/// pathologically long-lived seen-set entries. +pub const MAX_REPLAY_TTL_SECS: u64 = 3600; + /// Shared seen-set for NIP-98 event ids, scoped per community. /// /// The production implementation lives in `buzz-pubsub` (Redis `SET NX EX`). @@ -65,6 +77,11 @@ pub trait Nip98ReplayGuard: Send + Sync { /// `ttl_secs` MUST be at least [`DEFAULT_REPLAY_TTL_SECS`]. Implementations /// MAY clamp a smaller value up to the floor rather than reject; they MUST /// NOT honor it as-given. + /// + /// `ttl_secs` MUST be clamped down to [`MAX_REPLAY_TTL_SECS`] if larger. + /// The replay window's natural maximum is the verifier's ±60s tolerance; + /// values past an hour are implausible and risk Redis `EX` parse failures + /// (Redis interprets `EX` as a signed 64-bit integer). fn try_mark( &self, ctx: &TenantContext, @@ -156,12 +173,43 @@ mod tests { ); } + #[test] + fn key_components_are_lowercase() { + // Stability/idempotence: if event id hex or community Display ever + // started emitting uppercase, a same logical claim would produce two + // distinct Redis rows → the seen-set would no longer be a seen-set. + let ctx = fixture_ctx("relay-a.example"); + let eid = fixture_event_id(); + let key = nip98_replay_key(&ctx, &eid); + for c in key.chars() { + assert!( + !c.is_ascii_uppercase(), + "nip98 replay key {key} must be all-lowercase ASCII" + ); + } + } + #[test] fn default_ttl_meets_gate_floor() { // §5 gate: TTL ≥ 120s. Drift this constant down and the gate breaks. assert!(DEFAULT_REPLAY_TTL_SECS >= 120); } + #[test] + fn ttl_floor_below_ceiling() { + // Sanity: any caller's clamped TTL must end up in [DEFAULT, MAX]. + // If these ever cross, the impl can't satisfy both bounds and the + // contract is broken. + assert!(DEFAULT_REPLAY_TTL_SECS < MAX_REPLAY_TTL_SECS); + } + + #[test] + fn max_ttl_fits_in_redis_signed_ex() { + // Redis `EX` is parsed as i64. `MAX_REPLAY_TTL_SECS` must fit so the + // clamp itself can't push us into a Redis-side parse failure. + assert!(MAX_REPLAY_TTL_SECS <= i64::MAX as u64); + } + #[tokio::test] async fn always_fresh_returns_true() { let guard = AlwaysFreshReplayGuard; diff --git a/crates/buzz-auth/src/rate_limit.rs b/crates/buzz-auth/src/rate_limit.rs index 2c59c7b0c..8fd42c50f 100644 --- a/crates/buzz-auth/src/rate_limit.rs +++ b/crates/buzz-auth/src/rate_limit.rs @@ -287,6 +287,24 @@ mod tests { ); } + #[test] + fn rate_limit_key_components_are_lowercase() { + // Stability/idempotence invariant: if pubkey hex or community Display + // ever started emitting uppercase, the same (community, pubkey) would + // produce two distinct Redis keys → effective 2× quota. Pin the + // lowercase property here so the regression surfaces in unit tests, + // not in production traffic. + let ctx = fixture_ctx("relay-a.example"); + let keys = Keys::generate(); + let key = rate_limit_key(&ctx, &keys.public_key(), &LimitType::Messages); + for c in key.chars() { + assert!( + !c.is_ascii_uppercase(), + "rate-limit key {key} must be all-lowercase ASCII" + ); + } + } + #[test] fn ip_rate_limit_key_format() { // IP fence stays operator-global — no community in the key. diff --git a/crates/buzz-pubsub/src/nip98_replay.rs b/crates/buzz-pubsub/src/nip98_replay.rs index b10817c25..264e935d5 100644 --- a/crates/buzz-pubsub/src/nip98_replay.rs +++ b/crates/buzz-pubsub/src/nip98_replay.rs @@ -6,7 +6,9 @@ use buzz_auth::{ error::AuthError, - nip98_replay::{nip98_replay_key, Nip98ReplayGuard, DEFAULT_REPLAY_TTL_SECS}, + nip98_replay::{ + nip98_replay_key, Nip98ReplayGuard, DEFAULT_REPLAY_TTL_SECS, MAX_REPLAY_TTL_SECS, + }, }; use buzz_core::TenantContext; use nostr::EventId; @@ -37,14 +39,25 @@ impl Nip98ReplayGuard for RedisNip98ReplayGuard { event_id: &EventId, ttl_secs: u64, ) -> Result { - // §5 gate floor — never accept a sub-floor TTL silently. - let ttl = ttl_secs.max(DEFAULT_REPLAY_TTL_SECS); + // §5 gate floor + safety ceiling. Sub-floor values are lifted to the + // floor (contract permits clamping); above-ceiling values are pushed + // down to MAX_REPLAY_TTL_SECS (contract REQUIRES clamping) so a buggy + // caller cannot send a Redis-incompatible `EX` arg or pin a slot for + // implausibly long. + let ttl = ttl_secs + .max(DEFAULT_REPLAY_TTL_SECS) + .min(MAX_REPLAY_TTL_SECS); - let mut conn = self - .pool - .get() - .await - .map_err(|e| AuthError::Internal(format!("Redis pool: {e}")))?; + let mut conn = self.pool.get().await.map_err(|e| { + // Structured field for ops; the user-facing AuthError stays a + // bounded category string. + tracing::warn!( + community = %ctx.community(), + error = %e, + "nip98 replay: redis pool acquire failed — caller MUST fail closed" + ); + AuthError::Internal(format!("Redis pool: {e}")) + })?; let key = nip98_replay_key(ctx, event_id); @@ -59,14 +72,28 @@ impl Nip98ReplayGuard for RedisNip98ReplayGuard { .arg(ttl) .query_async(&mut *conn) .await - .map_err(|e| AuthError::Internal(format!("Redis SET NX EX: {e}")))?; + .map_err(|e| { + tracing::warn!( + community = %ctx.community(), + error = %e, + "nip98 replay: redis SET NX EX failed — caller MUST fail closed" + ); + AuthError::Internal(format!("Redis SET NX EX: {e}")) + })?; match result.as_deref() { Some("OK") => Ok(true), None => Ok(false), - Some(other) => Err(AuthError::Internal(format!( - "unexpected SET NX EX reply: {other}" - ))), + Some(other) => { + tracing::error!( + community = %ctx.community(), + reply = %other, + "nip98 replay: redis SET NX EX returned an unexpected reply — investigate" + ); + Err(AuthError::Internal(format!( + "unexpected SET NX EX reply: {other}" + ))) + } } } } @@ -148,4 +175,28 @@ mod tests { assert!(guard.try_mark(&ctx, &eid, 30).await.expect("mark")); assert!(!guard.try_mark(&ctx, &eid, 30).await.expect("replay")); } + + #[tokio::test] + #[ignore = "requires Redis"] + async fn above_ceiling_ttl_is_clamped() { + let guard = RedisNip98ReplayGuard::new(redis_pool()); + let ctx = fresh_ctx(); + let eid = fresh_event_id(); + + // Caller asks for u64::MAX (well past Redis's i64::MAX `EX` limit). + // Impl MUST clamp down to MAX_REPLAY_TTL_SECS so the SET succeeds; if + // we instead forwarded u64::MAX, Redis would reject the EX arg and + // try_mark would return Err — and per the trait contract, callers + // fail closed on Err. That's correctness-preserving but UX-hostile: + // every nip98 request errors. The clamp turns a foot-gun into a + // contained warning. + assert!(guard + .try_mark(&ctx, &eid, u64::MAX) + .await + .expect("mark with extreme ttl must succeed via clamp")); + assert!(!guard + .try_mark(&ctx, &eid, u64::MAX) + .await + .expect("replay with extreme ttl must succeed via clamp")); + } }