diff --git a/crates/buzz-auth/src/access.rs b/crates/buzz-auth/src/access.rs index 1fc1a1ca8..392f6c0e8 100644 --- a/crates/buzz-auth/src/access.rs +++ b/crates/buzz-auth/src/access.rs @@ -6,6 +6,7 @@ use std::collections::HashSet; use std::future::Future; +use buzz_core::TenantContext; use nostr::PublicKey; use uuid::Uuid; @@ -17,24 +18,39 @@ use crate::scope::Scope; /// Implemented by the database layer (`buzz-db`) in production. The `buzz-auth` /// crate defines the trait so it can enforce access rules without a direct dependency /// on `buzz-db`. +/// +/// ## Tenant scoping +/// +/// Every method takes `&TenantContext`. Channel UUIDs are not globally unique under +/// multi-tenant — the frozen schema's `channels` PK is `(community_id, id)`, so the +/// same UUID can legitimately exist in two communities. A bare `WHERE id = $1` +/// implementation would be a cross-community existence oracle and could return +/// `true` for a B-community membership when the request bound community is A. +/// Implementations MUST scope every query by `ctx.community()` (S1 cross-community +/// fence at the access layer). pub trait ChannelAccessChecker: Send + Sync { - /// Return the set of channel UUIDs accessible to `pubkey`. + /// Return the set of channel UUIDs in `ctx`'s community accessible to `pubkey`. + /// + /// Channels in other communities, even with the same UUID, MUST NOT appear. fn accessible_channel_ids( &self, + ctx: &TenantContext, pubkey: &PublicKey, ) -> impl Future, AuthError>> + Send; - /// Returns `true` if `pubkey` is a member of `channel_id`. + /// Returns `true` if `pubkey` is a member of `(ctx.community, channel_id)`. /// - /// Default implementation calls [`Self::accessible_channel_ids`] and checks membership. - /// Implementations may override this with a more efficient point-lookup query. + /// Default implementation calls [`Self::accessible_channel_ids`] and checks + /// membership. Implementations may override this with a more efficient + /// scoped point-lookup query. fn can_access( &self, + ctx: &TenantContext, pubkey: &PublicKey, channel_id: Uuid, ) -> impl Future> + Send { async move { - let ids = self.accessible_channel_ids(pubkey).await?; + let ids = self.accessible_channel_ids(ctx, pubkey).await?; Ok(ids.contains(&channel_id)) } } @@ -52,30 +68,32 @@ pub fn require_scope(scopes: &[Scope], required: Scope) -> Result<(), AuthError> } } -/// Verify read access: scope + membership. +/// Verify read access: scope + membership in `ctx`'s community. pub async fn check_read_access( checker: &impl ChannelAccessChecker, + ctx: &TenantContext, pubkey: &PublicKey, channel_id: Uuid, scopes: &[Scope], ) -> Result<(), AuthError> { require_scope(scopes, Scope::MessagesRead)?; - if checker.can_access(pubkey, channel_id).await? { + if checker.can_access(ctx, pubkey, channel_id).await? { Ok(()) } else { Err(AuthError::ChannelAccessDenied) } } -/// Verify write access: scope + membership. +/// Verify write access: scope + membership in `ctx`'s community. pub async fn check_write_access( checker: &impl ChannelAccessChecker, + ctx: &TenantContext, pubkey: &PublicKey, channel_id: Uuid, scopes: &[Scope], ) -> Result<(), AuthError> { require_scope(scopes, Scope::MessagesWrite)?; - if checker.can_access(pubkey, channel_id).await? { + if checker.can_access(ctx, pubkey, channel_id).await? { Ok(()) } else { Err(AuthError::ChannelAccessDenied) @@ -83,9 +101,12 @@ pub async fn check_write_access( } /// In-memory [`ChannelAccessChecker`] for unit tests. +/// +/// Membership is keyed on the full `(community_id, pubkey, channel_id)` tuple +/// so the mock can't accidentally model a non-tenant-scoped checker. #[cfg(any(test, feature = "test-utils"))] pub struct MockAccessChecker { - allowed: HashSet<(String, Uuid)>, + allowed: HashSet<(uuid::Uuid, String, Uuid)>, } #[cfg(any(test, feature = "test-utils"))] @@ -97,9 +118,10 @@ impl MockAccessChecker { } } - /// Grant `pubkey` access to `channel_id`. - pub fn allow(&mut self, pubkey: &PublicKey, channel_id: Uuid) { - self.allowed.insert((pubkey.to_hex(), channel_id)); + /// Grant `pubkey` access to `channel_id` inside `ctx`'s community. + pub fn allow(&mut self, ctx: &TenantContext, pubkey: &PublicKey, channel_id: Uuid) { + self.allowed + .insert((*ctx.community().as_uuid(), pubkey.to_hex(), channel_id)); } } @@ -112,13 +134,18 @@ impl Default for MockAccessChecker { #[cfg(any(test, feature = "test-utils"))] impl ChannelAccessChecker for MockAccessChecker { - async fn accessible_channel_ids(&self, pubkey: &PublicKey) -> Result, AuthError> { + async fn accessible_channel_ids( + &self, + ctx: &TenantContext, + pubkey: &PublicKey, + ) -> Result, AuthError> { + let community = *ctx.community().as_uuid(); let hex = pubkey.to_hex(); Ok(self .allowed .iter() - .filter(|(pk, _)| pk == &hex) - .map(|(_, id)| *id) + .filter(|(c, pk, _)| *c == community && pk == &hex) + .map(|(_, _, id)| *id) .collect()) } } @@ -126,61 +153,99 @@ impl ChannelAccessChecker for MockAccessChecker { #[cfg(test)] mod tests { use super::*; + use buzz_core::CommunityId; use nostr::Keys; + fn fixture_ctx() -> TenantContext { + TenantContext::resolved(CommunityId::from_uuid(Uuid::new_v4()), "test.example") + } + #[tokio::test] async fn mock_checker_allow_and_deny() { + let ctx = fixture_ctx(); let keys = Keys::generate(); let pk = keys.public_key(); let allowed_ch = Uuid::new_v4(); let denied_ch = Uuid::new_v4(); let mut checker = MockAccessChecker::new(); - checker.allow(&pk, allowed_ch); + checker.allow(&ctx, &pk, allowed_ch); - assert!(checker.can_access(&pk, allowed_ch).await.unwrap()); - assert!(!checker.can_access(&pk, denied_ch).await.unwrap()); + assert!(checker.can_access(&ctx, &pk, allowed_ch).await.unwrap()); + assert!(!checker.can_access(&ctx, &pk, denied_ch).await.unwrap()); } #[tokio::test] async fn read_access_denied_by_scope() { + let ctx = fixture_ctx(); let keys = Keys::generate(); let pk = keys.public_key(); let ch = Uuid::new_v4(); let mut checker = MockAccessChecker::new(); - checker.allow(&pk, ch); + checker.allow(&ctx, &pk, ch); assert!(matches!( - check_read_access(&checker, &pk, ch, &[]).await, + check_read_access(&checker, &ctx, &pk, ch, &[]).await, Err(AuthError::InsufficientScope { .. }) )); } #[tokio::test] async fn read_access_denied_by_membership() { + let ctx = fixture_ctx(); let keys = Keys::generate(); let pk = keys.public_key(); let ch = Uuid::new_v4(); let checker = MockAccessChecker::new(); assert!(matches!( - check_read_access(&checker, &pk, ch, &[Scope::MessagesRead]).await, + check_read_access(&checker, &ctx, &pk, ch, &[Scope::MessagesRead]).await, Err(AuthError::ChannelAccessDenied) )); } #[tokio::test] async fn read_access_granted() { + let ctx = fixture_ctx(); let keys = Keys::generate(); let pk = keys.public_key(); let ch = Uuid::new_v4(); let mut checker = MockAccessChecker::new(); - checker.allow(&pk, ch); + checker.allow(&ctx, &pk, ch); - assert!(check_read_access(&checker, &pk, ch, &[Scope::MessagesRead]) + assert!( + check_read_access(&checker, &ctx, &pk, ch, &[Scope::MessagesRead]) + .await + .is_ok() + ); + } + + #[tokio::test] + async fn access_does_not_cross_communities() { + // S1 fence at the access layer: same pubkey, same channel UUID, two + // communities. A grant in A MUST NOT show up under B's TenantContext. + // This bites the existence-oracle direction a bare `WHERE id=$1` + // checker would have left open. + let ctx_a = fixture_ctx(); + let ctx_b = fixture_ctx(); + let keys = Keys::generate(); + let pk = keys.public_key(); + let ch = Uuid::new_v4(); + + let mut checker = MockAccessChecker::new(); + checker.allow(&ctx_a, &pk, ch); + + assert!(checker.can_access(&ctx_a, &pk, ch).await.unwrap()); + assert!( + !checker.can_access(&ctx_b, &pk, ch).await.unwrap(), + "access in community A must NOT leak into community B for same (pubkey, channel_id)" + ); + assert!(checker + .accessible_channel_ids(&ctx_b, &pk) .await - .is_ok()); + .unwrap() + .is_empty()); } } diff --git a/crates/buzz-auth/src/error.rs b/crates/buzz-auth/src/error.rs index 3ac71b58f..7f8131bc3 100644 --- a/crates/buzz-auth/src/error.rs +++ b/crates/buzz-auth/src/error.rs @@ -30,6 +30,12 @@ pub enum AuthError { #[error("NIP-98 HTTP Auth verification failed: {0}")] Nip98Invalid(String), + /// A NIP-98 event with the same id has already been observed within the + /// replay-prevention window. The event itself was structurally valid; the + /// rejection is on freshness, not validity. + #[error("NIP-98 replay: event id already seen within window")] + Nip98Replay, + /// The pubkey in the auth event does not match the expected identity. #[error("pubkey mismatch: event pubkey does not match authenticated identity")] PubkeyMismatch, diff --git a/crates/buzz-auth/src/lib.rs b/crates/buzz-auth/src/lib.rs index fcb39010c..4f971b0ba 100644 --- a/crates/buzz-auth/src/lib.rs +++ b/crates/buzz-auth/src/lib.rs @@ -23,6 +23,8 @@ pub mod error; pub mod nip42; /// NIP-98 HTTP Auth verification (kind:27235). pub mod nip98; +/// NIP-98 replay protection — shared, community-scoped, atomic seen-set. +pub mod nip98_replay; /// Per-connection rate limiting. pub mod rate_limit; /// OAuth scope parsing and enforcement. @@ -32,6 +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, MAX_REPLAY_TTL_SECS, +}; pub use rate_limit::{ ip_rate_limit_key, rate_limit_key, LimitType, RateLimitConfig, RateLimitResult, RateLimiter, }; @@ -40,6 +45,8 @@ pub use scope::{parse_scopes, Scope}; #[cfg(any(test, feature = "test-utils"))] pub use access::MockAccessChecker; #[cfg(any(test, feature = "test-utils"))] +pub use nip98_replay::AlwaysFreshReplayGuard; +#[cfg(any(test, feature = "test-utils"))] pub use rate_limit::AlwaysAllowRateLimiter; /// How the connection was authenticated. diff --git a/crates/buzz-auth/src/nip98.rs b/crates/buzz-auth/src/nip98.rs index 277cbe27c..74ed8c265 100644 --- a/crates/buzz-auth/src/nip98.rs +++ b/crates/buzz-auth/src/nip98.rs @@ -134,17 +134,19 @@ pub fn verify_nip98_event( /// /// - Lowercases scheme and host (already done by the `url` crate). /// - Strips trailing slash from path. -/// - Treats `localhost` and `::1` as equivalent to `127.0.0.1`. +/// +/// **No loopback aliasing.** `localhost`, `::1`, and `127.0.0.1` are three +/// distinct hosts here. Under multi-tenant the `u`-tag host is the row-zero +/// community binding (`docs/multi-tenant-conformance.md`, NIP-98 row): if +/// `verify_nip98_event` collapses them, an event signed for `localhost` +/// would pass against a `127.0.0.1`-resolved community (or vice versa) — +/// a host-binding side door. Tests reconstruct `expected_url` from their +/// own bound host, the same shape production does. fn normalize_url(raw: &str) -> String { let mut parsed = match Url::parse(raw) { Ok(u) => u, Err(_) => return raw.to_lowercase(), }; - if let Some(host) = parsed.host_str() { - if host == "localhost" || host == "::1" { - let _ = parsed.set_host(Some("127.0.0.1")); - } - } let path = parsed.path().trim_end_matches('/').to_string(); parsed.set_path(&path); parsed.to_string() @@ -284,12 +286,32 @@ mod tests { } #[test] - fn localhost_normalized() { + fn loopback_aliases_are_distinct_hosts() { + // Under multi-tenant, the `u`-tag host is the row-zero community + // binding. An event signed for `localhost` MUST NOT pass against an + // expected URL on `127.0.0.1` (or `::1`) — collapsing the three would + // be a host-check side door. Production reconstructs `expected_url` + // from the community-bound host; tests do the same. let keys = Keys::generate(); let localhost_url = "http://localhost:3000/api/tokens"; let loopback_url = "http://127.0.0.1:3000/api/tokens"; let json = make_nip98_event(&keys, localhost_url, TEST_METHOD, None, None); let result = verify_nip98_event(&json, loopback_url, TEST_METHOD, None); - assert!(result.is_ok()); + assert!( + matches!(result, Err(AuthError::Nip98Invalid(_))), + "localhost u-tag must NOT match a 127.0.0.1 expected_url; got {result:?}" + ); + + // Symmetric: signed-for-127.0.0.1 against expected localhost — same answer. + let json2 = make_nip98_event(&keys, loopback_url, TEST_METHOD, None, None); + let result2 = verify_nip98_event(&json2, localhost_url, TEST_METHOD, None); + assert!( + matches!(result2, Err(AuthError::Nip98Invalid(_))), + "127.0.0.1 u-tag must NOT match a localhost expected_url; got {result2:?}" + ); + + // And identity still holds — same host on both sides verifies. + let json3 = make_nip98_event(&keys, loopback_url, TEST_METHOD, None, None); + assert!(verify_nip98_event(&json3, loopback_url, TEST_METHOD, None).is_ok()); } } diff --git a/crates/buzz-auth/src/nip98_replay.rs b/crates/buzz-auth/src/nip98_replay.rs new file mode 100644 index 000000000..0b91065c9 --- /dev/null +++ b/crates/buzz-auth/src/nip98_replay.rs @@ -0,0 +1,223 @@ +//! NIP-98 replay protection — shared, community-scoped, atomic seen-set. +//! +//! NIP-98 verification ([`crate::nip98::verify_nip98_event`]) is structurally +//! complete: it checks signature, kind, timestamp window, URL, method, and +//! optional body hash. It does **not** check whether the same event id has +//! already been used — that requires shared state. With multiple relay pods +//! ("any pod, any connection" per the rewrite §4 architecture), an in-process +//! cache (moka, DashMap) does not carry the freshness proof across pods, so +//! replay protection is a §5 hard gate. +//! +//! The required shape (§5): +//! +//! - shared state (Redis), atomic set-if-absent, TTL ≥ 120s +//! - community-scoped key — see [`nip98_replay_key`] +//! +//! ## Usage shape +//! +//! Verify first, then mark. Burning a seen-set slot on a forgery would let an +//! attacker who knows a future event id of a victim DoS the legitimate event. +//! +//! ```ignore +//! let pubkey = buzz_auth::verify_nip98_event(json, url, method, body)?; +//! if !replay.try_mark(&ctx, &event_id).await? { +//! return Err(AuthError::Nip98Replay); +//! } +//! // safe to honor the request as `pubkey` +//! ``` +//! +//! The TTL must cover the verifier's clock-skew tolerance (currently ±60s, so +//! the window over which a duplicate event id is even plausible is 2×60 = 120s). +//! [`DEFAULT_REPLAY_TTL_SECS`] is the floor; deployments may raise it. + +use buzz_core::TenantContext; +use nostr::EventId; + +use crate::error::AuthError; + +/// Floor for the replay-prevention window, in seconds. +/// +/// Matches the §5 gate ("TTL ≥ 120s") and the doubled NIP-98 timestamp +/// tolerance (±60s window → 120s span). Implementations MAY use a larger TTL +/// 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`). +/// A test impl is provided behind `cfg(any(test, feature = "test-utils"))`. +pub trait Nip98ReplayGuard: Send + Sync { + /// Atomically claim `event_id` for `ctx`'s community. + /// + /// Returns `Ok(true)` when the id is newly inserted (proceed) and + /// `Ok(false)` when an entry already exists (the caller MUST reject the + /// request as replay). + /// + /// On `Err` (Redis unreachable, etc.) callers MUST fail closed — reject + /// the request rather than admitting it. The shared seen-set is a + /// correctness fence; degrading to "best effort, allow on error" forfeits + /// the freshness proof. + /// + /// Implementations MUST use an atomic set-if-absent operation; a + /// read-then-write sequence loses to concurrent inserts and forfeits the + /// freshness proof. + /// + /// `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, + event_id: &EventId, + ttl_secs: u64, + ) -> impl std::future::Future> + Send; +} + +/// Redis key for a NIP-98 replay marker: +/// `buzz:{community}:nip98:{event_id_hex}`. +/// +/// The community prefix is the S1 isolation fence at the replay layer. +/// Event ids are content-addressed (SHA-256 of the canonical event tuple) so +/// natural cross-community collision is zero, but the gate is fail-closed +/// isolation: a same-id replay across communities must consult two distinct +/// seen-set rows, not one shared row. +pub fn nip98_replay_key(ctx: &TenantContext, event_id: &EventId) -> String { + format!("buzz:{}:nip98:{}", ctx.community(), event_id.to_hex()) +} + +/// Always-fresh seen-set for unit tests — every `try_mark` returns `Ok(true)`. +/// +/// Use only in test code that does not exercise the replay path itself. +#[cfg(any(test, feature = "test-utils"))] +pub struct AlwaysFreshReplayGuard; + +#[cfg(any(test, feature = "test-utils"))] +impl Nip98ReplayGuard for AlwaysFreshReplayGuard { + async fn try_mark( + &self, + _ctx: &TenantContext, + _event_id: &EventId, + _ttl_secs: u64, + ) -> Result { + Ok(true) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use buzz_core::CommunityId; + use nostr::{EventBuilder, Keys, Kind}; + use sha2::{Digest, Sha256}; + use uuid::Uuid; + + fn fixture_ctx(host: &str) -> TenantContext { + let bytes = Sha256::digest(host.as_bytes()); + let mut uuid_bytes = [0u8; 16]; + uuid_bytes.copy_from_slice(&bytes[..16]); + let id = CommunityId::from_uuid(Uuid::from_bytes(uuid_bytes)); + TenantContext::resolved(id, host) + } + + fn fixture_event_id() -> EventId { + let keys = Keys::generate(); + EventBuilder::new(Kind::HttpAuth, "") + .sign_with_keys(&keys) + .expect("sign") + .id + } + + #[test] + fn key_includes_community_prefix() { + let ctx = fixture_ctx("relay-a.example"); + let eid = fixture_event_id(); + let key = nip98_replay_key(&ctx, &eid); + let expected_prefix = format!("buzz:{}:nip98:", ctx.community()); + assert!( + key.starts_with(&expected_prefix), + "key {key} should start with {expected_prefix}" + ); + assert!(key.ends_with(&eid.to_hex())); + } + + #[test] + fn key_isolates_communities_for_same_event_id() { + // Belt-and-suspenders: even if a same-id event surfaces in two + // communities (which content-addressing makes implausible), the + // seen-set MUST consult two distinct rows. + let eid = fixture_event_id(); + let ctx_a = fixture_ctx("relay-a.example"); + let ctx_b = fixture_ctx("relay-b.example"); + let key_a = nip98_replay_key(&ctx_a, &eid); + let key_b = nip98_replay_key(&ctx_b, &eid); + assert_ne!( + key_a, key_b, + "same event id in two communities must not share a seen-set key" + ); + } + + #[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; + let ctx = fixture_ctx("relay-a.example"); + let eid = fixture_event_id(); + assert!(guard + .try_mark(&ctx, &eid, DEFAULT_REPLAY_TTL_SECS) + .await + .unwrap()); + } +} diff --git a/crates/buzz-auth/src/rate_limit.rs b/crates/buzz-auth/src/rate_limit.rs index a77da4fa7..8fd42c50f 100644 --- a/crates/buzz-auth/src/rate_limit.rs +++ b/crates/buzz-auth/src/rate_limit.rs @@ -8,6 +8,7 @@ use std::net::IpAddr; +use buzz_core::TenantContext; use nostr::PublicKey; use serde::{Deserialize, Serialize}; @@ -147,14 +148,32 @@ impl Default for RateLimitConfig { /// The Redis-backed production implementation lives in `buzz-relay` / `buzz-pubsub`. /// A no-op `AlwaysAllowRateLimiter` is provided for unit tests. /// +/// ## Tenant scoping +/// +/// Pubkey-keyed limits ([`check_and_increment`]) take `&TenantContext` and the Redis +/// key is community-prefixed (`buzz:{community}:ratelimit:{pubkey}:{suffix}`). The +/// same pubkey active in two communities consumes two independent quotas — that is +/// the correct behavior under multi-tenant isolation (S1 cross-community fence). +/// +/// IP-keyed limits ([`check_ip_connection`]) are **operator-global** by design. They +/// gate connection acceptance at the network edge, before host→community resolution +/// has completed (or, on resolve failure, instead of it). Threading `&TenantContext` +/// through the connection-rate fence would invert the order of operations. If +/// per-(community, IP) caps are ever needed as a tenant-fairness signal, that +/// belongs in an additive `LimitType` keyed on `(community, ip)`, not in this trait. +/// /// ⚠️ The fixed-window algorithm used by the Redis implementation allows up to 2× /// burst at window boundaries. Upgrade to a sliding window or token bucket if strict /// per-second limiting is required. pub trait RateLimiter: Send + Sync { - /// Increment the counter for `pubkey` + `limit_type` and return whether the - /// request is within the configured `limit` for the given `window_secs`. + /// Increment the per-(community, pubkey) counter for `limit_type` and return + /// whether the request is within `limit` for the given `window_secs`. + /// + /// `ctx` scopes the counter to the resolved community; the same pubkey in two + /// communities is two independent quotas. fn check_and_increment( &self, + ctx: &TenantContext, pubkey: &PublicKey, limit_type: LimitType, window_secs: u64, @@ -162,7 +181,10 @@ pub trait RateLimiter: Send + Sync { ) -> impl std::future::Future> + Send; /// Increment the per-IP connection counter and return whether the connection - /// is within the configured `limit` for the given `window_secs`. + /// is within `limit` for the given `window_secs`. + /// + /// Operator-global — see trait docs. This fence runs before / outside of host + /// resolution and intentionally does not take a `TenantContext`. fn check_ip_connection( &self, ip: &IpAddr, @@ -171,16 +193,23 @@ pub trait RateLimiter: Send + Sync { ) -> impl std::future::Future> + Send; } -/// Redis key for pubkey-based rate limit: `buzz:ratelimit::` -pub fn rate_limit_key(pubkey: &PublicKey, limit_type: &LimitType) -> String { +/// Redis key for pubkey-based rate limit: +/// `buzz:{community}:ratelimit:{pubkey_hex}:{suffix}`. +/// +/// Community-prefixed: the same pubkey in two communities maps to two distinct +/// keys, so quotas don't bleed across the tenancy fence. +pub fn rate_limit_key(ctx: &TenantContext, pubkey: &PublicKey, limit_type: &LimitType) -> String { format!( - "buzz:ratelimit:{}:{}", + "buzz:{}:ratelimit:{}:{}", + ctx.community(), pubkey.to_hex(), limit_type.key_suffix() ) } -/// Redis key for IP-based rate limit: `buzz:ratelimit:ip::conn` +/// Redis key for IP-based rate limit: `buzz:ratelimit:ip:{ip}:conn`. +/// +/// Operator-global by design — see [`RateLimiter`] docs. pub fn ip_rate_limit_key(ip: &IpAddr) -> String { format!("buzz:ratelimit:ip:{}:conn", ip) } @@ -193,6 +222,7 @@ pub struct AlwaysAllowRateLimiter; impl RateLimiter for AlwaysAllowRateLimiter { async fn check_and_increment( &self, + _ctx: &TenantContext, _pubkey: &PublicKey, _limit_type: LimitType, window_secs: u64, @@ -214,19 +244,70 @@ impl RateLimiter for AlwaysAllowRateLimiter { #[cfg(test)] mod tests { use super::*; + use buzz_core::CommunityId; use nostr::Keys; + use sha2::Digest; use std::net::Ipv4Addr; + use uuid::Uuid; + + fn fixture_ctx(host: &str) -> TenantContext { + // Deterministic community id from host so test assertions can name the prefix. + let bytes = sha2::Sha256::digest(host.as_bytes()); + let mut uuid_bytes = [0u8; 16]; + uuid_bytes.copy_from_slice(&bytes[..16]); + let id = CommunityId::from_uuid(Uuid::from_bytes(uuid_bytes)); + TenantContext::resolved(id, host) + } #[test] - fn rate_limit_key_format() { + fn rate_limit_key_includes_community_prefix() { + let ctx = fixture_ctx("relay-a.example"); let keys = Keys::generate(); - let key = rate_limit_key(&keys.public_key(), &LimitType::Messages); - assert!(key.starts_with("buzz:ratelimit:")); + let key = rate_limit_key(&ctx, &keys.public_key(), &LimitType::Messages); + let expected_prefix = format!("buzz:{}:ratelimit:", ctx.community()); + assert!( + key.starts_with(&expected_prefix), + "key {key} should start with {expected_prefix}" + ); assert!(key.ends_with(":msg")); } + #[test] + fn rate_limit_key_isolates_communities_for_same_pubkey() { + // The S1 cross-community isolation fence at the rate-limit key layer: + // same pubkey, two communities -> two distinct Redis keys -> independent quotas. + let keys = Keys::generate(); + let ctx_a = fixture_ctx("relay-a.example"); + let ctx_b = fixture_ctx("relay-b.example"); + let key_a = rate_limit_key(&ctx_a, &keys.public_key(), &LimitType::Messages); + let key_b = rate_limit_key(&ctx_b, &keys.public_key(), &LimitType::Messages); + assert_ne!( + key_a, key_b, + "same pubkey in two communities must not share a rate-limit key" + ); + } + + #[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. let ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 1)); assert_eq!(ip_rate_limit_key(&ip), "buzz:ratelimit:ip:192.168.1.1:conn"); } @@ -234,9 +315,10 @@ mod tests { #[tokio::test] async fn always_allow_limiter() { let limiter = AlwaysAllowRateLimiter; + let ctx = fixture_ctx("relay-a.example"); let keys = Keys::generate(); let result = limiter - .check_and_increment(&keys.public_key(), LimitType::Messages, 60, 60) + .check_and_increment(&ctx, &keys.public_key(), LimitType::Messages, 60, 60) .await .unwrap(); assert!(result.allowed); diff --git a/crates/buzz-pubsub/src/lib.rs b/crates/buzz-pubsub/src/lib.rs index d33bd87e6..de6618a87 100644 --- a/crates/buzz-pubsub/src/lib.rs +++ b/crates/buzz-pubsub/src/lib.rs @@ -25,6 +25,8 @@ pub mod cache_invalidation; /// Error types for pub/sub operations. pub mod error; +/// Redis-backed NIP-98 replay seen-set. +pub mod nip98_replay; /// Online/offline presence tracking in Redis. pub mod presence; /// Redis PUBLISH for channel event fan-out. diff --git a/crates/buzz-pubsub/src/nip98_replay.rs b/crates/buzz-pubsub/src/nip98_replay.rs new file mode 100644 index 000000000..264e935d5 --- /dev/null +++ b/crates/buzz-pubsub/src/nip98_replay.rs @@ -0,0 +1,202 @@ +//! Redis-backed NIP-98 replay seen-set. +//! +//! Implements the [`Nip98ReplayGuard`] trait from `buzz-auth`. Uses Redis +//! `SET NX EX` for an atomic set-if-absent with TTL — the §5 pre-build gate +//! for multi-tenant HA replay protection. + +use buzz_auth::{ + error::AuthError, + nip98_replay::{ + nip98_replay_key, Nip98ReplayGuard, DEFAULT_REPLAY_TTL_SECS, MAX_REPLAY_TTL_SECS, + }, +}; +use buzz_core::TenantContext; +use nostr::EventId; + +/// Redis-backed NIP-98 replay seen-set. +/// +/// Each `try_mark(ctx, event_id, ttl)` issues a single +/// `SET buzz:{community}:nip98:{event_id_hex} 1 NX EX ` against Redis. +/// `NX` makes the operation atomic set-if-absent — the freshness proof comes +/// from Redis returning `OK` only on the first claim. Subsequent claims within +/// the TTL window return `nil`, which we surface as `Ok(false)` so the caller +/// rejects the request as replay. +pub struct RedisNip98ReplayGuard { + pool: deadpool_redis::Pool, +} + +impl RedisNip98ReplayGuard { + /// Create a new replay guard backed by the given Redis connection pool. + pub fn new(pool: deadpool_redis::Pool) -> Self { + Self { pool } + } +} + +impl Nip98ReplayGuard for RedisNip98ReplayGuard { + async fn try_mark( + &self, + ctx: &TenantContext, + event_id: &EventId, + ttl_secs: u64, + ) -> Result { + // §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| { + // 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); + + // SET key 1 NX EX . redis-rs typed return: Some("OK") on first + // claim, None on existing key. Any other value would be a Redis-side + // bug; treat it as internal error. + let result: Option = redis::cmd("SET") + .arg(&key) + .arg("1") + .arg("NX") + .arg("EX") + .arg(ttl) + .query_async(&mut *conn) + .await + .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) => { + 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}" + ))) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use buzz_core::{CommunityId, TenantContext}; + use deadpool_redis::{Config, Runtime}; + use nostr::{EventBuilder, Keys, Kind}; + use uuid::Uuid; + + fn redis_pool() -> deadpool_redis::Pool { + let url = std::env::var("REDIS_URL").unwrap_or_else(|_| "redis://127.0.0.1:6379".into()); + Config::from_url(url) + .create_pool(Some(Runtime::Tokio1)) + .expect("create pool") + } + + fn fresh_ctx() -> TenantContext { + TenantContext::resolved(CommunityId::from_uuid(Uuid::new_v4()), "test.example") + } + + fn fresh_event_id() -> EventId { + EventBuilder::new(Kind::HttpAuth, "") + .sign_with_keys(&Keys::generate()) + .expect("sign") + .id + } + + #[tokio::test] + #[ignore = "requires Redis"] + async fn first_claim_succeeds_replay_fails() { + let guard = RedisNip98ReplayGuard::new(redis_pool()); + let ctx = fresh_ctx(); + let eid = fresh_event_id(); + + assert!(guard + .try_mark(&ctx, &eid, DEFAULT_REPLAY_TTL_SECS) + .await + .expect("first mark")); + assert!(!guard + .try_mark(&ctx, &eid, DEFAULT_REPLAY_TTL_SECS) + .await + .expect("replay mark")); + } + + #[tokio::test] + #[ignore = "requires Redis"] + async fn isolation_between_communities() { + let guard = RedisNip98ReplayGuard::new(redis_pool()); + let ctx_a = fresh_ctx(); + let ctx_b = fresh_ctx(); + let eid = fresh_event_id(); + + assert!(guard + .try_mark(&ctx_a, &eid, DEFAULT_REPLAY_TTL_SECS) + .await + .expect("mark in A")); + // Same event id under ctx_b is still a first claim — communities are + // independent seen-sets. + assert!(guard + .try_mark(&ctx_b, &eid, DEFAULT_REPLAY_TTL_SECS) + .await + .expect("mark in B")); + } + + #[tokio::test] + #[ignore = "requires Redis"] + async fn sub_floor_ttl_is_lifted_to_default() { + let guard = RedisNip98ReplayGuard::new(redis_pool()); + let ctx = fresh_ctx(); + let eid = fresh_event_id(); + + // Caller asks for 30s; impl lifts to ≥ DEFAULT_REPLAY_TTL_SECS. + // Smoke-test the path: pass sub-floor TTL, claim once, then expect + // replay rejection — the TTL lift kept the marker alive past 30s and + // the contract holds. + 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")); + } +} diff --git a/crates/buzz-pubsub/src/rate_limiter.rs b/crates/buzz-pubsub/src/rate_limiter.rs index e466893aa..8d5494f1d 100644 --- a/crates/buzz-pubsub/src/rate_limiter.rs +++ b/crates/buzz-pubsub/src/rate_limiter.rs @@ -13,6 +13,7 @@ use buzz_auth::{ error::AuthError, rate_limit::{LimitType, RateLimitResult, RateLimiter}, }; +use buzz_core::TenantContext; use nostr::PublicKey; use redis::Script; @@ -79,9 +80,11 @@ async fn run_rate_limit( /// Redis-backed rate limiter using fixed-window counters. /// -/// Each key is `buzz:ratelimit::` (pubkey) or -/// `buzz:ratelimit:ip::conn` (IP). The counter and its TTL are managed -/// atomically via a Lua script to prevent keys from persisting without expiry. +/// Pubkey keys are community-scoped via `&TenantContext`: +/// `buzz:{community}:ratelimit:{pubkey_hex}:{suffix}`. IP keys remain +/// operator-global: `buzz:ratelimit:ip:{ip}:conn`. The counter and its TTL are +/// managed atomically via a Lua script to prevent keys from persisting without +/// expiry. pub struct RedisRateLimiter { pool: deadpool_redis::Pool, } @@ -96,12 +99,13 @@ impl RedisRateLimiter { impl RateLimiter for RedisRateLimiter { async fn check_and_increment( &self, + ctx: &TenantContext, pubkey: &PublicKey, limit_type: LimitType, window_secs: u64, limit: u64, ) -> Result { - let key = buzz_auth::rate_limit::rate_limit_key(pubkey, &limit_type); + let key = buzz_auth::rate_limit::rate_limit_key(ctx, pubkey, &limit_type); run_rate_limit(&self.pool, &key, window_secs, limit).await }