From 6a92f0b7fface356edeaace8bc659c04dc63b8e7 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Fri, 26 Jun 2026 11:53:45 -0400 Subject: [PATCH 1/4] feat(auth): community-scope RateLimiter pubkey quotas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RateLimiter::check_and_increment now takes &TenantContext, and rate_limit_key emits buzz:{community}:ratelimit:{pubkey_hex}:{suffix}. Same pubkey active in two communities consumes two independent quotas, matching the S1 cross-community isolation fence in the buzz-relay rewrite spec. check_ip_connection stays operator-global by design. The IP fence runs at connection acceptance, before host->community resolution has completed (or, on resolve failure, instead of it). Threading &TenantContext through it would invert the order of operations. Per- (community, IP) caps, if ever needed as a tenant-fairness signal, belong in an additive LimitType keyed on (community, ip) — not in this trait. RedisRateLimiter in buzz-pubsub follows the new trait signature. AlwaysAllowRateLimiter test impl mirrors it. Two new tests pin the behavior: the key includes the community prefix, and same-pubkey-two- communities yields two distinct Redis keys. Local cargo test -p buzz-auth: 36 passed. Local cargo test -p buzz-pubsub: 3 passed, 6 Redis-required ignored. Workspace-wide check not run locally (sqlx 0.9.0 requires rustc 1.94, local toolchain is 1.89 — same constraint Max hit on the pubsub lane); relying on CI for the full integration compile. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-auth/src/rate_limit.rs | 86 ++++++++++++++++++++++---- crates/buzz-pubsub/src/rate_limiter.rs | 12 ++-- 2 files changed, 83 insertions(+), 15 deletions(-) diff --git a/crates/buzz-auth/src/rate_limit.rs b/crates/buzz-auth/src/rate_limit.rs index a77da4fa7..2c59c7b0c 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,52 @@ 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 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 +297,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/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 } From a2a9ef4f213bf69c8df06eab601a6e2b6aa3f181 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Fri, 26 Jun 2026 12:00:13 -0400 Subject: [PATCH 2/4] =?UTF-8?q?feat(auth):=20NIP-98=20replay=20seen-set=20?= =?UTF-8?q?=E2=80=94=20shared,=20community-scoped,=20atomic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the §5 pre-build gate for multi-tenant replay protection. buzz-auth gains a Nip98ReplayGuard trait plus the nip98_replay_key(ctx, event_id) helper. The trait's try_mark contract requires atomic set-if-absent semantics; an in-process cache (moka, DashMap) does not carry the freshness proof across pods under the "any pod, any connection" architecture (§4B), so the production implementation MUST be shared state. The Redis-backed impl lives in buzz-pubsub as RedisNip98ReplayGuard and uses a single SET key 1 NX EX per claim. Key shape: buzz:{community}:nip98:{event_id_hex}. Event ids are content-addressed 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. Tests pin both the prefix and the cross-community isolation guarantee. TTL floor is DEFAULT_REPLAY_TTL_SECS = 120, matching the §5 gate requirement and the doubled NIP-98 ±60s timestamp tolerance. Implementations MAY clamp sub-floor TTLs up to the floor; they MUST NOT honor smaller values. The Redis impl clamps. Caller contract documented in the trait: verify first, then mark. Burning a seen-set slot on a forgery would let an attacker who learns a future event id DoS the legitimate event. On Err (Redis unreachable) callers MUST fail closed. Not wired into a call site in this commit — there is no NIP-98 HTTP handler in Lane 0 yet. Eva's relay-wiring lane will consume the trait when the HTTP path lands; the contract is documented for that integration. Validation: - cargo test -p buzz-auth --lib ✅ 40 passed (4 new in nip98_replay). - cargo test -p buzz-pubsub --lib ✅ 3 passed, 9 Redis-required ignored (3 new in nip98_replay). - cargo test -p buzz-pubsub --lib nip98_replay -- --ignored against local Redis ✅ 3 passed: first-claim/replay, cross-community isolation, sub-floor TTL lifted to floor. - Workspace check not run locally (sqlx 0.9.0 / rustc 1.94 vs local 1.89); CI catches it. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-auth/src/error.rs | 6 + crates/buzz-auth/src/lib.rs | 5 + crates/buzz-auth/src/nip98_replay.rs | 175 +++++++++++++++++++++++++ crates/buzz-pubsub/src/lib.rs | 2 + crates/buzz-pubsub/src/nip98_replay.rs | 151 +++++++++++++++++++++ 5 files changed, 339 insertions(+) create mode 100644 crates/buzz-auth/src/nip98_replay.rs create mode 100644 crates/buzz-pubsub/src/nip98_replay.rs 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..4e801e20c 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,7 @@ 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 rate_limit::{ ip_rate_limit_key, rate_limit_key, LimitType, RateLimitConfig, RateLimitResult, RateLimiter, }; @@ -40,6 +43,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_replay.rs b/crates/buzz-auth/src/nip98_replay.rs new file mode 100644 index 000000000..429d380e1 --- /dev/null +++ b/crates/buzz-auth/src/nip98_replay.rs @@ -0,0 +1,175 @@ +//! 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; + +/// 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. + 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 default_ttl_meets_gate_floor() { + // §5 gate: TTL ≥ 120s. Drift this constant down and the gate breaks. + assert!(DEFAULT_REPLAY_TTL_SECS >= 120); + } + + #[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-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..b10817c25 --- /dev/null +++ b/crates/buzz-pubsub/src/nip98_replay.rs @@ -0,0 +1,151 @@ +//! 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}, +}; +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 — never accept a sub-floor TTL silently. + let ttl = ttl_secs.max(DEFAULT_REPLAY_TTL_SECS); + + let mut conn = self + .pool + .get() + .await + .map_err(|e| 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| 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}" + ))), + } + } +} + +#[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")); + } +} From f54d728e2529501869c709ace55cb395f5fec339 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Fri, 26 Jun 2026 12:07:21 -0400 Subject: [PATCH 3/4] hardening(auth): TTL ceiling, key-case invariant, structured error tracing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Red-team pass against the auth lane surfaced one real bug and two robustness gaps. All three caught by tests, the bug verified by temporarily reverting the fix and watching the test fail with the real Redis error. 1. Real bug: a caller passing ttl_secs > i64::MAX (e.g. u64::MAX from a config bug) caused Redis to return "ResponseError: value is not an integer or out of range" from `SET NX EX `. RedisNip98ReplayGuard then returned Err, the trait contract forces callers to fail closed, and every NIP-98-gated request from that point would have errored with no visible link back to the bad config. Fix: introduce MAX_REPLAY_TTL_SECS (1 hour — 30× the natural physical maximum, well inside i64::MAX) and clamp ttl_secs into [DEFAULT, MAX] before the SET. New ignored-Redis test `above_ceiling_ttl_is_clamped` exercises the path with u64::MAX and asserts the claim+replay sequence succeeds, which it only does with the clamp. 2. Robustness: pin "all rate-limit and replay key components are lowercase ASCII" as a unit-level invariant. If pubkey::to_hex, Uuid::Display, or LimitType::key_suffix ever started emitting uppercase, the same logical (community, pubkey/event_id) would map to two distinct Redis keys — silently doubling the rate-limit quota or breaking the seen-set's identity. Two new tests (`rate_limit_key_components_are_lowercase`, `key_components_are_lowercase`) catch the regression in CI rather than production. 3. Robustness: structured tracing on every Redis failure path with `community = %ctx.community()` as a structured field, so ops can group log alerts by tenant without needing the community id to be embedded in the AuthError string. The user-facing AuthError::Internal payload stays the existing convention (consistent with rate_limit.rs neighbors); the per-tenant context lives in tracing fields, not in the error string. Also: add `ttl_floor_below_ceiling` and `max_ttl_fits_in_redis_signed_ex` unit tests so the two TTL constants can't drift past each other or above Redis's signed-EX limit in a future edit. Out of scope for this lane (flagged to other lane owners): - AuthError::Internal generally embeds raw downstream error strings (existing pattern across rate_limit.rs and nip98_replay.rs). Could leak community/tenant identifiers if those strings ever surface to clients. Audit lane (Quinn) owns the error-message safety rule per Eva's [6] lane split. - check_ip_connection MUST be called before host resolution / on every connection (including failed-host-resolution attempts). Otherwise an attacker who picks a non-matching host header bypasses the IP cap. Wiring lives in the relay-wiring lane (Eva). Validation: - cargo test -p buzz-auth --lib: 44 passed (4 new red-team tests). - cargo test -p buzz-pubsub --lib: 3 passed, 10 Redis-required ignored. - cargo test -p buzz-pubsub --lib nip98_replay -- --ignored against local Redis: 4 passed (1 new ceiling-clamp test). - Bug verified: with the clamp temporarily reverted, the above_ceiling_ttl_is_clamped test fails with the real Redis error "value is not an integer or out of range" — proving the test catches the regression, not just the fix. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-auth/src/lib.rs | 4 +- crates/buzz-auth/src/nip98_replay.rs | 48 +++++++++++++++++ crates/buzz-auth/src/rate_limit.rs | 18 +++++++ crates/buzz-pubsub/src/nip98_replay.rs | 75 +++++++++++++++++++++----- 4 files changed, 132 insertions(+), 13 deletions(-) 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")); + } } From 3df6179d035d9ea7d2487a30cd44c7255db063a8 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Fri, 26 Jun 2026 16:46:04 -0400 Subject: [PATCH 4/4] fence(auth): host-binding side door + access-checker community fence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two adversarially-proven multi-tenant fences for the auth lane on the frozen Lane 0 SHA: 1. NIP-98 verifier: drop loopback aliasing unconditionally. normalize_url() collapsed localhost / ::1 -> 127.0.0.1 — a testing convenience that becomes a row-zero side door under multi-tenant. The u-tag host is the community binding (docs/multi-tenant-conformance.md, NIP-98 row); collapsing the three would let an event signed for localhost pass against a 127.0.0.1-resolved community (or vice versa). Inverted the localhost test to bite the new strict rule: signed-for-one vs expected-other now REJECTS, identity still passes. Adversarial: re-introduced the aliasing -> test goes red -> restored. 2. ChannelAccessChecker: thread &TenantContext through every method. Frozen 0001 has channels PK (community_id, id), so the same UUID legitimately co-exists across communities. A bare WHERE id = implementation would be a cross-community existence oracle. Mirror of buzz-db rule 4a.1 on the auth side. MockAccessChecker keyed on (community, pubkey, channel_id); new test access_does_not_cross_communities bites the bare-id direction. Adversarial: dropped the community filter from the mock -> test goes red -> restored. No external impl of ChannelAccessChecker in-tree (DB uses a separate free function under Mari's lane), so the trait signature change is contained. cargo test -p buzz-auth: 45 passed / 0 failed. Lane: auth (buzz-auth). Base: e349d7649 (frozen Lane 0). Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-auth/src/access.rs | 115 ++++++++++++++++++++++++++------- crates/buzz-auth/src/nip98.rs | 38 ++++++++--- 2 files changed, 120 insertions(+), 33 deletions(-) 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/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()); } }