From 6b474cc337668bd562424f96b51f19c678940c96 Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 13:39:25 +0800 Subject: [PATCH 01/12] Add NIP-98 service module and NostrAuth extractor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First in a sequence of PRs adding Nostr auth. This lands the plumbing in isolation — no endpoints wired up yet. - `src/service/nip98.rs` encapsulates the `nostr` crate: base64 decode, kind/created_at/tag checks, Schnorr verification. Returns the pubkey as bech32 (`npub1...`), matching the encoding used by the existing `user.npub` column. - `src/rest/nostr_auth.rs` defines `NostrAuth { npub: Option }` as an Actix-Web `FromRequest` extractor, mirroring the existing `Auth` bearer extractor. Reads the `Authorization: Nostr ` header (case-insensitive scheme per RFC 9110), derives the expected `u` tag from the actual request URL so a signature valid for one endpoint cannot be replayed on another. - Tests cover both modules: valid/invalid signatures, URL/method/kind mismatches, recency drift, lowercase scheme, missing header, Bearer-scheme rejection. Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 127 +++++++++++++++++++++ Cargo.toml | 7 +- src/rest/mod.rs | 1 + src/rest/nostr_auth.rs | 175 +++++++++++++++++++++++++++++ src/service/mod.rs | 1 + src/service/nip98.rs | 242 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 552 insertions(+), 1 deletion(-) create mode 100644 src/rest/nostr_auth.rs create mode 100644 src/service/nip98.rs diff --git a/Cargo.lock b/Cargo.lock index ad65136e..54bfbaa0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -452,6 +452,40 @@ version = "1.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2af50177e190e07a26ab74f8b1efbfe2ef87da2116221318cb1c2e82baf7de06" +[[package]] +name = "bech32" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32637268377fc7b10a8c6d51de3e7fba1ce5dd371a96e342b34e6078db558e7f" + +[[package]] +name = "bip39" +version = "2.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "90dbd31c98227229239363921e60fcf5e558e43ec69094d46fc4996f08d1d5bc" +dependencies = [ + "bitcoin_hashes", + "serde", + "unicode-normalization", +] + +[[package]] +name = "bitcoin-io" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dee39a0ee5b4095224a0cfc6bf4cc1baf0f9624b96b367e53b66d974e51d953" + +[[package]] +name = "bitcoin_hashes" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26ec84b80c482df901772e931a9a681e26a1b9ee2302edeff23cb30328745c8b" +dependencies = [ + "bitcoin-io", + "hex-conservative", + "serde", +] + [[package]] name = "bitflags" version = "1.3.2" @@ -551,6 +585,7 @@ dependencies = [ "include_dir", "matrix-sdk", "names", + "nostr", "regex", "reqwest", "rusqlite", @@ -1598,6 +1633,21 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + +[[package]] +name = "hex-conservative" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fda06d18ac606267c40c04e41b9947729bf8b9efe74bd4e82b61a5f26a510b9f" +dependencies = [ + "arrayvec", +] + [[package]] name = "hkdf" version = "0.12.4" @@ -2023,6 +2073,18 @@ dependencies = [ "generic-array 0.14.7", ] +[[package]] +name = "instant" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0242819d153cba4b4b05a5a8f2a7e9bbf97b6055b2a002b395c96b5ff3c0222" +dependencies = [ + "cfg-if", + "js-sys", + "wasm-bindgen", + "web-sys", +] + [[package]] name = "ipnet" version = "2.12.0" @@ -2449,6 +2511,30 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086" +[[package]] +name = "nostr" +version = "0.44.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3aa5e3b6a278ed061835fe1ee293b71641e6bf8b401cfe4e1834bbf4ef0a34e1" +dependencies = [ + "base64", + "bech32", + "bip39", + "bitcoin_hashes", + "cbc", + "chacha20", + "chacha20poly1305", + "getrandom 0.2.17", + "hex", + "instant", + "scrypt", + "secp256k1", + "serde", + "serde_json", + "unicode-normalization", + "url", +] + [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -3503,12 +3589,53 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" +[[package]] +name = "salsa20" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97a22f5af31f73a954c10289c93e8a50cc23d971e80ee446f1f6f7137a088213" +dependencies = [ + "cipher", +] + [[package]] name = "scopeguard" version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "scrypt" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0516a385866c09368f0b5bcd1caff3366aace790fcd46e2bb032697bb172fd1f" +dependencies = [ + "password-hash", + "pbkdf2", + "salsa20", + "sha2", +] + +[[package]] +name = "secp256k1" +version = "0.29.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9465315bc9d4566e1724f0fffcbcc446268cb522e60f9a27bcded6b19c108113" +dependencies = [ + "rand 0.8.5", + "secp256k1-sys", + "serde", +] + +[[package]] +name = "secp256k1-sys" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4387882333d3aa8cb20530a17c69a3752e97837832f34f6dccc760e715001d9" +dependencies = [ + "cc", +] + [[package]] name = "semver" version = "1.0.28" diff --git a/Cargo.toml b/Cargo.toml index 8d878be9..b4fe771d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,4 +108,9 @@ strum = { version = "0.28.0", features = ["derive"] } tokio = "1.52.1" # https://github.com/fnichol/names/blob/main/CHANGELOG.md -names = "0.14.0" \ No newline at end of file +names = "0.14.0" + +# Nostr protocol types + Schnorr signature verification, used by the NIP-98 +# HTTP auth extractor +# https://github.com/rust-nostr/nostr/releases +nostr = { version = "0.44.2", default-features = false, features = ["std"] } diff --git a/src/rest/mod.rs b/src/rest/mod.rs index 46942a47..593a9ea3 100644 --- a/src/rest/mod.rs +++ b/src/rest/mod.rs @@ -1,5 +1,6 @@ pub mod auth; pub mod error; +pub mod nostr_auth; pub mod v2; pub mod v3; pub mod v4; diff --git a/src/rest/nostr_auth.rs b/src/rest/nostr_auth.rs new file mode 100644 index 00000000..c3fe0eea --- /dev/null +++ b/src/rest/nostr_auth.rs @@ -0,0 +1,175 @@ +// See the note in `src/service/nip98.rs` — this extractor is infrastructure +// ahead of the endpoints that will consume it in a follow-up PR. +#![allow(dead_code)] + +use actix_web::{dev::Payload, http::header, FromRequest, HttpRequest}; +use std::future::Future; +use std::pin::Pin; + +use crate::service::nip98; + +/// NIP-98 extractor. Mirrors the `Auth` bearer extractor: never fails the +/// request, always yields a struct. When `npub` is `None` the handler +/// decides whether to reject (401) or treat auth as optional. +/// +/// A `Some(npub)` value guarantees the event was: +/// - base64-decoded + JSON-parsed successfully +/// - kind 27235 with an in-window `created_at` +/// - signed such that the `u` tag matches the actual request URL +/// - signed such that the `method` tag matches the actual request method +/// - verified under a valid Schnorr signature +/// +/// The `npub` is bech32-encoded (`npub1...`), matching the encoding used by +/// the `user.npub` DB column. +pub struct NostrAuth { + pub npub: Option, +} + +impl FromRequest for NostrAuth { + type Error = actix_web::Error; + type Future = Pin>>>; + + fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future { + let req = req.clone(); + Box::pin(async move { + let Some(payload) = req + .headers() + .get(header::AUTHORIZATION) + .and_then(|h| h.to_str().ok()) + .and_then(nip98::extract_nostr_auth) + else { + return Ok(NostrAuth { npub: None }); + }; + + // The `u` tag must match the URL the client signed against. We + // reconstruct it from the actual request so a signature for one + // endpoint can't be replayed on another. + let conn_info = req.connection_info(); + let scheme = conn_info.scheme(); + let host = conn_info.host(); + let path_and_query = req + .uri() + .path_and_query() + .map(|p| p.as_str()) + .unwrap_or(req.uri().path()); + let full_url = format!("{scheme}://{host}{path_and_query}"); + let method = req.method().as_str().to_string(); + drop(conn_info); + + match nip98::verify(payload, &full_url, &method) { + Ok(event) => Ok(NostrAuth { + npub: Some(event.npub), + }), + Err(_) => Ok(NostrAuth { npub: None }), + } + }) + } +} + +#[cfg(test)] +mod test { + use super::*; + use actix_web::test::TestRequest; + use actix_web::{test, web, App, HttpResponse}; + use base64::engine::general_purpose::STANDARD as BASE64; + use base64::Engine; + use nostr::event::EventBuilder; + use nostr::key::Keys; + use nostr::nips::nip19::ToBech32; + use nostr::{JsonUtil, Kind, Tag, Timestamp}; + + async fn handler(auth: NostrAuth) -> HttpResponse { + match auth.npub { + Some(npub) => HttpResponse::Ok().body(npub), + None => HttpResponse::Unauthorized().finish(), + } + } + + fn signed_nip98(keys: &Keys, url: &str, method: &str) -> String { + let event = EventBuilder::new(Kind::from_u16(27235), "") + .tags(vec![ + Tag::parse(["u", url]).unwrap(), + Tag::parse(["method", method]).unwrap(), + ]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(keys) + .unwrap(); + BASE64.encode(event.as_json().as_bytes()) + } + + #[test] + async fn missing_header_yields_none() { + let app = test::init_service(App::new().route("/", web::post().to(handler))).await; + let req = TestRequest::post().uri("/").to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 401); + } + + #[test] + async fn bearer_header_yields_none() { + let app = test::init_service(App::new().route("/", web::post().to(handler))).await; + let req = TestRequest::post() + .uri("/") + .insert_header((header::AUTHORIZATION, "Bearer something")) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 401); + } + + #[test] + async fn valid_header_yields_bech32_npub() { + let keys = Keys::generate(); + let app = test::init_service(App::new().route("/auth", web::post().to(handler))).await; + let signed = signed_nip98(&keys, "http://localhost:8080/auth", "POST"); + let req = TestRequest::post() + .uri("/auth") + .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 200); + let body = test::read_body(res).await; + let npub = std::str::from_utf8(&body).unwrap(); + assert_eq!(npub, keys.public_key().to_bech32().unwrap()); + } + + #[test] + async fn lowercase_scheme_accepted() { + let keys = Keys::generate(); + let app = test::init_service(App::new().route("/auth", web::post().to(handler))).await; + let signed = signed_nip98(&keys, "http://localhost:8080/auth", "POST"); + let req = TestRequest::post() + .uri("/auth") + .insert_header((header::AUTHORIZATION, format!("nostr {signed}"))) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 200); + } + + #[test] + async fn url_mismatch_yields_none() { + let keys = Keys::generate(); + let app = test::init_service(App::new().route("/auth", web::post().to(handler))).await; + // Event signs a different URL than the request is for. + let signed = signed_nip98(&keys, "http://localhost/different", "POST"); + let req = TestRequest::post() + .uri("/auth") + .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 401); + } + + #[test] + async fn method_derived_from_request() { + let keys = Keys::generate(); + let app = test::init_service(App::new().route("/auth", web::get().to(handler))).await; + // Event signs POST, request is GET — should fail. + let signed = signed_nip98(&keys, "http://localhost:8080/auth", "POST"); + let req = TestRequest::get() + .uri("/auth") + .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 401); + } +} diff --git a/src/service/mod.rs b/src/service/mod.rs index ba76a227..ab728ed0 100644 --- a/src/service/mod.rs +++ b/src/service/mod.rs @@ -8,6 +8,7 @@ pub mod gitea; pub mod invoice; pub mod log; pub mod matrix; +pub mod nip98; pub mod og; pub mod osm; pub mod overpass; diff --git a/src/service/nip98.rs b/src/service/nip98.rs new file mode 100644 index 00000000..9910869e --- /dev/null +++ b/src/service/nip98.rs @@ -0,0 +1,242 @@ +// Lands in isolation ahead of the endpoints that will consume it. The next +// PR wires `NostrAuth` into `POST /v4/auth/nostr`; until then clippy sees +// these items as dead. +#![allow(dead_code)] + +use crate::error::Error; +use base64::engine::general_purpose::STANDARD as BASE64; +use base64::Engine; +use nostr::event::Event; +use nostr::nips::nip19::ToBech32; +use nostr::JsonUtil; +use nostr::Kind; +use std::time::{SystemTime, UNIX_EPOCH}; + +const NIP98_KIND: u16 = 27235; +const MAX_TIMESTAMP_DRIFT_SECS: u64 = 60; + +/// Result of a successfully verified NIP-98 event. +#[derive(Debug)] +pub struct VerifiedNip98Event { + /// Bech32-encoded (`npub1...`) Nostr public key, per NIP-19. Matches the + /// encoding used by the `user.npub` column (see `select_by_npub` tests). + pub npub: String, +} + +/// Verify a NIP-98 HTTP auth event. +/// +/// `authorization_payload` is the base64-encoded event string (the part after +/// "Nostr " in the Authorization header). +/// +/// Validates: +/// 1. Base64 decoding and JSON parsing +/// 2. `kind == 27235` +/// 3. `created_at` within 60 seconds of server time +/// 4. `u` tag matches `expected_url` +/// 5. `method` tag matches `expected_method` (case-insensitive) +/// 6. Schnorr signature is valid +pub fn verify( + authorization_payload: &str, + expected_url: &str, + expected_method: &str, +) -> Result { + let decoded = BASE64 + .decode(authorization_payload) + .map_err(|e| Error::Other(format!("Invalid base64: {e}")))?; + + let json_str = + String::from_utf8(decoded).map_err(|e| Error::Other(format!("Invalid UTF-8: {e}")))?; + + let event = Event::from_json(&json_str) + .map_err(|e| Error::Other(format!("Invalid Nostr event JSON: {e}")))?; + + if event.kind != Kind::from_u16(NIP98_KIND) { + return Err(Error::Other(format!( + "Invalid event kind: expected {NIP98_KIND}, got {}", + event.kind.as_u16() + ))); + } + + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| Error::Other(format!("System time error: {e}")))? + .as_secs(); + let event_time = event.created_at.as_secs(); + let drift = now.abs_diff(event_time); + if drift > MAX_TIMESTAMP_DRIFT_SECS { + return Err(Error::Other(format!( + "Event timestamp too far from server time (drift: {drift}s, max: {MAX_TIMESTAMP_DRIFT_SECS}s)" + ))); + } + + let u_tag = find_tag_value(&event, "u") + .ok_or_else(|| Error::Other("Missing 'u' tag in NIP-98 event".to_string()))?; + if u_tag != expected_url { + return Err(Error::Other(format!( + "URL mismatch: event has '{u_tag}', expected '{expected_url}'" + ))); + } + + let method_tag = find_tag_value(&event, "method") + .ok_or_else(|| Error::Other("Missing 'method' tag in NIP-98 event".to_string()))?; + if !method_tag.eq_ignore_ascii_case(expected_method) { + return Err(Error::Other(format!( + "Method mismatch: event has '{method_tag}', expected '{expected_method}'" + ))); + } + + event + .verify() + .map_err(|e| Error::Other(format!("Signature verification failed: {e}")))?; + + let npub = event + .pubkey + .to_bech32() + .map_err(|e| Error::Other(format!("Pubkey bech32 encoding failed: {e}")))?; + + Ok(VerifiedNip98Event { npub }) +} + +/// Extract the base64 payload from an `Authorization` header value. Matches +/// the `Nostr` scheme case-insensitively per RFC 9110. +pub fn extract_nostr_auth(authorization_header: &str) -> Option<&str> { + let (scheme, rest) = authorization_header.split_once(' ')?; + if scheme.eq_ignore_ascii_case("Nostr") { + Some(rest) + } else { + None + } +} + +fn find_tag_value(event: &Event, tag_name: &str) -> Option { + for tag in event.tags.iter() { + let tag_vec = tag.as_slice(); + if tag_vec.len() >= 2 && tag_vec[0] == tag_name { + return Some(tag_vec[1].to_string()); + } + } + None +} + +#[cfg(test)] +mod test { + use super::*; + use nostr::event::EventBuilder; + use nostr::key::Keys; + use nostr::Tag; + use nostr::Timestamp; + + fn make_nip98_event(keys: &Keys, url: &str, method: &str) -> String { + let event = EventBuilder::new(Kind::from_u16(NIP98_KIND), "") + .tags(vec![ + Tag::parse(["u", url]).unwrap(), + Tag::parse(["method", method]).unwrap(), + ]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(keys) + .unwrap(); + let json = event.as_json(); + BASE64.encode(json.as_bytes()) + } + + #[test] + fn valid_event_returns_bech32_npub() { + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let b64 = make_nip98_event(&keys, url, "POST"); + let result = verify(&b64, url, "POST").unwrap(); + assert!( + result.npub.starts_with("npub1"), + "npub should be bech32-encoded, got {}", + result.npub + ); + assert_eq!(result.npub, keys.public_key().to_bech32().unwrap()); + } + + #[test] + fn method_match_is_case_insensitive() { + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let b64 = make_nip98_event(&keys, url, "post"); + assert!(verify(&b64, url, "POST").is_ok()); + } + + #[test] + fn wrong_url() { + let keys = Keys::generate(); + let b64 = make_nip98_event(&keys, "https://example.com/wrong", "POST"); + let err = verify(&b64, "https://api.btcmap.org/v4/auth/nostr", "POST").unwrap_err(); + assert!(err.to_string().contains("URL mismatch")); + } + + #[test] + fn wrong_method() { + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let b64 = make_nip98_event(&keys, url, "GET"); + let err = verify(&b64, url, "POST").unwrap_err(); + assert!(err.to_string().contains("Method mismatch")); + } + + #[test] + fn expired_event() { + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let event = EventBuilder::new(Kind::from_u16(NIP98_KIND), "") + .tags(vec![ + Tag::parse(["u", url]).unwrap(), + Tag::parse(["method", "POST"]).unwrap(), + ]) + .custom_created_at(Timestamp::from(0)) + .sign_with_keys(&keys) + .unwrap(); + let b64 = BASE64.encode(event.as_json().as_bytes()); + let err = verify(&b64, url, "POST").unwrap_err(); + assert!(err.to_string().contains("timestamp too far")); + } + + #[test] + fn wrong_kind() { + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let event = EventBuilder::new(Kind::from_u16(1), "") + .tags(vec![ + Tag::parse(["u", url]).unwrap(), + Tag::parse(["method", "POST"]).unwrap(), + ]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(&keys) + .unwrap(); + let b64 = BASE64.encode(event.as_json().as_bytes()); + let err = verify(&b64, url, "POST").unwrap_err(); + assert!(err.to_string().contains("Invalid event kind")); + } + + #[test] + fn invalid_base64() { + let err = verify("not-valid-base64!!!", "https://example.com", "POST").unwrap_err(); + assert!(err.to_string().contains("Invalid base64")); + } + + #[test] + fn extract_nostr_auth_uppercase_scheme() { + let header = "Nostr eyJpZCI6IjEyMyJ9"; + assert_eq!(extract_nostr_auth(header), Some("eyJpZCI6IjEyMyJ9")); + } + + #[test] + fn extract_nostr_auth_lowercase_scheme() { + let header = "nostr eyJpZCI6IjEyMyJ9"; + assert_eq!(extract_nostr_auth(header), Some("eyJpZCI6IjEyMyJ9")); + } + + #[test] + fn extract_nostr_auth_rejects_bearer() { + assert_eq!(extract_nostr_auth("Bearer some-token"), None); + } + + #[test] + fn extract_nostr_auth_rejects_no_space() { + assert_eq!(extract_nostr_auth("Nostr"), None); + } +} From bf0cefe5f557317774619f1dadf8a5eb4de66a0f Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 13:53:27 +0800 Subject: [PATCH 02/12] nostr_auth: remove explicit drop(conn_info), scope it instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses reviewer feedback #4. The drop() was defensive — NLL already handles the lifetime. Scoping the reference into a block for URL construction is cleaner and yields identical behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/rest/nostr_auth.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/rest/nostr_auth.rs b/src/rest/nostr_auth.rs index c3fe0eea..ea559db7 100644 --- a/src/rest/nostr_auth.rs +++ b/src/rest/nostr_auth.rs @@ -44,19 +44,18 @@ impl FromRequest for NostrAuth { // The `u` tag must match the URL the client signed against. We // reconstruct it from the actual request so a signature for one // endpoint can't be replayed on another. - let conn_info = req.connection_info(); - let scheme = conn_info.scheme(); - let host = conn_info.host(); - let path_and_query = req - .uri() - .path_and_query() - .map(|p| p.as_str()) - .unwrap_or(req.uri().path()); - let full_url = format!("{scheme}://{host}{path_and_query}"); - let method = req.method().as_str().to_string(); - drop(conn_info); - - match nip98::verify(payload, &full_url, &method) { + let full_url = { + let ci = req.connection_info(); + let path_and_query = req + .uri() + .path_and_query() + .map(|p| p.as_str()) + .unwrap_or(req.uri().path()); + format!("{}://{}{}", ci.scheme(), ci.host(), path_and_query) + }; + let method = req.method().as_str(); + + match nip98::verify(payload, &full_url, method) { Ok(event) => Ok(NostrAuth { npub: Some(event.npub), }), From c0fb05be83be37e0da49eaaf85c7566f4855eddb Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 13:56:07 +0800 Subject: [PATCH 03/12] nostr_auth: pin expected URL to config-provided ApiBaseUrl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses reviewer feedback #2. Previously the extractor derived the signed `u` tag's expected value from the request's connection info, which trusts the `Host` and `X-Forwarded-*` headers. Those are attacker-controlled on any deployment without a header-stripping proxy in front. An attacker who tricked a user into signing a NIP-98 event for `http://evil.example/auth` could then replay the event against the real server while spoofing `Host: evil.example`, and the reconstructed URL would match — authenticating as the victim. New design: an `ApiBaseUrl(pub String)` struct is expected in app_data. The extractor pins the scheme+host from config and takes only `path_and_query` from the request. Missing app_data is treated as a fail-closed: `npub: None`, same pattern as the `Auth` bearer extractor when the DB pool is absent. A follow-up PR that wires the extractor into a handler will also add the `main.rs` wiring (env-var-driven init of `ApiBaseUrl`). Two new tests cover the new invariants: - spoofed_host_header_is_ignored - no_base_url_configured_yields_none Co-Authored-By: Claude Opus 4.6 (1M context) --- src/rest/nostr_auth.rs | 115 ++++++++++++++++++++++++++++++++--------- 1 file changed, 92 insertions(+), 23 deletions(-) diff --git a/src/rest/nostr_auth.rs b/src/rest/nostr_auth.rs index ea559db7..9a91efb2 100644 --- a/src/rest/nostr_auth.rs +++ b/src/rest/nostr_auth.rs @@ -2,12 +2,23 @@ // ahead of the endpoints that will consume it in a follow-up PR. #![allow(dead_code)] +use actix_web::web::Data; use actix_web::{dev::Payload, http::header, FromRequest, HttpRequest}; use std::future::Future; use std::pin::Pin; use crate::service::nip98; +/// Trusted external base URL of the API (e.g. `https://api.btcmap.org`, or +/// `http://localhost:8080` in dev). Must be injected via `app_data` by +/// `main.rs`. The extractor uses this, not the `Host`/`X-Forwarded-*` +/// headers, to reconstruct the URL the signed NIP-98 event is expected to +/// bind to. Trusting the request headers here would allow an attacker who +/// had tricked a user into signing a bogus-host URL to replay the event +/// against the real server by spoofing the `Host` header. +#[derive(Clone)] +pub struct ApiBaseUrl(pub String); + /// NIP-98 extractor. Mirrors the `Auth` bearer extractor: never fails the /// request, always yields a struct. When `npub` is `None` the handler /// decides whether to reject (401) or treat auth as optional. @@ -31,7 +42,15 @@ impl FromRequest for NostrAuth { fn from_request(req: &HttpRequest, _payload: &mut Payload) -> Self::Future { let req = req.clone(); + let base_url = req.app_data::>().cloned(); Box::pin(async move { + // Without a trusted base URL we can't safely verify the `u` tag, + // so fail closed (matches the `Auth` extractor's pattern of + // returning None-auth when state is missing). + let Some(base_url) = base_url else { + return Ok(NostrAuth { npub: None }); + }; + let Some(payload) = req .headers() .get(header::AUTHORIZATION) @@ -41,18 +60,16 @@ impl FromRequest for NostrAuth { return Ok(NostrAuth { npub: None }); }; - // The `u` tag must match the URL the client signed against. We - // reconstruct it from the actual request so a signature for one - // endpoint can't be replayed on another. - let full_url = { - let ci = req.connection_info(); - let path_and_query = req - .uri() - .path_and_query() - .map(|p| p.as_str()) - .unwrap_or(req.uri().path()); - format!("{}://{}{}", ci.scheme(), ci.host(), path_and_query) - }; + // Base URL comes from config, never from request headers — path + // and query are the only request-derived pieces. An attacker who + // spoofs `Host` or `X-Forwarded-*` cannot influence what the + // signature is checked against. + let path_and_query = req + .uri() + .path_and_query() + .map(|p| p.as_str()) + .unwrap_or(req.uri().path()); + let full_url = format!("{}{}", base_url.0.trim_end_matches('/'), path_and_query); let method = req.method().as_str(); match nip98::verify(payload, &full_url, method) { @@ -77,6 +94,8 @@ mod test { use nostr::nips::nip19::ToBech32; use nostr::{JsonUtil, Kind, Tag, Timestamp}; + const TEST_BASE: &str = "https://api.example/test"; + async fn handler(auth: NostrAuth) -> HttpResponse { match auth.npub { Some(npub) => HttpResponse::Ok().body(npub), @@ -84,6 +103,22 @@ mod test { } } + fn app_with_base() -> App< + impl actix_web::dev::ServiceFactory< + actix_web::dev::ServiceRequest, + Config = (), + Response = actix_web::dev::ServiceResponse, + Error = actix_web::Error, + InitError = (), + >, + > { + App::new() + .app_data(Data::new(ApiBaseUrl(TEST_BASE.to_string()))) + .route("/auth", web::post().to(handler)) + .route("/auth", web::get().to(handler)) + .route("/", web::post().to(handler)) + } + fn signed_nip98(keys: &Keys, url: &str, method: &str) -> String { let event = EventBuilder::new(Kind::from_u16(27235), "") .tags(vec![ @@ -98,7 +133,7 @@ mod test { #[test] async fn missing_header_yields_none() { - let app = test::init_service(App::new().route("/", web::post().to(handler))).await; + let app = test::init_service(app_with_base()).await; let req = TestRequest::post().uri("/").to_request(); let res = test::call_service(&app, req).await; assert_eq!(res.status(), 401); @@ -106,7 +141,7 @@ mod test { #[test] async fn bearer_header_yields_none() { - let app = test::init_service(App::new().route("/", web::post().to(handler))).await; + let app = test::init_service(app_with_base()).await; let req = TestRequest::post() .uri("/") .insert_header((header::AUTHORIZATION, "Bearer something")) @@ -118,8 +153,8 @@ mod test { #[test] async fn valid_header_yields_bech32_npub() { let keys = Keys::generate(); - let app = test::init_service(App::new().route("/auth", web::post().to(handler))).await; - let signed = signed_nip98(&keys, "http://localhost:8080/auth", "POST"); + let app = test::init_service(app_with_base()).await; + let signed = signed_nip98(&keys, &format!("{TEST_BASE}/auth"), "POST"); let req = TestRequest::post() .uri("/auth") .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) @@ -134,8 +169,8 @@ mod test { #[test] async fn lowercase_scheme_accepted() { let keys = Keys::generate(); - let app = test::init_service(App::new().route("/auth", web::post().to(handler))).await; - let signed = signed_nip98(&keys, "http://localhost:8080/auth", "POST"); + let app = test::init_service(app_with_base()).await; + let signed = signed_nip98(&keys, &format!("{TEST_BASE}/auth"), "POST"); let req = TestRequest::post() .uri("/auth") .insert_header((header::AUTHORIZATION, format!("nostr {signed}"))) @@ -147,9 +182,9 @@ mod test { #[test] async fn url_mismatch_yields_none() { let keys = Keys::generate(); - let app = test::init_service(App::new().route("/auth", web::post().to(handler))).await; - // Event signs a different URL than the request is for. - let signed = signed_nip98(&keys, "http://localhost/different", "POST"); + let app = test::init_service(app_with_base()).await; + // Event signs a different path than the request is for. + let signed = signed_nip98(&keys, &format!("{TEST_BASE}/different"), "POST"); let req = TestRequest::post() .uri("/auth") .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) @@ -161,9 +196,9 @@ mod test { #[test] async fn method_derived_from_request() { let keys = Keys::generate(); - let app = test::init_service(App::new().route("/auth", web::get().to(handler))).await; + let app = test::init_service(app_with_base()).await; // Event signs POST, request is GET — should fail. - let signed = signed_nip98(&keys, "http://localhost:8080/auth", "POST"); + let signed = signed_nip98(&keys, &format!("{TEST_BASE}/auth"), "POST"); let req = TestRequest::get() .uri("/auth") .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) @@ -171,4 +206,38 @@ mod test { let res = test::call_service(&app, req).await; assert_eq!(res.status(), 401); } + + #[test] + async fn spoofed_host_header_is_ignored() { + // Attacker signs an event for "http://evil.example/auth" and replays + // it to the real server, sending a spoofed Host header. Because the + // extractor pins the base URL from app_data (not from connection + // info), the `u` tag in the event will not match, and auth must fail. + let keys = Keys::generate(); + let app = test::init_service(app_with_base()).await; + let signed = signed_nip98(&keys, "http://evil.example/auth", "POST"); + let req = TestRequest::post() + .uri("/auth") + .insert_header((header::HOST, "evil.example")) + .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 401); + } + + #[test] + async fn no_base_url_configured_yields_none() { + let keys = Keys::generate(); + // Intentionally omit Data::new(ApiBaseUrl(...)). Auth must fail + // closed: if main.rs forgets to inject the config, we do not fall + // back to reconstructing from headers. + let app = test::init_service(App::new().route("/auth", web::post().to(handler))).await; + let signed = signed_nip98(&keys, "https://api.example/test/auth", "POST"); + let req = TestRequest::post() + .uri("/auth") + .insert_header((header::AUTHORIZATION, format!("Nostr {signed}"))) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), 401); + } } From 59df5818102c5de8113e0426f84a0a0cb26d243f Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 13:58:34 +0800 Subject: [PATCH 04/12] nostr_auth: log NIP-98 verification errors at debug level Addresses reviewer feedback #3. Previously the `Err` arm of the verify call silently mapped to `npub: None`, which makes triage hard when an operator reports "Nostr login isn't working": there was no signal distinguishing bad base64 from URL mismatch from signature failure. Keeps the extractor fail-closed at runtime (no behavior change) but surfaces the reason at `tracing::debug!`, with the full URL and method so operators can correlate against incoming request logs. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/rest/nostr_auth.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/rest/nostr_auth.rs b/src/rest/nostr_auth.rs index 9a91efb2..e9aee309 100644 --- a/src/rest/nostr_auth.rs +++ b/src/rest/nostr_auth.rs @@ -76,7 +76,15 @@ impl FromRequest for NostrAuth { Ok(event) => Ok(NostrAuth { npub: Some(event.npub), }), - Err(_) => Ok(NostrAuth { npub: None }), + Err(e) => { + tracing::debug!( + error = %e, + url = %full_url, + method = %method, + "NIP-98 verification failed" + ); + Ok(NostrAuth { npub: None }) + } } }) } From d22874ef9c589a42071cc1ac91a38fcf643f5b36 Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 14:00:24 +0800 Subject: [PATCH 05/12] nip98: add test for event with tampered Schnorr signature Addresses reviewer feedback #5. The existing tests covered malformed input (bad base64, missing/wrong tags, wrong kind, stale timestamp) but not the case of a structurally valid event whose signature has been tampered with. This is the classic attack the Schnorr verification is there to prevent, so it deserves a direct assertion. The test signs a real event, deserialises, flips one hex character of `sig`, re-encodes, and confirms verify() rejects it. Exercises the `event.verify()` call which the other tests never reach. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/service/nip98.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/service/nip98.rs b/src/service/nip98.rs index 9910869e..cd5dcd97 100644 --- a/src/service/nip98.rs +++ b/src/service/nip98.rs @@ -218,6 +218,44 @@ mod test { assert!(err.to_string().contains("Invalid base64")); } + #[test] + fn tampered_signature_rejected() { + // Build a valid event, then flip one hex nibble of its signature so + // the structure and tags remain valid but the Schnorr check fails. + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let event = EventBuilder::new(Kind::from_u16(NIP98_KIND), "") + .tags(vec![ + Tag::parse(["u", url]).unwrap(), + Tag::parse(["method", "POST"]).unwrap(), + ]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(&keys) + .unwrap(); + + // Flip one character of the sig hex. The event's `id` still matches + // the content, the tags are valid, only the signature is wrong. + let mut json: serde_json::Value = serde_json::from_str(&event.as_json()).unwrap(); + let sig = json["sig"].as_str().unwrap().to_string(); + let first = sig.chars().next().unwrap(); + let flipped = if first == '0' { '1' } else { '0' }; + let mut new_sig = flipped.to_string(); + new_sig.push_str(&sig[1..]); + json["sig"] = serde_json::Value::String(new_sig); + let tampered_json = serde_json::to_string(&json).unwrap(); + let b64 = BASE64.encode(tampered_json.as_bytes()); + + let err = verify(&b64, url, "POST").unwrap_err(); + let msg = err.to_string(); + // `event.verify()` checks both the id and the signature; either + // failing here is acceptable — we just need the verifier to reject. + assert!( + msg.contains("Signature verification failed") + || msg.contains("Invalid Nostr event JSON"), + "expected rejection, got: {msg}" + ); + } + #[test] fn extract_nostr_auth_uppercase_scheme() { let header = "Nostr eyJpZCI6IjEyMyJ9"; From da2789c6af64960a8e8e1150a849b2f23f63cf59 Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 14:05:55 +0800 Subject: [PATCH 06/12] nip98: document the missing request-body binding Addresses reviewer feedback #1. NIP-98 defines an optional `payload` tag that binds the signature to the SHA256 of the request body. This module does not verify that tag today. For the upcoming `POST /v4/auth/nostr` endpoint the body is empty, so `payload` would always hash to a constant and add no security. But future endpoints that do carry request bodies (e.g. `PUT /v4/users/me/nostr`) must verify the tag to prevent body swap attacks within the recency window. This commit adds a prominent module-level doc comment so the next PR author and reviewers cannot miss the limitation. No runtime behavior change. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/service/nip98.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/service/nip98.rs b/src/service/nip98.rs index cd5dcd97..e64ee499 100644 --- a/src/service/nip98.rs +++ b/src/service/nip98.rs @@ -1,3 +1,25 @@ +//! NIP-98 HTTP auth event verification. +//! +//! ## Intentional limitation: no request-body binding +//! +//! NIP-98 specifies an optional `payload` tag whose value is the SHA256 hash +//! of the HTTP request body. When present it binds the signature to the body +//! content, preventing an attacker who captures a signed event from replaying +//! it with a different body inside the ±60s recency window. +//! +//! **This module does not verify the `payload` tag.** The follow-up endpoint +//! it will be used from (`POST /v4/auth/nostr`) carries no request body — +//! the signed event travels entirely in the `Authorization` header. For that +//! endpoint a `payload` check would compute `SHA256("")` on every call and +//! add no security. +//! +//! Future endpoints with request bodies (e.g. `PUT /v4/users/me/nostr`) +//! **must** add body-hash verification. Wiring that in after the fact +//! requires either a second extractor variant that consumes `web::Bytes` +//! (so the handler cannot also read the body) or a middleware that buffers +//! and replays. See https://github.com/nostr-protocol/nips/blob/master/98.md +//! for the spec text. + // Lands in isolation ahead of the endpoints that will consume it. The next // PR wires `NostrAuth` into `POST /v4/auth/nostr`; until then clippy sees // these items as dead. From 07543e01f82151e29cd6d38d3ad9575b85ad92fb Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 14:18:51 +0800 Subject: [PATCH 07/12] nip98: tolerate multiple spaces between auth-scheme and credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per RFC 9110 §11.6.2, `credentials = auth-scheme 1*SP (...)` — one or more SP is valid. split_once(' ') only handled exactly one. Replace with split_whitespace + a "no trailing tokens" check so the extractor accepts "Nostr " and "Nostr " consistently while still rejecting malformed values like "Nostr b64 extra". Addresses reviewer feedback on PR #86. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/service/nip98.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/service/nip98.rs b/src/service/nip98.rs index e64ee499..f60afde5 100644 --- a/src/service/nip98.rs +++ b/src/service/nip98.rs @@ -120,11 +120,20 @@ pub fn verify( } /// Extract the base64 payload from an `Authorization` header value. Matches -/// the `Nostr` scheme case-insensitively per RFC 9110. +/// the `Nostr` scheme case-insensitively per RFC 9110. Tolerates multiple +/// whitespace characters between scheme and credentials, as `1*SP` in the +/// `credentials` ABNF permits. Rejects headers with extra trailing tokens. pub fn extract_nostr_auth(authorization_header: &str) -> Option<&str> { - let (scheme, rest) = authorization_header.split_once(' ')?; + let mut parts = authorization_header.split_whitespace(); + let scheme = parts.next()?; + let payload = parts.next()?; + // `token68` in the ABNF contains no whitespace — any further tokens mean + // the header is malformed. + if parts.next().is_some() { + return None; + } if scheme.eq_ignore_ascii_case("Nostr") { - Some(rest) + Some(payload) } else { None } @@ -299,4 +308,17 @@ mod test { fn extract_nostr_auth_rejects_no_space() { assert_eq!(extract_nostr_auth("Nostr"), None); } + + #[test] + fn extract_nostr_auth_tolerates_multiple_spaces() { + // Per RFC 9110 §11.6.2: credentials = auth-scheme 1*SP (...) — one + // or more SP characters between scheme and credentials is legal. + assert_eq!(extract_nostr_auth("Nostr abc"), Some("abc")); + } + + #[test] + fn extract_nostr_auth_rejects_extra_tokens() { + // token68 contains no whitespace, so "Nostr abc extra" is malformed. + assert_eq!(extract_nostr_auth("Nostr abc extra"), None); + } } From 94fca3149950b6e7b5b949df0829f3eb1dce592f Mon Sep 17 00:00:00 2001 From: escapedcat Date: Mon, 13 Apr 2026 15:48:37 +0800 Subject: [PATCH 08/12] nostr_auth: fix port in ApiBaseUrl doc comment (8000, not 8080) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trivial doc correction per maintainer note — the API runs on 8000 in dev, not 8080. Only the example URL in the doc comment; no code paths affected. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/rest/nostr_auth.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rest/nostr_auth.rs b/src/rest/nostr_auth.rs index e9aee309..d08a2195 100644 --- a/src/rest/nostr_auth.rs +++ b/src/rest/nostr_auth.rs @@ -10,7 +10,7 @@ use std::pin::Pin; use crate::service::nip98; /// Trusted external base URL of the API (e.g. `https://api.btcmap.org`, or -/// `http://localhost:8080` in dev). Must be injected via `app_data` by +/// `http://localhost:8000` in dev). Must be injected via `app_data` by /// `main.rs`. The extractor uses this, not the `Host`/`X-Forwarded-*` /// headers, to reconstruct the URL the signed NIP-98 event is expected to /// bind to. Trusting the request headers here would allow an attacker who From 82948f5d99eac5e19615a703de3bd40f6b1d3081 Mon Sep 17 00:00:00 2001 From: escapedcat Date: Sat, 9 May 2026 15:39:08 +0200 Subject: [PATCH 09/12] nostr_auth: use #[actix_web::test] explicitly on async tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain #[test] resolves to actix-web's async-aware proc-macro here only because `use actix_web::{test, ...}` brings it into the macro namespace alongside the `test` module — a subtle shadow. Spelling the macro out makes the dependency on actix's test runtime obvious to readers and avoids relying on import order for correctness. No behavior change; the futures were already being polled. --- src/rest/nostr_auth.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rest/nostr_auth.rs b/src/rest/nostr_auth.rs index d08a2195..4ae0077d 100644 --- a/src/rest/nostr_auth.rs +++ b/src/rest/nostr_auth.rs @@ -139,7 +139,7 @@ mod test { BASE64.encode(event.as_json().as_bytes()) } - #[test] + #[actix_web::test] async fn missing_header_yields_none() { let app = test::init_service(app_with_base()).await; let req = TestRequest::post().uri("/").to_request(); @@ -147,7 +147,7 @@ mod test { assert_eq!(res.status(), 401); } - #[test] + #[actix_web::test] async fn bearer_header_yields_none() { let app = test::init_service(app_with_base()).await; let req = TestRequest::post() @@ -158,7 +158,7 @@ mod test { assert_eq!(res.status(), 401); } - #[test] + #[actix_web::test] async fn valid_header_yields_bech32_npub() { let keys = Keys::generate(); let app = test::init_service(app_with_base()).await; @@ -174,7 +174,7 @@ mod test { assert_eq!(npub, keys.public_key().to_bech32().unwrap()); } - #[test] + #[actix_web::test] async fn lowercase_scheme_accepted() { let keys = Keys::generate(); let app = test::init_service(app_with_base()).await; @@ -187,7 +187,7 @@ mod test { assert_eq!(res.status(), 200); } - #[test] + #[actix_web::test] async fn url_mismatch_yields_none() { let keys = Keys::generate(); let app = test::init_service(app_with_base()).await; @@ -201,7 +201,7 @@ mod test { assert_eq!(res.status(), 401); } - #[test] + #[actix_web::test] async fn method_derived_from_request() { let keys = Keys::generate(); let app = test::init_service(app_with_base()).await; @@ -215,7 +215,7 @@ mod test { assert_eq!(res.status(), 401); } - #[test] + #[actix_web::test] async fn spoofed_host_header_is_ignored() { // Attacker signs an event for "http://evil.example/auth" and replays // it to the real server, sending a spoofed Host header. Because the @@ -233,7 +233,7 @@ mod test { assert_eq!(res.status(), 401); } - #[test] + #[actix_web::test] async fn no_base_url_configured_yields_none() { let keys = Keys::generate(); // Intentionally omit Data::new(ApiBaseUrl(...)). Auth must fail From a99e646b094f42dd2a07500a0b66b0ecf6d3c2d6 Mon Sep 17 00:00:00 2001 From: escapedcat Date: Sat, 9 May 2026 16:01:27 +0200 Subject: [PATCH 10/12] nip98: reject duplicate u/method tags, tighten method check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review-driven changes in src/service/nip98.rs: 1. Reject duplicate u/method tags. NIP-98 doesn't explicitly require uniqueness, but accepting duplicates would let a hostile frontend construct an event with [u: real, u: decoy] that the user signs in one click and the verifier accepts via the matching one. Replaces find_tag_value with require_unique_tag, which surfaces a clear "Duplicate '' tag" error. 2. Make the method-tag comparison case-sensitive. RFC 9110 §9.1 defines HTTP method tokens as case-sensitive, and NIP-98 says the tag value MUST be the same HTTP method. All standard methods are uppercase, so the strict comparison costs nothing in practice and avoids a needless deviation from the relevant specs. The previous method_match_is_case_insensitive test is replaced with its inverse. 3. Tighten tampered_signature_rejected to assert "Signature verification failed" only. The earlier assertion accepted "Invalid Nostr event JSON" too, on the (incorrect) reasoning that event.verify()'s id-hash check might reject first. The Nostr event id is sha256 over (pubkey, created_at, kind, tags, content) and does not include the signature, so mutating only sig leaves the id check intact and the rejection must come from the Schnorr step. Locking the assertion proves Schnorr actually runs. Plus four new tests: missing_u_tag, missing_method_tag, duplicate_u_tag, duplicate_method_tag. --- src/service/nip98.rs | 128 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 113 insertions(+), 15 deletions(-) diff --git a/src/service/nip98.rs b/src/service/nip98.rs index f60afde5..bc9f057a 100644 --- a/src/service/nip98.rs +++ b/src/service/nip98.rs @@ -91,17 +91,19 @@ pub fn verify( ))); } - let u_tag = find_tag_value(&event, "u") - .ok_or_else(|| Error::Other("Missing 'u' tag in NIP-98 event".to_string()))?; + let u_tag = require_unique_tag(&event, "u")?; if u_tag != expected_url { return Err(Error::Other(format!( "URL mismatch: event has '{u_tag}', expected '{expected_url}'" ))); } - let method_tag = find_tag_value(&event, "method") - .ok_or_else(|| Error::Other("Missing 'method' tag in NIP-98 event".to_string()))?; - if !method_tag.eq_ignore_ascii_case(expected_method) { + // RFC 9110 §9.1 makes HTTP method tokens case-sensitive, and NIP-98 + // requires the tag value be the same HTTP method as the request. All + // standard methods are uppercase, so a strict comparison costs nothing + // and matches what every real client sends. + let method_tag = require_unique_tag(&event, "method")?; + if method_tag != expected_method { return Err(Error::Other(format!( "Method mismatch: event has '{method_tag}', expected '{expected_method}'" ))); @@ -139,14 +141,28 @@ pub fn extract_nostr_auth(authorization_header: &str) -> Option<&str> { } } -fn find_tag_value(event: &Event, tag_name: &str) -> Option { +/// Look up a NIP-98 tag, rejecting both absence and duplication. +/// +/// NIP-98 doesn't explicitly say "exactly one", but accepting duplicates +/// would let a malicious frontend trick a user into signing an event with +/// e.g. two `u` tags — one matching, one bogus — and have the verifier +/// accept it via the matching one. The signature covers all tags so an +/// attacker can't add tags after the fact, but nothing stops a hostile +/// client from constructing such an event in the first place. +fn require_unique_tag(event: &Event, tag_name: &str) -> Result { + let mut found: Option = None; for tag in event.tags.iter() { let tag_vec = tag.as_slice(); if tag_vec.len() >= 2 && tag_vec[0] == tag_name { - return Some(tag_vec[1].to_string()); + if found.is_some() { + return Err(Error::Other(format!( + "Duplicate '{tag_name}' tag in NIP-98 event" + ))); + } + found = Some(tag_vec[1].to_string()); } } - None + found.ok_or_else(|| Error::Other(format!("Missing '{tag_name}' tag in NIP-98 event"))) } #[cfg(test)] @@ -185,11 +201,14 @@ mod test { } #[test] - fn method_match_is_case_insensitive() { + fn method_match_is_case_sensitive() { + // RFC 9110 §9.1: HTTP method tokens are case-sensitive. Lowercase + // 'post' must not match 'POST'. let keys = Keys::generate(); let url = "https://api.btcmap.org/v4/auth/nostr"; let b64 = make_nip98_event(&keys, url, "post"); - assert!(verify(&b64, url, "POST").is_ok()); + let err = verify(&b64, url, "POST").unwrap_err(); + assert!(err.to_string().contains("Method mismatch")); } #[test] @@ -278,12 +297,91 @@ mod test { let err = verify(&b64, url, "POST").unwrap_err(); let msg = err.to_string(); - // `event.verify()` checks both the id and the signature; either - // failing here is acceptable — we just need the verifier to reject. + // Only `sig` is mutated. The Nostr event id (sha256 of pubkey, + // created_at, kind, tags, content) does not include the signature, + // so `event.verify()`'s id check still passes — the failure must + // come from the Schnorr step specifically. + assert!( + msg.contains("Signature verification failed"), + "expected Schnorr rejection, got: {msg}" + ); + } + + #[test] + fn missing_u_tag_rejected() { + let keys = Keys::generate(); + let event = EventBuilder::new(Kind::from_u16(NIP98_KIND), "") + .tags(vec![Tag::parse(["method", "POST"]).unwrap()]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(&keys) + .unwrap(); + let b64 = BASE64.encode(event.as_json().as_bytes()); + let err = verify(&b64, "https://api.btcmap.org/v4/auth/nostr", "POST").unwrap_err(); + assert!( + err.to_string().contains("Missing 'u' tag"), + "got: {err}" + ); + } + + #[test] + fn missing_method_tag_rejected() { + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let event = EventBuilder::new(Kind::from_u16(NIP98_KIND), "") + .tags(vec![Tag::parse(["u", url]).unwrap()]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(&keys) + .unwrap(); + let b64 = BASE64.encode(event.as_json().as_bytes()); + let err = verify(&b64, url, "POST").unwrap_err(); + assert!( + err.to_string().contains("Missing 'method' tag"), + "got: {err}" + ); + } + + #[test] + fn duplicate_u_tag_rejected() { + // Defense in depth: a hostile client could construct an event with + // two `u` tags — one real, one decoy. The signature covers both, so + // the user signs both. Reject outright. + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let event = EventBuilder::new(Kind::from_u16(NIP98_KIND), "") + .tags(vec![ + Tag::parse(["u", url]).unwrap(), + Tag::parse(["u", "https://evil.example/auth"]).unwrap(), + Tag::parse(["method", "POST"]).unwrap(), + ]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(&keys) + .unwrap(); + let b64 = BASE64.encode(event.as_json().as_bytes()); + let err = verify(&b64, url, "POST").unwrap_err(); + assert!( + err.to_string().contains("Duplicate 'u' tag"), + "got: {err}" + ); + } + + #[test] + fn duplicate_method_tag_rejected() { + let keys = Keys::generate(); + let url = "https://api.btcmap.org/v4/auth/nostr"; + let event = EventBuilder::new(Kind::from_u16(NIP98_KIND), "") + .tags(vec![ + Tag::parse(["u", url]).unwrap(), + Tag::parse(["method", "POST"]).unwrap(), + Tag::parse(["method", "GET"]).unwrap(), + ]) + .custom_created_at(Timestamp::now()) + .sign_with_keys(&keys) + .unwrap(); + let b64 = BASE64.encode(event.as_json().as_bytes()); + let err = verify(&b64, url, "POST").unwrap_err(); assert!( - msg.contains("Signature verification failed") - || msg.contains("Invalid Nostr event JSON"), - "expected rejection, got: {msg}" + err.to_string().contains("Duplicate 'method' tag"), + "got: {err}" ); } From 686fc48dcfdf698842476b6eeafc4a84aa912a61 Mon Sep 17 00:00:00 2001 From: escapedcat Date: Sat, 9 May 2026 16:08:59 +0200 Subject: [PATCH 11/12] =?UTF-8?q?nip98:=20cargo=20fmt=20=E2=80=94=20collap?= =?UTF-8?q?se=20two=20assert!=20macros=20to=20single=20line?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rustfmt rule: assert! with macro args that fit on one line shouldn't span multiple lines. Pure formatting; no behavior change. (fixup for a99e646 — squash on next rebase if desired) --- src/service/nip98.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/service/nip98.rs b/src/service/nip98.rs index bc9f057a..a77cd417 100644 --- a/src/service/nip98.rs +++ b/src/service/nip98.rs @@ -317,10 +317,7 @@ mod test { .unwrap(); let b64 = BASE64.encode(event.as_json().as_bytes()); let err = verify(&b64, "https://api.btcmap.org/v4/auth/nostr", "POST").unwrap_err(); - assert!( - err.to_string().contains("Missing 'u' tag"), - "got: {err}" - ); + assert!(err.to_string().contains("Missing 'u' tag"), "got: {err}"); } #[test] @@ -358,10 +355,7 @@ mod test { .unwrap(); let b64 = BASE64.encode(event.as_json().as_bytes()); let err = verify(&b64, url, "POST").unwrap_err(); - assert!( - err.to_string().contains("Duplicate 'u' tag"), - "got: {err}" - ); + assert!(err.to_string().contains("Duplicate 'u' tag"), "got: {err}"); } #[test] From be015d9242dfdc4926553c03dacd779ce5983c9c Mon Sep 17 00:00:00 2001 From: escapedcat Date: Sat, 9 May 2026 16:29:24 +0200 Subject: [PATCH 12/12] nostr: address Copilot doc-comment feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two doc-only fixes from Copilot review on PR #84: 1. nip98::verify doc list said "(case-insensitive)" for the method tag match — left over from before a99e646 tightened the comparison to strict equality. Update to "(case-sensitive, per RFC 9110 §9.1)" so the doc, the implementation, and the method_match_is_case_sensitive test all agree. 2. NostrAuth's "fail closed" comment was internally contradictory: yielding npub: None doesn't fail closed, it delegates to the handler — handlers that treat None as anonymous fail open. Reword to describe what the extractor actually does and call out the handler's responsibility. No behavior change. --- src/rest/nostr_auth.rs | 7 +++++-- src/service/nip98.rs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/rest/nostr_auth.rs b/src/rest/nostr_auth.rs index 4ae0077d..4597883a 100644 --- a/src/rest/nostr_auth.rs +++ b/src/rest/nostr_auth.rs @@ -45,8 +45,11 @@ impl FromRequest for NostrAuth { let base_url = req.app_data::>().cloned(); Box::pin(async move { // Without a trusted base URL we can't safely verify the `u` tag, - // so fail closed (matches the `Auth` extractor's pattern of - // returning None-auth when state is missing). + // so refuse to attempt verification: yield `npub: None` and let + // the handler decide whether to reject (matches the `Auth` + // extractor's pattern when state is missing). Whether this ends + // up fail-closed or fail-open in practice depends on the + // handler — required-auth handlers must treat `None` as 401. let Some(base_url) = base_url else { return Ok(NostrAuth { npub: None }); }; diff --git a/src/service/nip98.rs b/src/service/nip98.rs index a77cd417..d3bc96d9 100644 --- a/src/service/nip98.rs +++ b/src/service/nip98.rs @@ -55,7 +55,7 @@ pub struct VerifiedNip98Event { /// 2. `kind == 27235` /// 3. `created_at` within 60 seconds of server time /// 4. `u` tag matches `expected_url` -/// 5. `method` tag matches `expected_method` (case-insensitive) +/// 5. `method` tag matches `expected_method` (case-sensitive, per RFC 9110 §9.1) /// 6. Schnorr signature is valid pub fn verify( authorization_payload: &str,