mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
audit(relay): collapse AuthError to category-only on wire — close existence-oracle
Per Eva's [12] handoff: the multi-tenant Redis keys (buzz:{community}:nip98:{id},
buzz:{community}:ratelimit:{hex}:{kind}) leak through AuthError::Internal(_) when
the relay wire converter stringifies the underlying redis::RedisError. The
bridge.rs:52 site forwarded the whole chain as the user-visible body —
`api_error(401, &format!("NIP-98: {e}"))` — turning the auth path into a
cross-tenant existence oracle on community-prefixed keys.
Two policy decisions land here as code + tests:
P1 — AuthError::Internal(_) NEVER reaches the wire as a string. The new
crate::auth_wire::auth_error_wire mapper maps Internal(_) -> InternalRedacted,
which produces a fixed "internal error" body regardless of the inner
String. The construction sites still log the detail (tracing::warn! at
construction is unchanged); only the wire converter redacts.
P2 — Nip98Invalid and Nip98Replay are byte-indistinguishable on the wire.
Returning a distinct error for replay tells an attacker that a guessed
event id has been seen in THIS community (the seen-set is community-scoped
per the S1 isolation fence). Both variants -> AuthErrorWireCategory::AuthFailed
-> identical status + body. Same reasoning collapses InvalidSignature,
ChallengeMismatch, RelayUrlMismatch, EventExpired, PubkeyMismatch into the
same category — all are "the auth artifact didn't verify."
Authorization-class outcomes (InsufficientScope, ChannelAccessDenied) stay
distinguishable on the wire — they're remediation-different (re-auth vs
re-request access) and don't carry tenant-scoped detail. If that ever
changes, authorization_class_distinguishable will need re-thinking.
The bridge.rs:52 site is the only current consumer; Sami's pending NIP-98
HTTP wire-up will use the same mapper. CI lint to forbid raw AuthError
stringification in response-construction is P5 in the audit note —
proposing that as a follow-up grep-lint Eva can wire into the §5 gate set.
Files:
- crates/buzz-relay/src/auth_wire.rs (new, +281 lines incl 6 property tests)
- crates/buzz-relay/src/api/bridge.rs:52 (use mapper, log detail)
- crates/buzz-relay/src/lib.rs (declare module)
Tests:
- replay_indistinguishable_from_invalid_on_wire (P2)
- verification_class_all_coalesce (P2 extended — 7 variants byte-identical)
- internal_redacted_does_not_leak_inner_string (P1 — synthesizes a leaked
Redis error containing buzz:<uuid>:nip98:<id> and asserts none of those
substrings appear on the wire)
- internal_two_communities_byte_identical (P1 — two distinct communities'
inner strings produce the same wire bytes; closes the cross-tenant oracle)
- authorization_class_distinguishable (negative — InsufficientScope and
ChannelAccessDenied stay distinct)
- ws_notice_redacts_internal_and_coalesces_verification (WS NOTICE parity)
Validation:
- cargo test -p buzz-relay --lib: 384 passed, 0 failed (378 prior + 6 new)
- cargo fmt -p buzz-relay --check: clean
- cargo clippy -p buzz-relay --no-deps --lib --tests: 0 new warnings
(one pre-existing on publish_nipia_unarchived at side_effects.rs:2619,
unrelated to this change — confirmed by git stash)
Base: rewrite/relay-wiring @ 87d5a8e35 (Eva's local merge of auth-tenant-scope
into relay-wiring; Sami's Nip98Replay variant is present here, NOT yet in
origin/rewrite/relay-wiring's pushed tip).
Holding for Eva's push of 87d5a8e35 + review per the audit lane handoff.
Audit note: RESEARCH/RELAY_REWRITE_AUTH_ERROR_ORACLE_AUDIT.md
Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
ed33878b7d
commit
7fc43fb391
@@ -49,8 +49,20 @@ fn verify_bridge_auth(
|
||||
.map_err(|_| api_error(StatusCode::UNAUTHORIZED, "invalid NIP-98 event JSON"))?;
|
||||
let event_id_bytes = event.id.to_bytes();
|
||||
|
||||
let pubkey = buzz_auth::verify_nip98_event(&event_json, url, method, body)
|
||||
.map_err(|e| api_error(StatusCode::UNAUTHORIZED, &format!("NIP-98: {e}")))?;
|
||||
let pubkey =
|
||||
buzz_auth::verify_nip98_event(&event_json, url, method, body).map_err(|e| {
|
||||
// Existence-oracle policy P1+P2 (audit/auth-error-payload-policy):
|
||||
// do NOT stringify the AuthError chain — Internal variants can
|
||||
// carry community-prefixed Redis keys, and a distinguishable
|
||||
// Nip98Replay reply would be a presence oracle on community-
|
||||
// scoped activity. Log detail, return category-only.
|
||||
tracing::warn!(
|
||||
auth_error = %e,
|
||||
"NIP-98 verification failed at HTTP bridge",
|
||||
);
|
||||
let (status, body) = crate::auth_wire::auth_error_wire(&e).http_response();
|
||||
(status, body)
|
||||
})?;
|
||||
|
||||
return Ok((pubkey, event_id_bytes));
|
||||
}
|
||||
|
||||
@@ -0,0 +1,286 @@
|
||||
//! Wire-side mapping for [`buzz_auth::AuthError`] → user-visible response.
|
||||
//!
|
||||
//! ## Why this exists
|
||||
//!
|
||||
//! `AuthError` is a rich operational enum — variants like `Internal(String)`
|
||||
//! and `Nip98Invalid(String)` carry detail that is useful in server logs and
|
||||
//! ruinous on the wire. Two specific risks the multi-tenant rewrite added:
|
||||
//!
|
||||
//! 1. **Key-prefix leak via `Internal(_)`.** The pubsub adapters wrap raw
|
||||
//! `redis::RedisError` values into `AuthError::Internal(format!("Redis ...: {e}"))`
|
||||
//! after touching community-prefixed keys (`buzz:{community}:nip98:{id}`,
|
||||
//! `buzz:{community}:ratelimit:{hex}:{kind}`). `RedisError::Display` is not
|
||||
//! guaranteed key-free across variants — `ResponseError`, `MOVED`/`ASK`,
|
||||
//! cluster redirects routinely include command/key context. Forwarding the
|
||||
//! chain to a client turns it into a cross-tenant existence oracle on Redis
|
||||
//! keys.
|
||||
//! 2. **Variant-distinguishable replay reply.** Returning a distinct error for
|
||||
//! `Nip98Replay` tells the attacker the event id has been seen in *this*
|
||||
//! community (the seen-set is community-scoped per the S1 isolation fence).
|
||||
//! Even with zero key text leaked, a distinguishable reply is a presence
|
||||
//! oracle on community-scoped activity for any guessed/sniffed event id.
|
||||
//!
|
||||
//! See `RESEARCH/RELAY_REWRITE_AUTH_ERROR_ORACLE_AUDIT.md` for the full
|
||||
//! payload-policy decisions (P1–P5).
|
||||
//!
|
||||
//! ## Contract
|
||||
//!
|
||||
//! All wire-side conversions of an `AuthError` go through
|
||||
//! [`auth_error_wire`]. The returned [`AuthErrorWireCategory`] is the only
|
||||
//! shape that may reach a client. The mapper deliberately collapses
|
||||
//! `Nip98Invalid` and `Nip98Replay` to the same category (P2) and maps
|
||||
//! `Internal(_)` to `InternalRedacted` (P1) — the original detail is logged
|
||||
//! at the construction site, not re-emitted here.
|
||||
|
||||
use axum::{http::StatusCode, response::Json};
|
||||
use buzz_auth::AuthError;
|
||||
use serde_json::json;
|
||||
|
||||
/// User-visible category for an [`AuthError`] on the wire.
|
||||
///
|
||||
/// The discriminants are user-visible. Each maps to a fixed byte sequence on
|
||||
/// each protocol surface (HTTP body, WS NOTICE). Adding a variant means
|
||||
/// adding a new oracle-channel decision; do not add a variant without a
|
||||
/// matching audit note.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum AuthErrorWireCategory {
|
||||
/// NIP-42 / NIP-98 verification failed for ANY reason (signature, replay,
|
||||
/// timestamp, pubkey, malformed event). Collapsed to one category to
|
||||
/// avoid a replay-vs-invalid oracle.
|
||||
AuthFailed,
|
||||
/// The authenticated identity lacks the required scope.
|
||||
InsufficientScope,
|
||||
/// The authenticated identity is not a member of the requested channel.
|
||||
ChannelAccessDenied,
|
||||
/// A server-side error occurred. No detail is exposed (P1).
|
||||
InternalRedacted,
|
||||
}
|
||||
|
||||
impl AuthErrorWireCategory {
|
||||
/// Stable HTTP status code for this category.
|
||||
pub fn status_code(self) -> StatusCode {
|
||||
match self {
|
||||
Self::AuthFailed => StatusCode::UNAUTHORIZED,
|
||||
Self::InsufficientScope => StatusCode::FORBIDDEN,
|
||||
Self::ChannelAccessDenied => StatusCode::FORBIDDEN,
|
||||
Self::InternalRedacted => StatusCode::INTERNAL_SERVER_ERROR,
|
||||
}
|
||||
}
|
||||
|
||||
/// Stable, byte-identical user-visible message for this category.
|
||||
///
|
||||
/// Two `AuthError` values that map to the same category MUST produce the
|
||||
/// same bytes here — that's the property-test invariant in
|
||||
/// `tests/auth_error_payload_oracle.rs`.
|
||||
pub fn message(self) -> &'static str {
|
||||
match self {
|
||||
// Coalesces Nip98Invalid, Nip98Replay, InvalidSignature,
|
||||
// ChallengeMismatch, RelayUrlMismatch, EventExpired,
|
||||
// PubkeyMismatch — all "the auth artifact didn't verify."
|
||||
Self::AuthFailed => "authentication failed",
|
||||
Self::InsufficientScope => "insufficient scope",
|
||||
Self::ChannelAccessDenied => "channel access denied",
|
||||
Self::InternalRedacted => "internal error",
|
||||
}
|
||||
}
|
||||
|
||||
/// Render this category as a standard JSON HTTP error response.
|
||||
pub fn http_response(self) -> (StatusCode, Json<serde_json::Value>) {
|
||||
(self.status_code(), Json(json!({ "error": self.message() })))
|
||||
}
|
||||
|
||||
/// Render this category as a Nostr-protocol NOTICE message body.
|
||||
///
|
||||
/// Returns the bare string (without the `["NOTICE", ...]` envelope) so
|
||||
/// the caller can decide whether to wrap with `RelayMessage::notice` or
|
||||
/// embed in an OK/CLOSED frame.
|
||||
pub fn notice_body(self) -> String {
|
||||
format!("error: {}", self.message())
|
||||
}
|
||||
}
|
||||
|
||||
/// Map an [`AuthError`] to its wire category.
|
||||
///
|
||||
/// **Do not stringify `AuthError` for the wire by any other path.** This
|
||||
/// function is the only sanctioned conversion; the CI grep-lint enforces
|
||||
/// that no other call site uses `auth_err.to_string()` or `format!("{}", auth_err)`
|
||||
/// in a response-construction chain (P5).
|
||||
pub fn auth_error_wire(err: &AuthError) -> AuthErrorWireCategory {
|
||||
match err {
|
||||
// Verification class — coalesce.
|
||||
AuthError::InvalidSignature
|
||||
| AuthError::ChallengeMismatch
|
||||
| AuthError::RelayUrlMismatch
|
||||
| AuthError::EventExpired
|
||||
| AuthError::Nip98Invalid(_)
|
||||
| AuthError::Nip98Replay
|
||||
| AuthError::PubkeyMismatch => AuthErrorWireCategory::AuthFailed,
|
||||
|
||||
AuthError::InsufficientScope { .. } => AuthErrorWireCategory::InsufficientScope,
|
||||
AuthError::ChannelAccessDenied => AuthErrorWireCategory::ChannelAccessDenied,
|
||||
|
||||
// Internal class — never leak detail. The construction site logs the
|
||||
// detail; the wire only sees the category.
|
||||
AuthError::Internal(_) => AuthErrorWireCategory::InternalRedacted,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use axum::body::to_bytes;
|
||||
use axum::response::IntoResponse;
|
||||
use buzz_auth::AuthError;
|
||||
|
||||
/// Render a wire category as the exact bytes a client would observe on
|
||||
/// the HTTP path. Used for byte-identity assertions across variants.
|
||||
async fn http_bytes(cat: AuthErrorWireCategory) -> (u16, Vec<u8>) {
|
||||
let (status, json) = cat.http_response();
|
||||
let resp = (status, json).into_response();
|
||||
let status = resp.status().as_u16();
|
||||
let body = to_bytes(resp.into_body(), 64 * 1024)
|
||||
.await
|
||||
.expect("body")
|
||||
.to_vec();
|
||||
(status, body)
|
||||
}
|
||||
|
||||
/// P2: Nip98Invalid and Nip98Replay collapse to one wire category.
|
||||
#[tokio::test]
|
||||
async fn replay_indistinguishable_from_invalid_on_wire() {
|
||||
let invalid = auth_error_wire(&AuthError::Nip98Invalid("garbage".into()));
|
||||
let replay = auth_error_wire(&AuthError::Nip98Replay);
|
||||
assert_eq!(invalid, replay);
|
||||
|
||||
let (invalid_status, invalid_body) = http_bytes(invalid).await;
|
||||
let (replay_status, replay_body) = http_bytes(replay).await;
|
||||
assert_eq!(invalid_status, replay_status);
|
||||
assert_eq!(
|
||||
invalid_body, replay_body,
|
||||
"Nip98Invalid and Nip98Replay MUST produce byte-identical responses (P2)"
|
||||
);
|
||||
}
|
||||
|
||||
/// P2 extended: all verification-class errors coalesce to one category.
|
||||
#[tokio::test]
|
||||
async fn verification_class_all_coalesce() {
|
||||
let errs = [
|
||||
AuthError::InvalidSignature,
|
||||
AuthError::ChallengeMismatch,
|
||||
AuthError::RelayUrlMismatch,
|
||||
AuthError::EventExpired,
|
||||
AuthError::Nip98Invalid("any detail".into()),
|
||||
AuthError::Nip98Replay,
|
||||
AuthError::PubkeyMismatch,
|
||||
];
|
||||
let cats: Vec<_> = errs.iter().map(auth_error_wire).collect();
|
||||
for c in &cats {
|
||||
assert_eq!(*c, AuthErrorWireCategory::AuthFailed);
|
||||
}
|
||||
// Body+status all identical.
|
||||
let first = http_bytes(cats[0]).await;
|
||||
for c in cats {
|
||||
assert_eq!(http_bytes(c).await, first);
|
||||
}
|
||||
}
|
||||
|
||||
/// P1: Internal(_) NEVER carries detail to the wire, regardless of the
|
||||
/// string. Synthesizes a worst-case payload that mimics a leaked Redis
|
||||
/// error containing a community-prefixed key, and asserts the wire body
|
||||
/// does not echo any byte of it.
|
||||
#[tokio::test]
|
||||
async fn internal_redacted_does_not_leak_inner_string() {
|
||||
let leaky =
|
||||
"Redis SET NX EX: ResponseError: WRONGTYPE on key buzz:00112233-4455-6677-8899-aabbccddeeff:nip98:deadbeef";
|
||||
let err = AuthError::Internal(leaky.into());
|
||||
let cat = auth_error_wire(&err);
|
||||
assert_eq!(cat, AuthErrorWireCategory::InternalRedacted);
|
||||
|
||||
let (status, body) = http_bytes(cat).await;
|
||||
assert_eq!(status, 500);
|
||||
let body_str = std::str::from_utf8(&body).expect("body utf8");
|
||||
// The category-only response must NOT contain any byte of the inner
|
||||
// detail — not the key prefix, not the community UUID, not "Redis".
|
||||
assert!(
|
||||
!body_str.contains("buzz:"),
|
||||
"wire body leaks community key prefix: {body_str}"
|
||||
);
|
||||
assert!(
|
||||
!body_str.contains("nip98"),
|
||||
"wire body leaks Redis key shape: {body_str}"
|
||||
);
|
||||
assert!(
|
||||
!body_str.contains("Redis"),
|
||||
"wire body leaks Redis driver string: {body_str}"
|
||||
);
|
||||
assert!(
|
||||
!body_str.contains("00112233"),
|
||||
"wire body leaks community UUID: {body_str}"
|
||||
);
|
||||
}
|
||||
|
||||
/// P1: two distinct Internal(_) values for two distinct communities
|
||||
/// produce byte-identical wire responses. Proves the wire shape is
|
||||
/// independent of the inner detail (closes the cross-tenant oracle).
|
||||
#[tokio::test]
|
||||
async fn internal_two_communities_byte_identical() {
|
||||
let community_a = "Redis pool: SET buzz:aaaaaaaa-1111-2222-3333-444444444444:nip98:cafe";
|
||||
let community_b = "Redis pool: SET buzz:bbbbbbbb-5555-6666-7777-888888888888:nip98:beef";
|
||||
let cat_a = auth_error_wire(&AuthError::Internal(community_a.into()));
|
||||
let cat_b = auth_error_wire(&AuthError::Internal(community_b.into()));
|
||||
assert_eq!(cat_a, cat_b);
|
||||
assert_eq!(http_bytes(cat_a).await, http_bytes(cat_b).await);
|
||||
}
|
||||
|
||||
/// Scope and channel errors are distinct from auth-failed — they are
|
||||
/// authorization-class outcomes that the client must distinguish (different
|
||||
/// remediation: re-auth vs re-request access vs join channel). These do
|
||||
/// NOT carry tenant-scoped detail in the variant, so they're safe to
|
||||
/// distinguish. (If that ever changes, this test fails by design.)
|
||||
#[test]
|
||||
fn authorization_class_distinguishable() {
|
||||
assert_eq!(
|
||||
auth_error_wire(&AuthError::InsufficientScope {
|
||||
required: "write".into(),
|
||||
have: vec![]
|
||||
}),
|
||||
AuthErrorWireCategory::InsufficientScope
|
||||
);
|
||||
assert_eq!(
|
||||
auth_error_wire(&AuthError::ChannelAccessDenied),
|
||||
AuthErrorWireCategory::ChannelAccessDenied
|
||||
);
|
||||
assert_ne!(
|
||||
AuthErrorWireCategory::AuthFailed,
|
||||
AuthErrorWireCategory::InsufficientScope
|
||||
);
|
||||
assert_ne!(
|
||||
AuthErrorWireCategory::AuthFailed,
|
||||
AuthErrorWireCategory::ChannelAccessDenied
|
||||
);
|
||||
}
|
||||
|
||||
/// NOTICE body parity: every category produces a NOTICE that does not
|
||||
/// leak inner detail, and verification-class NOTICEs are byte-identical.
|
||||
#[test]
|
||||
fn ws_notice_redacts_internal_and_coalesces_verification() {
|
||||
let leaky =
|
||||
AuthError::Internal("buzz:abcd:nip98:dead — should never appear on wire".into());
|
||||
let notice = auth_error_wire(&leaky).notice_body();
|
||||
assert!(
|
||||
!notice.contains("buzz:"),
|
||||
"NOTICE leaks key prefix: {notice}"
|
||||
);
|
||||
assert!(
|
||||
!notice.contains("nip98"),
|
||||
"NOTICE leaks Redis key: {notice}"
|
||||
);
|
||||
|
||||
let invalid = auth_error_wire(&AuthError::Nip98Invalid("x".into())).notice_body();
|
||||
let replay = auth_error_wire(&AuthError::Nip98Replay).notice_body();
|
||||
assert_eq!(
|
||||
invalid, replay,
|
||||
"WS NOTICE for invalid and replay MUST be byte-identical"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@
|
||||
pub mod api;
|
||||
/// WebSocket audio relay for huddle voice channels.
|
||||
pub mod audio;
|
||||
pub mod auth_wire;
|
||||
/// Relay configuration from environment variables.
|
||||
pub mod config;
|
||||
/// WebSocket connection lifecycle and state.
|
||||
|
||||
Reference in New Issue
Block a user