From 3df6179d035d9ea7d2487a30cd44c7255db063a8 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Fri, 26 Jun 2026 16:46:04 -0400 Subject: [PATCH] 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()); } }