diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f894c0e12fb..62a5a4728b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -722,6 +722,22 @@ jobs: --run-ignored ignored-only env: DATABASE_URL: postgres://buzz:${{ env.BUZZ_TEST_POSTGRES_PASSWORD }}@localhost:5432/buzz + - name: NIP-OA owner materialization gate tests + # Call-site integration for agent→owner materialization: a direct + # member's attestation is recorded on a closed relay, while a + # non-member owner and an expired attestation are refused. Selected by + # name because these span two modules (api::bridge, handlers::auth) and + # nothing else runs this crate's default test set — a test added here + # without this step would never execute. Needs Redis as well as + # Postgres: the submit path takes the NIP-98 replay guard. + run: | + cargo nextest run \ + --archive-file target/ci/backend-integration-tests.tar.zst \ + -E 'package(buzz-relay) and test(/nip_oa_owner_/)' \ + --run-ignored ignored-only + env: + DATABASE_URL: postgres://buzz:${{ env.BUZZ_TEST_POSTGRES_PASSWORD }}@localhost:5432/buzz + REDIS_URL: redis://localhost:6379 - name: NIP-ER reminder e2e # Feature e2e for NIP-ER (Event Reminders, kind:30300): write-path # validation, author-only read filtering, and scheduler delivery against diff --git a/Cargo.lock b/Cargo.lock index 73dbd8c2512..ef5f2a09cb1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -117,7 +117,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -128,7 +128,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -1666,7 +1666,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -2721,7 +2721,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -6126,6 +6126,17 @@ dependencies = [ "objc2-encode", ] +[[package]] +name = "objc2-app-kit" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d49e936b501e5c5bf01fda3a9452ff86dc3ea98ad5f283e1455153142d97518c" +dependencies = [ + "bitflags 2.13.0", + "objc2", + "objc2-foundation", +] + [[package]] name = "objc2-core-foundation" version = "0.3.2" @@ -7444,7 +7455,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.60.2", + "windows-sys 0.59.0", ] [[package]] @@ -8128,7 +8139,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -8187,7 +8198,7 @@ dependencies = [ "security-framework 3.7.0", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -8470,7 +8481,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b55fb86dfd3a2f5f76ea78310a88f96c4ea21a3031f8d212443d56123fd0521" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -8967,7 +8978,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -9595,7 +9606,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix 1.1.4", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -9608,7 +9619,7 @@ dependencies = [ "parking_lot", "rustix 1.1.4", "signal-hook", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -10378,7 +10389,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e" dependencies = [ "memoffset", "tempfile", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -10802,15 +10813,15 @@ dependencies = [ [[package]] name = "webbrowser" -version = "1.2.1" +version = "1.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fc95580916af1e68ff6a7be07446fc5db73ebf71cf092de939bbf5f7e189f72" +checksum = "62c35be770821a214dbc362fc26908c853e776c0004294d0b10b8a6bad582f94" dependencies = [ - "core-foundation 0.10.1", "jni 0.22.4", "log", "ndk-context", "objc2", + "objc2-app-kit", "objc2-foundation", "url", "web-sys", @@ -10962,7 +10973,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/crates/buzz-auth/src/lib.rs b/crates/buzz-auth/src/lib.rs index aed9624d9d6..dfd19f89e5b 100644 --- a/crates/buzz-auth/src/lib.rs +++ b/crates/buzz-auth/src/lib.rs @@ -72,10 +72,18 @@ pub struct AuthContext { pub channel_ids: Option>, /// How the connection was authenticated. pub auth_method: AuthMethod, - /// NIP-OA verified owner pubkey (if authenticated via owner attestation). + /// NIP-OA verified owner pubkey. /// - /// `None` for direct relay members or non-NIP-OA auth paths. - /// Set by the relay membership gate when NIP-OA fallback succeeds. + /// Set for any caller whose presented attestation resolved to a trusted + /// owner — including a direct relay member, which is why membership alone + /// no longer implies `None`. On a closed relay the claimed owner must also + /// be a relay member; an expired or not-yet-valid attestation resolves to + /// `None`. + /// + /// `None` for non-NIP-OA auth paths, and whenever the attestation could not + /// be trusted. Note this field selects the agent rate class in + /// `connection.rs`, so it must never be populated from an unverified or + /// untrusted owner. pub agent_owner_pubkey: Option, } diff --git a/crates/buzz-relay/src/api/bridge.rs b/crates/buzz-relay/src/api/bridge.rs index a118ff453fd..5d4b964c75b 100644 --- a/crates/buzz-relay/src/api/bridge.rs +++ b/crates/buzz-relay/src/api/bridge.rs @@ -55,17 +55,28 @@ async fn enforce_http_admission( } } +/// Outcome of bridge authentication. +#[derive(Debug)] +pub(crate) struct BridgeAuth { + /// The authenticated public key. + pub pubkey: nostr::PublicKey, + /// Event ID used for replay detection. Zero hash in X-Pubkey dev mode, + /// where there is no signed event and so no replay concern. + pub event_id_bytes: [u8; 32], + /// `created_at` of the NIP-98 request event, when one authenticated this + /// request. `None` in X-Pubkey dev mode: nothing was signed, so there is no + /// timestamp a NIP-OA attestation's bounds could be judged against. + pub created_at: Option, +} + /// Verify bridge auth: NIP-98 (production) or X-Pubkey (dev mode). -/// -/// Returns the authenticated public key and an event ID for replay detection. -/// For X-Pubkey dev mode, the event ID is a zero hash (no replay concern). pub(crate) fn verify_bridge_auth( headers: &HeaderMap, method: &str, url: &str, body: Option<&[u8]>, require_auth_token: bool, -) -> Result<(nostr::PublicKey, [u8; 32]), (StatusCode, Json)> { +) -> Result)> { verify_bridge_auth_with_options(headers, method, url, body, require_auth_token, false) } @@ -76,7 +87,7 @@ pub(crate) fn verify_bridge_auth_with_options( body: Option<&[u8]>, require_auth_token: bool, require_payload: bool, -) -> Result<(nostr::PublicKey, [u8; 32]), (StatusCode, Json)> { +) -> Result)> { // Try NIP-98 first (Authorization: Nostr ) if let Some(auth_str) = headers .get("authorization") @@ -111,7 +122,11 @@ pub(crate) fn verify_bridge_auth_with_options( let pubkey = buzz_auth::verify_nip98_event(&event_json, url, method, body) .map_err(|e| api_error(StatusCode::UNAUTHORIZED, &format!("NIP-98: {e}")))?; - return Ok((pubkey, event_id_bytes)); + return Ok(BridgeAuth { + pubkey, + event_id_bytes, + created_at: Some(event.created_at.as_secs()), + }); } // Dev-mode fallback: X-Pubkey header (only when require_auth_token is false) @@ -120,7 +135,11 @@ pub(crate) fn verify_bridge_auth_with_options( let pubkey = nostr::PublicKey::from_hex(hex_val) .map_err(|_| api_error(StatusCode::UNAUTHORIZED, "invalid X-Pubkey hex"))?; // Zero event ID — no replay detection needed for dev mode - return Ok((pubkey, [0u8; 32])); + return Ok(BridgeAuth { + pubkey, + event_id_bytes: [0u8; 32], + created_at: None, + }); } } @@ -637,7 +656,11 @@ pub async fn submit_event( })?; let url = nip98_expected_url(&state.config.relay_url, &tenant, "/events"); - let (pubkey, event_id_bytes) = verify_bridge_auth( + let BridgeAuth { + pubkey, + event_id_bytes, + created_at: auth_event_created_at, + } = verify_bridge_auth( &headers, "POST", &url, @@ -650,8 +673,16 @@ pub async fn submit_event( // runs inside the helper. The thin wrapper here owns the single terminal // attribution line so it fires for every outcome, including admission/ // replay/membership failures that previously returned before any log fired. - let outcome = - submit_event_authed(&state, &tenant, &headers, &body, pubkey, event_id_bytes).await; + let outcome = submit_event_authed( + &state, + &tenant, + &headers, + &body, + pubkey, + event_id_bytes, + auth_event_created_at, + ) + .await; match &outcome { SubmitOutcome::Ok { accepted, .. } => { @@ -758,6 +789,7 @@ async fn submit_event_authed( body: &[u8], pubkey: nostr::PublicKey, event_id_bytes: [u8; 32], + auth_event_created_at: Option, ) -> SubmitOutcome { // Admission and replay checks fire before body parse — a 429 or replay // reject on a malformed body must still be attributed. @@ -809,13 +841,23 @@ async fn submit_event_authed( ) .await { - Ok(owner) => owner.or_else(|| { - if !state.config.require_relay_membership { - super::relay_members::extract_nip_oa_owner(&pubkey_bytes, auth_tag) - } else { - None + // Without a NIP-98 request event there is no signed timestamp to judge + // the attestation's time bounds against (X-Pubkey dev auth), so no + // ownership is recorded rather than treating the tag as unbounded. + Ok(owner) => match auth_event_created_at { + Some(created_at) => { + super::relay_members::resolve_nip_oa_owner( + state, + tenant.community(), + owner, + &pubkey_bytes, + auth_tag, + created_at, + ) + .await } - }), + None => None, + }, Err(e) => { return SubmitOutcome::Err { status: e.0, @@ -905,7 +947,11 @@ pub async fn query_events( })?; let url = nip98_expected_url(&state.config.relay_url, &tenant, "/query"); - let (pubkey, event_id_bytes) = verify_bridge_auth( + let BridgeAuth { + pubkey, + event_id_bytes, + .. + } = verify_bridge_auth( &headers, "POST", &url, @@ -1348,7 +1394,11 @@ pub async fn count_events( })?; let url = nip98_expected_url(&state.config.relay_url, &tenant, "/count"); - let (pubkey, event_id_bytes) = verify_bridge_auth( + let BridgeAuth { + pubkey, + event_id_bytes, + .. + } = verify_bridge_auth( &headers, "POST", &url, @@ -2087,8 +2137,11 @@ async fn authorize_moderation_read( _ => path.to_string(), }; let url = nip98_expected_url(&state.config.relay_url, &tenant, &path_with_query); - let (pubkey, event_id_bytes) = - verify_bridge_auth(headers, "GET", &url, None, state.config.require_auth_token)?; + let BridgeAuth { + pubkey, + event_id_bytes, + .. + } = verify_bridge_auth(headers, "GET", &url, None, state.config.require_auth_token)?; check_nip98_replay(state, &tenant, event_id_bytes).await?; let pubkey_bytes = pubkey.to_bytes().to_vec(); @@ -2550,7 +2603,7 @@ mod tests { let tenant_a = fresh_tenant("host-a.example"); let expected_url = nip98_expected_url(config_relay_url, &tenant_a, "/events"); - let (pubkey, _event_id_bytes) = + let BridgeAuth { pubkey, .. } = verify_bridge_auth(&headers, "POST", &expected_url, Some(b""), true) .expect("matching-host NIP-98 event must verify"); assert_eq!( @@ -2599,7 +2652,7 @@ mod tests { Some("limit=20&status=open"), ); - let (pubkey, _event_id_bytes) = + let BridgeAuth { pubkey, .. } = verify_bridge_auth(&headers, "GET", &expected_url, None, true) .expect("query-bearing moderation read must verify against the same query"); assert_eq!(pubkey, keys.public_key()); @@ -2656,7 +2709,7 @@ mod tests { Some("limit=20"), ); - let (pubkey, _event_id_bytes) = + let BridgeAuth { pubkey, .. } = verify_bridge_auth(&headers, "GET", &expected_url, None, true) .expect("audit query-bearing read must verify"); assert_eq!(pubkey, keys.public_key()); @@ -2681,7 +2734,7 @@ mod tests { ); assert_eq!(expected_url, "https://host-a.example/moderation/restricted"); - let (pubkey, _event_id_bytes) = + let BridgeAuth { pubkey, .. } = verify_bridge_auth(&headers, "GET", &expected_url, None, true) .expect("query-less restricted read must verify against the bare path"); assert_eq!(pubkey, keys.public_key()); @@ -3766,4 +3819,273 @@ mod tests { "attribution line must carry the pubkey;\nlog:\n{log}" ); } + // ── NIP-OA owner materialization over the real HTTP path ──────────────── + // + // These enter at `submit_event_authed`, the authenticated core of + // `POST /events`: everything outside it is NIP-98 verification and the + // attribution log, and everything the owner path touches — the membership + // gate, `resolve_nip_oa_owner`, `materialize_nip_oa_owner` — is inside. + // Reverting the production call site makes the first test below fail, + // which the previous pure unit tests did not. + // + // `#[ignore]`d because they need Postgres and Redis; CI selects them by + // name (`test(/nip_oa_owner_/)`), since no job runs this crate's default + // test set. + + /// These tests are `#[ignore]`d and run only when explicitly selected, so a + /// silent skip is never what the caller wanted: it turns "the database was + /// missing" into a passing run. Fail loudly instead — this is the same + /// false-green shape the tests themselves exist to rule out. + fn require_infra(value: Option) -> T { + value.expect( + "NIP-OA owner tests need Postgres and Redis (DATABASE_URL / REDIS_URL); \ + refusing to pass without them", + ) + } + + async fn nip_oa_test_state() -> Option<(Arc, sqlx::PgPool)> { + let mut config = crate::config::Config::from_env().ok()?; + // The regression is closed-relay-only: on an open relay the owner was + // always recorded. + config.require_relay_membership = true; + let pool = sqlx::PgPool::connect(&config.database_url).await.ok()?; + let db = buzz_db::Db::from_pool(pool.clone()); + let redis_pool = deadpool_redis::Config::from_url(&config.redis_url) + .create_pool(Some(deadpool_redis::Runtime::Tokio1)) + .ok()?; + let pubsub = Arc::new( + buzz_pubsub::PubSubManager::new(&config.redis_url, redis_pool.clone()) + .await + .ok()?, + ); + let audit = buzz_audit::AuditService::new(pool.clone()); + let auth = buzz_auth::AuthService::new(config.auth.clone()); + let search = buzz_search::SearchService::new(pool.clone()); + let workflow_engine = Arc::new(buzz_workflow::WorkflowEngine::new( + db.clone(), + buzz_workflow::WorkflowConfig::default(), + )); + let media_storage = buzz_media::MediaStorage::new(&config.media).ok()?; + let (state, _audit_shutdown) = AppState::new( + config, + db, + redis_pool, + audit, + pubsub, + auth, + search, + workflow_engine, + Keys::generate(), + media_storage, + ); + Some((Arc::new(state), pool)) + } + + async fn seed_community(pool: &sqlx::PgPool) -> TenantContext { + let id = uuid::Uuid::new_v4(); + let host = format!("nip-oa-{}.example", id.simple()); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(id) + .bind(&host) + .execute(pool) + .await + .expect("insert test community"); + TenantContext::resolved(buzz_core::CommunityId::from_uuid(id), host) + } + + /// A signed event for the request body. Its ingest outcome is irrelevant: + /// owner materialization happens before ingest, so the assertion holds + /// whether or not the event itself is accepted. + fn body_event(keys: &Keys) -> Vec { + let event = EventBuilder::new(Kind::TextNote, "nip-oa owner materialization probe") + .sign_with_keys(keys) + .expect("sign body event"); + serde_json::to_vec(&event).expect("serialize body event") + } + + fn auth_tag_headers(tag_json: &str) -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert("x-auth-tag", tag_json.parse().expect("header value")); + headers + } + + async fn stored_owner( + state: &AppState, + tenant: &TenantContext, + agent: &nostr::PublicKey, + ) -> Option> { + state + .db + .get_agent_channel_policy(tenant.community(), agent.as_bytes()) + .await + .expect("read agent policy") + .and_then(|(_, owner)| owner) + } + + /// Run one `POST /events` submission as `agent`, presenting `tag_json`. + /// + /// Panics if the submit fails before reaching owner materialization. + /// Admission and the NIP-98 replay guard run first and both fail closed on + /// a Redis blip, which short-circuits the request; without this check that + /// shows up downstream as "the owner was not recorded" and reads exactly + /// like a product bug instead of the infrastructure failure it is. + async fn submit_with_tag( + state: &Arc, + tenant: &TenantContext, + agent: &Keys, + tag_json: Option<&str>, + auth_event_created_at: u64, + ) { + let headers = match tag_json { + Some(tag) => auth_tag_headers(tag), + None => HeaderMap::new(), + }; + let outcome = submit_event_authed( + state, + tenant, + &headers, + &body_event(agent), + agent.public_key(), + fresh_nip98_event_id_bytes(), + Some(auth_event_created_at), + ) + .await; + if let SubmitOutcome::Err { status, .. } = &outcome { + panic!( + "submit failed before owner materialization (status {status}) — \ + infrastructure, not the owner path" + ); + } + } + + /// The regression itself: a direct relay member on a closed relay presents + /// a valid attestation, and the owner is recorded. Before the fix this + /// silently resolved to no owner, so `owner_only` policies had nothing to + /// match and observer frames were refused. + #[tokio::test] + #[ignore = "requires Postgres and Redis"] + async fn nip_oa_owner_http_records_owner_for_direct_member() { + let (state, pool) = require_infra(nip_oa_test_state().await); + let tenant = seed_community(&pool).await; + let agent = Keys::generate(); + let owner = Keys::generate(); + + for pubkey in [agent.public_key(), owner.public_key()] { + state + .db + .add_relay_member(tenant.community(), &pubkey.to_hex(), "member", None) + .await + .expect("add relay member"); + } + + let tag = buzz_sdk::nip_oa::compute_auth_tag(&owner, &agent.public_key(), "") + .expect("compute auth tag"); + submit_with_tag(&state, &tenant, &agent, Some(&tag), 1_000).await; + + assert_eq!( + stored_owner(&state, &tenant, &agent.public_key()).await, + Some(owner.public_key().to_bytes().to_vec()), + "a direct member's verified owner must be recorded on a closed relay", + ); + } + + /// A member cannot mint a throwaway keypair, attest itself, and have that + /// key trusted: the resolved owner selects the agent rate class and is + /// first-write-wins, so an untrusted key must never reach the record. + #[tokio::test] + #[ignore = "requires Postgres and Redis"] + async fn nip_oa_owner_http_refuses_an_owner_that_is_not_a_relay_member() { + let (state, pool) = require_infra(nip_oa_test_state().await); + let tenant = seed_community(&pool).await; + let agent = Keys::generate(); + let stranger = Keys::generate(); + + state + .db + .add_relay_member( + tenant.community(), + &agent.public_key().to_hex(), + "member", + None, + ) + .await + .expect("add relay member"); + + let tag = buzz_sdk::nip_oa::compute_auth_tag(&stranger, &agent.public_key(), "") + .expect("compute auth tag"); + submit_with_tag(&state, &tenant, &agent, Some(&tag), 1_000).await; + + assert_eq!( + stored_owner(&state, &tenant, &agent.public_key()).await, + None, + "a non-member owner must not be recorded on a closed relay", + ); + } + + /// A signature that verifies is not a credential that is valid: an expired + /// attestation must not materialize, or expiry means nothing. + #[tokio::test] + #[ignore = "requires Postgres and Redis"] + async fn nip_oa_owner_http_refuses_an_expired_attestation() { + let (state, pool) = require_infra(nip_oa_test_state().await); + let tenant = seed_community(&pool).await; + let agent = Keys::generate(); + let owner = Keys::generate(); + + for pubkey in [agent.public_key(), owner.public_key()] { + state + .db + .add_relay_member(tenant.community(), &pubkey.to_hex(), "member", None) + .await + .expect("add relay member"); + } + + let expired = + buzz_sdk::nip_oa::compute_auth_tag(&owner, &agent.public_key(), "created_at<1000") + .expect("compute auth tag"); + // Auth event is at the bound, which is outside it — bounds are strict. + submit_with_tag(&state, &tenant, &agent, Some(&expired), 1_000).await; + + assert_eq!( + stored_owner(&state, &tenant, &agent.public_key()).await, + None, + "an expired attestation must not be materialized", + ); + + // Same tag, inside its window: proves the refusal above is the time + // bound and not some unrelated rejection. + submit_with_tag(&state, &tenant, &agent, Some(&expired), 999).await; + assert_eq!( + stored_owner(&state, &tenant, &agent.public_key()).await, + Some(owner.public_key().to_bytes().to_vec()), + "the same attestation inside its window must be recorded", + ); + } + + /// Membership alone never invents an owner. + #[tokio::test] + #[ignore = "requires Postgres and Redis"] + async fn nip_oa_owner_http_without_a_tag_records_nothing() { + let (state, pool) = require_infra(nip_oa_test_state().await); + let tenant = seed_community(&pool).await; + let agent = Keys::generate(); + + state + .db + .add_relay_member( + tenant.community(), + &agent.public_key().to_hex(), + "member", + None, + ) + .await + .expect("add relay member"); + + submit_with_tag(&state, &tenant, &agent, None, 1_000).await; + + assert_eq!( + stored_owner(&state, &tenant, &agent.public_key()).await, + None, + ); + } } diff --git a/crates/buzz-relay/src/api/invites.rs b/crates/buzz-relay/src/api/invites.rs index 6104171ccad..1ad394e1360 100644 --- a/crates/buzz-relay/src/api/invites.rs +++ b/crates/buzz-relay/src/api/invites.rs @@ -247,7 +247,11 @@ async fn authenticate( })?; let url = bridge::nip98_expected_url(&state.config.relay_url, &tenant, path); - let (pubkey, event_id_bytes) = bridge::verify_bridge_auth_with_options( + let bridge::BridgeAuth { + pubkey, + event_id_bytes, + .. + } = bridge::verify_bridge_auth_with_options( headers, "POST", &url, diff --git a/crates/buzz-relay/src/api/mod.rs b/crates/buzz-relay/src/api/mod.rs index d9f829433b1..9f47b46586f 100644 --- a/crates/buzz-relay/src/api/mod.rs +++ b/crates/buzz-relay/src/api/mod.rs @@ -116,11 +116,13 @@ pub mod relay_members { /// its NIP-OA owner *is* — access is granted via delegation. /// /// On open relays (`require_relay_membership = false`), returns `Ok(None)` - /// immediately — no membership check is performed. Callers that need NIP-OA - /// owner extraction on open relays should call [`extract_nip_oa_owner`] directly. + /// immediately — no membership check is performed. /// /// Returns `Ok(None)` when the caller is a direct member (closed relay) or when - /// no NIP-OA tag is present/applicable (open relay without auth tag). + /// no NIP-OA tag is present/applicable (open relay without auth tag). `Ok(None)` + /// therefore means "admitted on its own", **not** "has no owner": callers that + /// record ownership must pass the result through [`resolve_nip_oa_owner`], which + /// recovers the owner from the presented tag in exactly those cases. pub async fn enforce_relay_membership( state: &AppState, community: CommunityId, @@ -144,13 +146,19 @@ pub mod relay_members { } } - /// Extract NIP-OA owner from an auth tag without membership enforcement. + /// Identify the NIP-OA owner named by an auth tag, **without** enforcing the + /// tag's time bounds. /// - /// Used on open relays (`require_relay_membership = false`) to opportunistically - /// extract the owner pubkey for agent→owner backfill. The NIP-OA signature is - /// cryptographically self-proving, so no feature flag is needed — if the tag - /// verifies, the owner relationship is authentic. Returns `None` if the tag - /// is absent or invalid. + /// This answers "who does this tag name as the owner?", which is the right + /// question for *restriction* paths: the ban cascades in `handlers::auth` + /// and `api::git::transport` deny an agent whose owner is banned, and an + /// expired attestation must not become an escape hatch from that. Widening + /// who gets denied is safe; widening who gets trusted is not. + /// + /// Anything that *grants* — recording ownership, admitting a session, + /// choosing a rate class — must use [`extract_nip_oa_owner_at`] instead, so + /// an expired credential cannot confer authority. Returns `None` if the tag + /// is absent or fails signature verification. pub fn extract_nip_oa_owner( pubkey_bytes: &[u8], auth_tag_header: Option<&str>, @@ -166,6 +174,130 @@ pub mod relay_members { } } + /// Verify a NIP-OA auth tag *and* its time bounds, for paths that grant + /// authority from the result. + /// + /// `auth_event_created_at` is the `created_at` of the signed authentication + /// event that carried the tag — the NIP-42 AUTH event on WebSocket, the + /// NIP-98 request event over HTTP. The bound is part of what the owner + /// authorized, so it is judged against the same signed artifact the + /// attestation travelled with rather than against wall clock. + /// + /// Returns `None` if the tag is absent, fails signature verification, or is + /// outside its attested window. + pub fn extract_nip_oa_owner_at( + pubkey_bytes: &[u8], + auth_tag_header: Option<&str>, + auth_event_created_at: u64, + ) -> Option { + let tag_json = auth_tag_header?; + let agent_pubkey = nostr::PublicKey::from_slice(pubkey_bytes).ok()?; + match buzz_sdk::nip_oa::verify_auth_tag_at(tag_json, &agent_pubkey, auth_event_created_at) { + Ok(owner) => Some(owner), + Err(e) => { + info!("extract_nip_oa_owner_at: auth tag not usable: {e}"); + None + } + } + } + + /// Resolve the NIP-OA owner to materialize for a caller that has already + /// passed the membership gate. + /// + /// `gate_owner` is what [`enforce_relay_membership`] returned: `Some` only + /// when membership was granted *through* the owner, in which case that owner + /// has already been proven to be a relay member and is kept as-is. Every + /// other admitted caller — a direct relay member on a closed relay, or + /// anyone on an open relay — arrives here with `None`, and the `auth` tag + /// they presented is verified here instead. + /// + /// Gating this on `require_relay_membership` inverted the deployment + /// posture — the stricter relay was the only one that never recorded + /// ownership, so `owner_only` policies, observer-frame authorization and the + /// agent rate class all silently degraded for agents enrolled as members + /// (#4223, #4937). + /// + /// # Why a self-presented tag is not sufficient on its own + /// + /// The attestation proves an owner key signed for this agent. It does *not* + /// prove that key is anyone this relay trusts, and the resolved owner is not + /// inert metadata: `materialize_nip_oa_owner` creates a user row for it, and + /// `connection.rs` derives the agent rate class from the session carrying + /// it. Believing an arbitrary key would let any direct member mint a + /// throwaway keypair, attest itself, and take the agent message quota — + /// while `set_agent_owner` is first-write-wins, so that bogus mapping would + /// then be permanent and the real owner refused. On a closed relay the + /// claimed owner must therefore be a relay member, exactly as the + /// `ViaOwner` branch of [`check_relay_membership`] already requires. + /// + /// Open relays are unchanged: with no membership boundary to honour there is + /// nothing to check the owner against, and extraction stays unconditional. + /// + /// `allow_nip_oa_auth` is deliberately not consulted — its own doc comment + /// scopes it to whether NIP-OA may *grant membership*, which this never + /// does. The boundary enforced here is owner membership, not that flag. + /// + /// `auth_event_created_at` is the `created_at` of the signed authentication + /// event that carried the tag; the attestation's own time bounds are + /// enforced against it, so an expired or not-yet-valid credential resolves + /// to `None`. + pub async fn resolve_nip_oa_owner( + state: &AppState, + community: CommunityId, + gate_owner: Option, + pubkey_bytes: &[u8], + auth_tag_header: Option<&str>, + auth_event_created_at: u64, + ) -> Option { + // Verified here even when the gate already resolved an owner: + // `check_relay_membership` does not evaluate the tag's time bounds, so + // an expired credential can still produce a `ViaOwner` decision. That + // pre-existing membership grant is out of scope here, but it must not + // become a *materialized* ownership record. + let owner = extract_nip_oa_owner_at(pubkey_bytes, auth_tag_header, auth_event_created_at)?; + + if let Some(gate_owner) = gate_owner { + // Membership came through this owner, so it is already a proven + // relay member and needs no second membership read. The keys are + // derived from the same tag and must agree; if they somehow do not, + // fail closed rather than pick one. + if gate_owner != owner { + tracing::error!( + "resolve_nip_oa_owner: gate owner and re-verified owner disagree; \ + refusing to record ownership" + ); + return None; + } + return Some(gate_owner); + } + + if !state.config.require_relay_membership { + return Some(owner); + } + + let owner_hex = owner.to_hex(); + match state.db.is_relay_member(community, &owner_hex).await { + Ok(true) => Some(owner), + Ok(false) => { + info!( + owner = %owner_hex, + "resolve_nip_oa_owner: claimed owner is not a relay member; \ + refusing to record ownership on a closed relay" + ); + None + } + Err(e) => { + // Fail closed: without a membership answer the owner cannot be + // trusted, and materializing it is not reversible. + tracing::error!( + owner = %owner_hex, + "resolve_nip_oa_owner: owner membership check failed: {e}" + ); + None + } + } + } + /// Persist a cryptographically verified NIP-OA agent→owner relationship. /// /// Both principals are ensured first because `agent_owner_pubkey` has a @@ -273,5 +405,97 @@ pub mod relay_members { assert_eq!(result, None); } + + /// A caller the gate admitted on its own — a direct relay member on a + /// closed relay — still has its verified owner recovered from the tag, + /// instead of the attestation being dropped. This is the identification + /// half of the fix; whether that owner is *trusted* is decided by + /// `resolve_nip_oa_owner`, which needs a relay and is covered by the + /// Postgres-backed tests over the real HTTP and NIP-42 paths. + #[test] + fn extract_at_recovers_owner_for_a_direct_member() { + let owner_keys = Keys::generate(); + let agent_keys = Keys::generate(); + let agent_pubkey = agent_keys.public_key(); + + let tag_json = compute_auth_tag(&owner_keys, &agent_pubkey, "") + .expect("compute_auth_tag must succeed"); + + let result = extract_nip_oa_owner_at(&agent_pubkey.to_bytes(), Some(&tag_json), 1_000); + + assert_eq!(result, Some(owner_keys.public_key())); + } + + /// A direct member that presents no tag stays ownerless — membership + /// alone never invents an owner. + #[test] + fn extract_at_without_a_tag_returns_none() { + let agent_keys = Keys::generate(); + + let result = extract_nip_oa_owner_at(&agent_keys.public_key().to_bytes(), None, 1_000); + + assert_eq!(result, None); + } + + /// A tag that attests a *different* agent is not evidence about this + /// one: `verify_auth_tag_at` binds the attestation to the signing + /// pubkey, so an intercepted tag can't be replayed onto another agent. + #[test] + fn extract_at_rejects_a_tag_minted_for_another_agent() { + let owner_keys = Keys::generate(); + let attested_agent_keys = Keys::generate(); + let impostor_keys = Keys::generate(); + + let tag_json = compute_auth_tag(&owner_keys, &attested_agent_keys.public_key(), "") + .expect("compute_auth_tag must succeed"); + + let result = extract_nip_oa_owner_at( + &impostor_keys.public_key().to_bytes(), + Some(&tag_json), + 1_000, + ); + + assert_eq!(result, None); + } + + /// An expired or not-yet-valid attestation resolves to no owner, so it + /// cannot be materialized, seed an auth context, or lift the rate class. + /// The signature-only entry point still accepts both — that difference + /// is the whole reason the two functions exist. + #[test] + fn extract_at_refuses_credentials_outside_their_window() { + let owner_keys = Keys::generate(); + let agent_keys = Keys::generate(); + let agent_pubkey = agent_keys.public_key(); + + let expired = compute_auth_tag(&owner_keys, &agent_pubkey, "created_at<1000") + .expect("compute_auth_tag must succeed"); + let not_yet = compute_auth_tag(&owner_keys, &agent_pubkey, "created_at>9000") + .expect("compute_auth_tag must succeed"); + + let agent_bytes = agent_pubkey.to_bytes(); + let owner = Some(owner_keys.public_key()); + + assert_eq!( + extract_nip_oa_owner_at(&agent_bytes, Some(&expired), 999), + owner + ); + assert_eq!( + extract_nip_oa_owner_at(&agent_bytes, Some(&expired), 1000), + None + ); + assert_eq!( + extract_nip_oa_owner_at(&agent_bytes, Some(¬_yet), 9001), + owner + ); + assert_eq!( + extract_nip_oa_owner_at(&agent_bytes, Some(¬_yet), 9000), + None + ); + + // The identification-only path is deliberately unaffected: ban + // cascades must still recognise the owner of an expired tag. + assert_eq!(extract_nip_oa_owner(&agent_bytes, Some(&expired)), owner); + } } } diff --git a/crates/buzz-relay/src/api/operator.rs b/crates/buzz-relay/src/api/operator.rs index 5b69a43874c..d53da49b33b 100644 --- a/crates/buzz-relay/src/api/operator.rs +++ b/crates/buzz-relay/src/api/operator.rs @@ -75,7 +75,11 @@ async fn authorize_operator_request( _ => path.to_string(), }; let url = format!("{origin}{path_with_query}"); - let (pubkey, event_id_bytes) = bridge::verify_bridge_auth_with_options( + let bridge::BridgeAuth { + pubkey, + event_id_bytes, + .. + } = bridge::verify_bridge_auth_with_options( headers, method, &url, diff --git a/crates/buzz-relay/src/handlers/auth.rs b/crates/buzz-relay/src/handlers/auth.rs index 127f1fc40e0..6cb6076663a 100644 --- a/crates/buzz-relay/src/handlers/auth.rs +++ b/crates/buzz-relay/src/handlers/auth.rs @@ -2,9 +2,14 @@ //! //! Relay membership enforcement uses the shared //! [`crate::api::relay_members::enforce_relay_membership`] helper, which supports -//! NIP-OA owner-delegation fallback on closed relays. On open relays, the auth -//! handler calls [`crate::api::relay_members::extract_nip_oa_owner`] directly to -//! extract the owner pubkey for agent→owner backfill (observer frame auth). +//! NIP-OA owner-delegation fallback on closed relays. The owner recorded for +//! agent→owner backfill (observer frame auth, agent rate class) is then resolved +//! with [`crate::api::relay_members::resolve_nip_oa_owner`], which keeps the +//! delegated owner when membership came through one and otherwise verifies the +//! presented tag — including for a direct member, whose attestation was +//! previously dropped. On a closed relay the claimed owner must itself be a +//! relay member, and the tag's time bounds are enforced against this AUTH +//! event's `created_at`. //! //! For WebSocket auth, the NIP-OA `auth` tag is extracted from the signed AUTH //! event itself (the tag is integrity-protected by the event signature). @@ -76,6 +81,10 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: // The tag is integrity-protected by the event's Schnorr signature — if // tampered, NIP-42 verification will fail before we ever inspect it. let auth_tag_json = extract_auth_tag_json(&event); + // Captured before `verify_auth_event` consumes the event. The NIP-OA tag + // rides inside this AUTH event and is integrity-protected by its signature, + // so this is the timestamp the attestation's time bounds are judged against. + let auth_event_created_at = event.created_at.as_secs(); let relay_url = crate::api::bridge::nip42_expected_relay_url(&state.config.relay_url, &conn.tenant); @@ -237,20 +246,23 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: } }; - // Open relay NIP-OA backfill: extract owner for agent→owner DB mapping - // (needed for observer frame auth). Only runs on open relays — on closed - // relays, enforce_relay_membership already handles NIP-OA delegation. - // No feature flag needed: NIP-OA is cryptographically self-proving. - let nip_oa_owner = nip_oa_owner.or_else(|| { - if !state.config.require_relay_membership && auth_tag_json.is_some() { - crate::api::relay_members::extract_nip_oa_owner( - pubkey.as_bytes(), - auth_tag_json.as_deref(), - ) - } else { - None - } - }); + // NIP-OA backfill: resolve the owner for the agent→owner DB mapping + // (needed for observer frame auth and for the agent rate class). + // `enforce_relay_membership` reports an owner only when membership was + // granted *through* it, so a direct member's tag is resolved here — + // on closed relays too, subject to the owner itself being a relay + // member. The tag's time bounds are judged against this AUTH event's + // `created_at`: the tag is carried inside it and integrity-protected + // by its signature, so it is the artifact the attestation authorized. + let nip_oa_owner = crate::api::relay_members::resolve_nip_oa_owner( + &state, + conn.tenant.community(), + nip_oa_owner, + pubkey.as_bytes(), + auth_tag_json.as_deref(), + auth_event_created_at, + ) + .await; // Stash NIP-OA owner on the auth context only after the shared // backfill confirms the first-write-wins relationship. @@ -347,4 +359,274 @@ mod tests { ]); assert_eq!(extract_auth_tag_json(&event), None); } + // ── NIP-OA owner materialization over the real NIP-42 path ────────────── + // + // These drive `handle_auth` itself, so reverting its production call site + // makes the first one fail. `#[ignore]`d — they need Postgres and Redis, + // and CI selects them by name (`test(/nip_oa_owner_/)`). + + use std::collections::HashMap; + use std::sync::atomic::AtomicU8; + use std::sync::Arc; + use tokio::sync::{mpsc, Mutex, RwLock}; + use tokio_util::sync::CancellationToken; + use uuid::Uuid; + + use crate::connection::{AuthState, ConnectionState}; + use crate::state::AppState; + use buzz_core::tenant::TenantContext; + + /// See the note on `require_infra` in `api::bridge::tests`: an `#[ignore]`d + /// test that was explicitly selected must never pass by skipping. + fn require_infra(value: Option) -> T { + value.expect( + "NIP-OA owner tests need Postgres and Redis (DATABASE_URL / REDIS_URL); \ + refusing to pass without them", + ) + } + + async fn ws_test_state() -> Option<(Arc, sqlx::PgPool)> { + let mut config = crate::config::Config::from_env().ok()?; + config.require_relay_membership = true; + let pool = sqlx::PgPool::connect(&config.database_url).await.ok()?; + let db = buzz_db::Db::from_pool(pool.clone()); + let redis_pool = deadpool_redis::Config::from_url(&config.redis_url) + .create_pool(Some(deadpool_redis::Runtime::Tokio1)) + .ok()?; + let pubsub = Arc::new( + buzz_pubsub::PubSubManager::new(&config.redis_url, redis_pool.clone()) + .await + .ok()?, + ); + let audit = buzz_audit::AuditService::new(pool.clone()); + let auth = buzz_auth::AuthService::new(config.auth.clone()); + let search = buzz_search::SearchService::new(pool.clone()); + let workflow_engine = Arc::new(buzz_workflow::WorkflowEngine::new( + db.clone(), + buzz_workflow::WorkflowConfig::default(), + )); + let media_storage = buzz_media::MediaStorage::new(&config.media).ok()?; + let (state, _audit_shutdown) = AppState::new( + config, + db, + redis_pool, + audit, + pubsub, + auth, + search, + workflow_engine, + Keys::generate(), + media_storage, + ); + Some((Arc::new(state), pool)) + } + + async fn ws_seed_community(pool: &sqlx::PgPool) -> TenantContext { + let id = Uuid::new_v4(); + let host = format!("nip-oa-ws-{}.example", id.simple()); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(id) + .bind(&host) + .execute(pool) + .await + .expect("insert test community"); + TenantContext::resolved(buzz_core::CommunityId::from_uuid(id), host) + } + + /// A pending connection ready to receive AUTH, plus its challenge. + fn pending_conn(tenant: &TenantContext) -> (Arc, String) { + let challenge = buzz_auth::generate_challenge(); + let (send_tx, _send_rx) = mpsc::channel(16); + let (ctrl_tx, _ctrl_rx) = mpsc::channel(16); + let conn = Arc::new(ConnectionState { + conn_id: Uuid::new_v4(), + tenant: tenant.clone(), + remote_addr: "127.0.0.1:1234".parse().expect("socket addr"), + auth_state: RwLock::new(AuthState::Pending { + challenge: challenge.clone(), + }), + subscriptions: Arc::new(Mutex::new(HashMap::new())), + send_tx, + ctrl_tx, + cancel: CancellationToken::new(), + backpressure_count: Arc::new(AtomicU8::new(0)), + grace_limit: 3, + }); + (conn, challenge) + } + + /// Sign a NIP-42 AUTH event for `state`'s relay URL, carrying `tag_json` + /// as an `auth` tag, stamped at `created_at`. + fn auth_event( + state: &AppState, + tenant: &TenantContext, + agent: &Keys, + challenge: &str, + tag_json: Option<&str>, + created_at: u64, + ) -> nostr::Event { + let relay_url = + crate::api::bridge::nip42_expected_relay_url(&state.config.relay_url, tenant); + let url = nostr::RelayUrl::parse(&relay_url).expect("relay url"); + let mut builder = EventBuilder::auth(challenge, url); + if let Some(tag) = tag_json { + let parts: Vec = serde_json::from_str(tag).expect("auth tag json"); + builder = builder.tags([Tag::parse(parts).expect("auth tag")]); + } + builder + .custom_created_at(nostr::Timestamp::from(created_at)) + .sign_with_keys(agent) + .expect("sign auth event") + } + + /// NIP-42 rejects a stale AUTH event, so these tests stamp at the real + /// clock and express the tag's bounds relative to it — which is also how a + /// live deployment presents an expiring credential. + fn now_secs() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system clock after epoch") + .as_secs() + } + + async fn ws_stored_owner( + state: &AppState, + tenant: &TenantContext, + agent: &nostr::PublicKey, + ) -> Option> { + state + .db + .get_agent_channel_policy(tenant.community(), agent.as_bytes()) + .await + .expect("read agent policy") + .and_then(|(_, owner)| owner) + } + + /// The regression on the WebSocket path: a direct member authenticating + /// with a valid attestation gets its owner recorded *and* carried onto the + /// live auth context, which is what observer-frame authorization and the + /// agent rate class both read. + #[tokio::test] + #[ignore = "requires Postgres and Redis"] + async fn nip_oa_owner_ws_records_owner_and_sets_auth_context() { + let (state, pool) = require_infra(ws_test_state().await); + let tenant = ws_seed_community(&pool).await; + let agent = Keys::generate(); + let owner = Keys::generate(); + + for pubkey in [agent.public_key(), owner.public_key()] { + state + .db + .add_relay_member(tenant.community(), &pubkey.to_hex(), "member", None) + .await + .expect("add relay member"); + } + + let tag = buzz_sdk::nip_oa::compute_auth_tag(&owner, &agent.public_key(), "") + .expect("compute auth tag"); + let (conn, challenge) = pending_conn(&tenant); + let event = auth_event(&state, &tenant, &agent, &challenge, Some(&tag), now_secs()); + + super::handle_auth(event, Arc::clone(&conn), Arc::clone(&state)).await; + + assert_eq!( + ws_stored_owner(&state, &tenant, &agent.public_key()).await, + Some(owner.public_key().to_bytes().to_vec()), + "NIP-42 auth by a direct member must record its verified owner", + ); + + let auth_state = conn.auth_state.read().await; + match &*auth_state { + AuthState::Authenticated(ctx) => assert_eq!( + ctx.agent_owner_pubkey, + Some(owner.public_key()), + "the live auth context must carry the owner", + ), + other => panic!("expected authenticated connection, got {other:?}"), + } + } + + /// An expired attestation authenticates the agent but confers nothing: + /// no ownership record, and no owner on the session, so the connection + /// cannot be classified into the agent rate tier. + #[tokio::test] + #[ignore = "requires Postgres and Redis"] + async fn nip_oa_owner_ws_refuses_an_expired_attestation() { + let (state, pool) = require_infra(ws_test_state().await); + let tenant = ws_seed_community(&pool).await; + let agent = Keys::generate(); + let owner = Keys::generate(); + + for pubkey in [agent.public_key(), owner.public_key()] { + state + .db + .add_relay_member(tenant.community(), &pubkey.to_hex(), "member", None) + .await + .expect("add relay member"); + } + + let now = now_secs(); + // Bound sits at the AUTH event's own timestamp; bounds are strict, so + // the credential is one second past expiry. + let expired = buzz_sdk::nip_oa::compute_auth_tag( + &owner, + &agent.public_key(), + &format!("created_at<{now}"), + ) + .expect("compute auth tag"); + let (conn, challenge) = pending_conn(&tenant); + let event = auth_event(&state, &tenant, &agent, &challenge, Some(&expired), now); + + super::handle_auth(event, Arc::clone(&conn), Arc::clone(&state)).await; + + assert_eq!( + ws_stored_owner(&state, &tenant, &agent.public_key()).await, + None, + "an expired attestation must not be materialized", + ); + + let auth_state = conn.auth_state.read().await; + match &*auth_state { + AuthState::Authenticated(ctx) => assert_eq!( + ctx.agent_owner_pubkey, None, + "an expired attestation must not classify the session as an agent", + ), + other => panic!("expected authenticated connection, got {other:?}"), + } + } + + /// A non-member owner is not trusted on a closed relay, so nothing is + /// recorded and the session stays unclassified. + #[tokio::test] + #[ignore = "requires Postgres and Redis"] + async fn nip_oa_owner_ws_refuses_an_owner_that_is_not_a_relay_member() { + let (state, pool) = require_infra(ws_test_state().await); + let tenant = ws_seed_community(&pool).await; + let agent = Keys::generate(); + let stranger = Keys::generate(); + + state + .db + .add_relay_member( + tenant.community(), + &agent.public_key().to_hex(), + "member", + None, + ) + .await + .expect("add relay member"); + + let tag = buzz_sdk::nip_oa::compute_auth_tag(&stranger, &agent.public_key(), "") + .expect("compute auth tag"); + let (conn, challenge) = pending_conn(&tenant); + let event = auth_event(&state, &tenant, &agent, &challenge, Some(&tag), now_secs()); + + super::handle_auth(event, Arc::clone(&conn), Arc::clone(&state)).await; + + assert_eq!( + ws_stored_owner(&state, &tenant, &agent.public_key()).await, + None, + "a non-member owner must not be recorded on a closed relay", + ); + } } diff --git a/crates/buzz-relay/src/handlers/identity_archive.rs b/crates/buzz-relay/src/handlers/identity_archive.rs index 9da920483fe..a97fb01137d 100644 --- a/crates/buzz-relay/src/handlers/identity_archive.rs +++ b/crates/buzz-relay/src/handlers/identity_archive.rs @@ -332,28 +332,9 @@ fn enforce_request_auth_time_bounds(auth_tag_json: &str, created_at: u64) -> Res .get(2) .ok_or_else(|| "auth tag missing conditions".to_string())?; - for clause in conditions.split('&').filter(|clause| !clause.is_empty()) { - if let Some(bound) = clause.strip_prefix("created_at<") { - let bound = bound - .parse::() - .map_err(|_| format!("invalid created_at< bound: {bound}"))?; - if created_at >= bound { - return Err(format!( - "request auth time bound not satisfied: created_at {created_at} >= {bound}" - )); - } - } else if let Some(bound) = clause.strip_prefix("created_at>") { - let bound = bound - .parse::() - .map_err(|_| format!("invalid created_at> bound: {bound}"))?; - if created_at <= bound { - return Err(format!( - "request auth time bound not satisfied: created_at {created_at} <= {bound}" - )); - } - } - } - Ok(()) + // Shared with the ownership-materialization path so the two cannot drift: + // this handler's notion of "expired" is the only one in the tree. + buzz_sdk::nip_oa::evaluate_time_bounds(conditions, created_at).map_err(|e| e.to_string()) } #[cfg(test)] diff --git a/crates/buzz-sdk/src/nip_oa.rs b/crates/buzz-sdk/src/nip_oa.rs index 2dff81bcf7a..56bdb5b9c78 100644 --- a/crates/buzz-sdk/src/nip_oa.rs +++ b/crates/buzz-sdk/src/nip_oa.rs @@ -165,6 +165,76 @@ pub fn compute_auth_tag( Ok(tag_json.to_string()) } +/// Evaluate the `created_at` clauses of a `conditions` string against the +/// timestamp of the signed event that presented the attestation. +/// +/// Both bounds are **strict**: `created_atN` requires `timestamp > N`, so a timestamp exactly on either +/// bound is rejected. `kind=` clauses are not time bounds and are ignored here; +/// callers that restrict kinds must do so themselves. +/// +/// Structural validity is *not* re-checked — [`validate_conditions`] runs as +/// part of tag verification, and this is deliberately tolerant of clauses it +/// does not interpret so that a future clause type cannot be silently read as +/// "unbounded". +/// +/// # Errors +/// +/// Returns [`SdkError::InvalidInput`] when a bound is unparseable or when the +/// timestamp falls outside the attested window. +pub fn evaluate_time_bounds(conditions: &str, timestamp: u64) -> Result<(), SdkError> { + for clause in conditions.split('&').filter(|clause| !clause.is_empty()) { + if let Some(bound) = clause.strip_prefix("created_at<") { + let bound = bound.parse::().map_err(|_| { + SdkError::InvalidInput(format!("invalid created_at< bound: {bound}")) + })?; + if timestamp >= bound { + return Err(SdkError::InvalidInput(format!( + "auth tag time bound not satisfied: {timestamp} >= {bound}" + ))); + } + } else if let Some(bound) = clause.strip_prefix("created_at>") { + let bound = bound.parse::().map_err(|_| { + SdkError::InvalidInput(format!("invalid created_at> bound: {bound}")) + })?; + if timestamp <= bound { + return Err(SdkError::InvalidInput(format!( + "auth tag time bound not satisfied: {timestamp} <= {bound}" + ))); + } + } + } + Ok(()) +} + +/// Verify a NIP-OA `auth` tag *and* enforce its time bounds against the +/// timestamp of the signed event that presented it. +/// +/// [`verify_auth_tag`] proves only that the owner signed this attestation for +/// this agent; it says nothing about *when* the attestation is valid. Any +/// caller that grants authority from a tag — recording ownership, admitting a +/// session, selecting a rate class — must use this function instead, or an +/// expired credential is indistinguishable from a live one. +/// +/// `timestamp` is the `created_at` of the signed authentication event carrying +/// the tag (the NIP-42 AUTH event, or the NIP-98 request event), never wall +/// clock: the bound is a property of what the owner authorized, so it has to be +/// judged against the same signed artifact the attestation travelled with. +/// +/// # Errors +/// +/// Everything [`verify_auth_tag`] rejects, plus [`SdkError::InvalidInput`] when +/// `timestamp` falls outside the attested window. +pub fn verify_auth_tag_at( + auth_tag_json: &str, + agent_pubkey: &PublicKey, + timestamp: u64, +) -> Result { + let (owner_pubkey, conditions) = verify_auth_tag_parts(auth_tag_json, agent_pubkey)?; + evaluate_time_bounds(&conditions, timestamp)?; + Ok(owner_pubkey) +} + /// Verify a NIP-OA `auth` tag JSON string against the given `agent_pubkey`. /// /// Reconstructs the preimage, hashes it, and verifies the Schnorr signature @@ -172,6 +242,10 @@ pub fn compute_auth_tag( /// /// Returns the owner's [`PublicKey`] on success. /// +/// **This does not evaluate the tag's `created_at` bounds.** Use +/// [`verify_auth_tag_at`] anywhere the result grants authority; this entry +/// point remains for callers that only need to read the attested owner. +/// /// # Errors /// /// Returns [`SdkError::InvalidInput`] for malformed JSON, wrong element count, @@ -180,6 +254,16 @@ pub fn verify_auth_tag( auth_tag_json: &str, agent_pubkey: &PublicKey, ) -> Result { + verify_auth_tag_parts(auth_tag_json, agent_pubkey).map(|(owner_pubkey, _)| owner_pubkey) +} + +/// Shared body of [`verify_auth_tag`] and [`verify_auth_tag_at`]: everything +/// except the time-bound evaluation. Returns the attested owner alongside the +/// `conditions` string the signature commits to. +fn verify_auth_tag_parts( + auth_tag_json: &str, + agent_pubkey: &PublicKey, +) -> Result<(PublicKey, String), SdkError> { let arr = parse_json_array(auth_tag_json)?; if arr.len() != 4 { @@ -232,7 +316,7 @@ pub fn verify_auth_tag( .verify_schnorr(&sig, &message, &xonly) .map_err(|e| SdkError::InvalidInput(format!("signature verification failed: {e}")))?; - Ok(owner_pubkey) + Ok((owner_pubkey, conditions.to_string())) } /// Parse a NIP-OA `auth` tag JSON string into a [`Tag`] without verifying the @@ -592,4 +676,77 @@ mod tests { serde_json::json!(["auth", OWNER_PUBKEY_HEX, "kind=1&", "a".repeat(128)]).to_string(); assert!(parse_auth_tag(&bad).is_err()); } + + /// Both bounds are strict, so a timestamp exactly on either one is outside + /// the window. Pinned explicitly because an off-by-one here is the + /// difference between honouring an expiry and ignoring it for one second. + #[test] + fn test_time_bounds_are_strict_at_both_edges() { + let conditions = "created_at>100&created_at<200"; + + assert!(evaluate_time_bounds(conditions, 150).is_ok(), "inside"); + assert!( + evaluate_time_bounds(conditions, 101).is_ok(), + "lower edge+1" + ); + assert!( + evaluate_time_bounds(conditions, 199).is_ok(), + "upper edge-1" + ); + + assert!(evaluate_time_bounds(conditions, 100).is_err(), "== lower"); + assert!(evaluate_time_bounds(conditions, 200).is_err(), "== upper"); + assert!( + evaluate_time_bounds(conditions, 99).is_err(), + "not yet valid" + ); + assert!(evaluate_time_bounds(conditions, 201).is_err(), "expired"); + } + + /// An unbounded tag never expires, and a `kind=` clause is not a time bound. + #[test] + fn test_time_bounds_ignore_non_temporal_conditions() { + assert!(evaluate_time_bounds("", 0).is_ok()); + assert!(evaluate_time_bounds("kind=1", u64::MAX).is_ok()); + assert!(evaluate_time_bounds("kind=1&created_at<200", 199).is_ok()); + assert!(evaluate_time_bounds("kind=1&created_at<200", 200).is_err()); + } + + /// The regression this whole entry point exists for: a signature that + /// verifies is not the same as a credential that is currently valid. + /// `verify_auth_tag` accepts both; `verify_auth_tag_at` separates them. + #[test] + fn test_verify_at_rejects_expired_and_not_yet_valid_tags() { + let owner_keys = Keys::generate(); + let agent_keys = Keys::generate(); + let agent_pubkey = agent_keys.public_key(); + + let expired = compute_auth_tag(&owner_keys, &agent_pubkey, "created_at<1000") + .expect("compute expired tag"); + let not_yet = compute_auth_tag(&owner_keys, &agent_pubkey, "created_at>9000") + .expect("compute not-yet-valid tag"); + + // Signature-only verification cannot tell these apart from a live tag. + assert!(verify_auth_tag(&expired, &agent_pubkey).is_ok()); + assert!(verify_auth_tag(¬_yet, &agent_pubkey).is_ok()); + + assert!(verify_auth_tag_at(&expired, &agent_pubkey, 999).is_ok()); + assert!(verify_auth_tag_at(&expired, &agent_pubkey, 1000).is_err()); + assert!(verify_auth_tag_at(¬_yet, &agent_pubkey, 9001).is_ok()); + assert!(verify_auth_tag_at(¬_yet, &agent_pubkey, 9000).is_err()); + } + + /// Time bounds are evaluated *after* the signature, never instead of it: + /// a tag minted for another agent stays rejected inside its window. + #[test] + fn test_verify_at_still_enforces_the_signature_binding() { + let owner_keys = Keys::generate(); + let attested_agent_keys = Keys::generate(); + let impostor_keys = Keys::generate(); + + let tag = compute_auth_tag(&owner_keys, &attested_agent_keys.public_key(), "") + .expect("compute auth tag"); + + assert!(verify_auth_tag_at(&tag, &impostor_keys.public_key(), 1_000).is_err()); + } }