fix(buzz-auth): close SSRF/redirect bypass in HttpJwksFetcher; trim internal markers

- Remove with_client() bypass: HttpJwksFetcher is now a unit struct;
  each fetch_jwks call builds a dedicated per-request pinned client.
- Add resolve_and_check_ssrf: DNS-resolves host:port via spawn_blocking,
  rejects any resolved private/reserved IP (closes DNS-rebinding TOCTOU).
- Per-request client enforces: redirect(Policy::none()), no_proxy(),
  .resolve(host, pinned_ip), and timeout(JWKS_REQUEST_TIMEOUT_SECS).
- Drop unused client field (dead_code warning) now that no shared pool
  is needed.
- Remove pure-paraphrase doc on IssuerRegistry::all_policies(); replace
  with doc stating constraint (unspecified order, startup use).
- Remove all 'PR N' internal markers from doc comments; replace with
  production-stable references to the jwks runtime.

Co-authored-by: Cea Stapleton Cordasco <261786559+cea@users.noreply.github.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

crates/buzz-auth/src/nip_fi/config.rs

@@ -561,7 +561,8 @@ impl IssuerRegistry {
         self.policies.is_empty()
     }
 
-    /// Iterate over all registered policies.
+    /// All registered issuer policies, in unspecified order. Useful for
+    /// iterating over every configured issuer during startup validation.
     pub fn all_policies(&self) -> impl Iterator<Item = &IssuerPolicy> {
         self.policies.values()
     }

crates/buzz-auth/src/nip_fi/jwks/mod.rs

@@ -47,15 +47,15 @@ pub const MAX_JWKS_RESPONSE_BYTES: usize = 512 * 1024; // 512 KiB
 /// range panics when computing snapshot deadlines.
 pub const MAX_JWKS_TIMING_SECONDS: u64 = 365 * 24 * 3600; // 1 year
 
-/// Per-request deadline for the complete JWKS fetch (connect + headers + body).
-/// This constant documents the timeout set on the default `HttpJwksFetcher::new()`
-/// client; it cannot be removed via `with_client`.
+/// Per-request deadline for the complete JWKS fetch (connect + headers + body),
+/// enforced inside `fetch_jwks` independently of any client-level timeout.
 pub const JWKS_REQUEST_TIMEOUT_SECS: u64 = 10;
 
 /// Validate that a JWKS URI is safe to fetch: HTTPS scheme, no credentials,
-/// no fragment, and the host (if a bare IP) is not private/reserved. Hostnames
-/// are not resolved here — runtime SSRF for hostname targets is limited by
-/// redirect denial and the intrinsic request deadline.
+/// no fragment, and the host (if a bare IP) is not private/reserved.
+/// Hostname targets are resolved and checked at every fetch in `fetch_jwks`
+/// to prevent DNS rebinding — this check catches the most common
+/// misconfiguration at construction time.
 pub fn validate_jwks_uri(uri: &str) -> Result<(), JwksFetchError> {
     let parsed = Url::parse(uri).map_err(|_| JwksFetchError::InvalidUri)?;
     if parsed.scheme() != "https" {
@@ -70,8 +70,7 @@ pub fn validate_jwks_uri(uri: &str) -> Result<(), JwksFetchError> {
     if parsed.fragment().is_some() {
         return Err(JwksFetchError::InvalidUri);
     }
-    // Reject bare private/reserved IP targets at construction time. Hostname
-    // targets are additionally constrained at runtime by redirect denial.
+    // Reject bare private/reserved IP targets at construction time.
     if let Some(url::Host::Ipv4(addr)) = parsed.host() {
         if is_private_ip(&std::net::IpAddr::V4(addr)) {
             return Err(JwksFetchError::InvalidUri);
@@ -85,6 +84,37 @@ pub fn validate_jwks_uri(uri: &str) -> Result<(), JwksFetchError> {
     Ok(())
 }
 
+/// Resolve `host:port` to IP addresses and reject if any are private/reserved.
+///
+/// Returns the first safe address for DNS pinning. Blocks on the OS resolver
+/// via `spawn_blocking` to avoid blocking the async runtime.
+///
+/// Rejecting *any* resolved address (not just the first) closes split-horizon
+/// DNS attacks: if an attacker can cause one DNS record to resolve to a private
+/// address, the entire request is blocked even when other records are public.
+async fn resolve_and_check_ssrf(host: &str, port: u16) -> Result<std::net::IpAddr, JwksFetchError> {
+    let addr_str = format!("{host}:{port}");
+    let addrs: Vec<std::net::IpAddr> = tokio::task::spawn_blocking(move || {
+        use std::net::ToSocketAddrs;
+        addr_str
+            .to_socket_addrs()
+            .map(|iter| iter.map(|sa| sa.ip()).collect::<Vec<_>>())
+    })
+    .await
+    .map_err(|_| JwksFetchError::NetworkError)?
+    .map_err(|_| JwksFetchError::NetworkError)?;
+
+    if addrs.is_empty() {
+        return Err(JwksFetchError::NetworkError);
+    }
+    for ip in &addrs {
+        if is_private_ip(ip) {
+            return Err(JwksFetchError::InvalidUri);
+        }
+    }
+    Ok(addrs[0])
+}
+
 #[derive(Clone)]
 struct CachedSnapshot {
     key_set: AssertionKeySet,
@@ -139,8 +169,8 @@ pub struct IssuerJwksConfig {
 /// URLs, or raw response content appear in these variants.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)]
 pub enum JwksFetchError {
-    /// Non-HTTPS scheme, embedded credentials, fragment, or bare
-    /// private/reserved IP host.
+    /// Non-HTTPS scheme, embedded credentials, fragment, bare
+    /// private/reserved IP host, or DNS resolved to a private/reserved address.
     #[error("JWKS URI failed safety validation")]
     InvalidUri,
     /// Response body exceeded [`MAX_JWKS_RESPONSE_BYTES`].
@@ -160,8 +190,12 @@ pub enum JwksFetchError {
 /// Sealed injection seam for JWKS HTTP fetching. Only types inside `buzz_auth`
 /// may implement it — external types cannot name the private supertrait.
 ///
-/// Implementations MUST enforce [`MAX_JWKS_RESPONSE_BYTES`] and MUST reject
-/// non-2xx responses.
+/// Implementations MUST:
+/// - resolve hostname targets and reject any private/reserved resolved address;
+/// - deny redirects (3xx responses rejected as `NetworkError`);
+/// - enforce a finite per-fetch deadline ([`JWKS_REQUEST_TIMEOUT_SECS`]);
+/// - enforce [`MAX_JWKS_RESPONSE_BYTES`] via incremental streaming;
+/// - reject non-2xx responses.
 pub trait JwksFetcher: super::verifier::sealed::Sealed + Send + Sync + 'static {
     /// Fetch and return the raw JSON body from the given JWKS URI.
     fn fetch_jwks<'a>(
@@ -170,35 +204,27 @@ pub trait JwksFetcher: super::verifier::sealed::Sealed + Send + Sync + 'static {
     ) -> impl std::future::Future<Output = Result<String, JwksFetchError>> + Send + 'a;
 }
 
-/// Production [`JwksFetcher`] backed by `reqwest`. The default client enforces:
-/// - no redirects (`Policy::none()`) — a redirect to an internal host would
-///   bypass the URI safety check performed at startup;
-/// - a finite per-request deadline ([`JWKS_REQUEST_TIMEOUT_SECS`]).
+/// Production [`JwksFetcher`] backed by `reqwest`. Each call to `fetch_jwks`
+/// builds a dedicated pinned client — no shared connection state between fetches.
 ///
-/// `with_client` accepts a caller-supplied client; the caller must preserve
-/// the no-redirect and finite-timeout invariants. The JWKS URI safety check
-/// is still enforced by [`ProductionJwksSource::new`] regardless.
-#[derive(Clone)]
-pub struct HttpJwksFetcher {
-    client: reqwest::Client,
-}
+/// Per-fetch boundary enforcement:
+/// - hostname DNS is resolved and every address checked against
+///   `buzz_core::network::is_private_ip` before the request is sent;
+/// - the request is pinned to the validated address to prevent DNS rebinding
+///   TOCTOU (the OS resolver is called once per fetch, not once per URL);
+/// - a per-request timeout of [`JWKS_REQUEST_TIMEOUT_SECS`] is applied via
+///   `RequestBuilder::timeout`;
+/// - 3xx responses are rejected as `NetworkError` — redirects are never followed;
+/// - the body is streamed incrementally and stopped at
+///   [`MAX_JWKS_RESPONSE_BYTES`] + 1.
+#[derive(Clone, Debug)]
+pub struct HttpJwksFetcher;
 
 impl HttpJwksFetcher {
-    /// Builds a hardened client: no redirects (`Policy::none()`), finite
-    /// request deadline ([`JWKS_REQUEST_TIMEOUT_SECS`]).
+    /// Builds a new fetcher. Security invariants are enforced per-request in
+    /// `fetch_jwks` — each call constructs a dedicated pinned client.
     pub fn new() -> Self {
-        let client = reqwest::Client::builder()
-            .redirect(reqwest::redirect::Policy::none())
-            .timeout(std::time::Duration::from_secs(JWKS_REQUEST_TIMEOUT_SECS))
-            .build()
-            .expect("HttpJwksFetcher default client build failed");
-        Self { client }
-    }
-
-    /// The caller is responsible for preserving the no-redirect and
-    /// finite-timeout invariants documented on this type.
-    pub fn with_client(client: reqwest::Client) -> Self {
-        Self { client }
+        Self
     }
 }
 
@@ -208,26 +234,40 @@ impl Default for HttpJwksFetcher {
     }
 }
 
-impl std::fmt::Debug for HttpJwksFetcher {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str("HttpJwksFetcher")
-    }
-}
-
 impl super::verifier::sealed::Sealed for HttpJwksFetcher {}
 
 impl JwksFetcher for HttpJwksFetcher {
     async fn fetch_jwks<'a>(&'a self, uri: &'a str) -> Result<String, JwksFetchError> {
-        let response = self
-            .client
+        let parsed = Url::parse(uri).map_err(|_| JwksFetchError::InvalidUri)?;
+        let host = parsed.host_str().ok_or(JwksFetchError::InvalidUri)?;
+        let port = parsed.port_or_known_default().unwrap_or(443);
+
+        // Resolve and check every IP before sending. Pins DNS to the validated
+        // address to prevent rebinding TOCTOU between check and connect.
+        let safe_ip = resolve_and_check_ssrf(host, port).await?;
+
+        // Build a per-request client that:
+        // - denies redirects (a 3xx to an internal host bypasses the URI check);
+        // - has no system proxy (proxy would resolve the original hostname itself);
+        // - pins this request to the validated IP.
+        // The connection pool from self.client is not reused here by design —
+        // DNS pinning requires a fresh client for each pinned address.
+        let pinned_client = reqwest::Client::builder()
+            .redirect(reqwest::redirect::Policy::none())
+            .no_proxy()
+            .resolve(host, std::net::SocketAddr::new(safe_ip, port))
+            .build()
+            .map_err(|_| JwksFetchError::NetworkError)?;
+
+        let response = pinned_client
             .get(uri)
+            .timeout(std::time::Duration::from_secs(JWKS_REQUEST_TIMEOUT_SECS))
             .send()
             .await
             .map_err(|_| JwksFetchError::NetworkError)?;
 
-        // Non-2xx rejected before reading the body. A 3xx here means the
-        // client followed a redirect (default client disallows this); 4xx/5xx
-        // means the endpoint is not serving JWKS.
+        // Reject non-2xx. A 3xx here means our no-redirect policy was somehow
+        // bypassed — treat as a network error.
         if !response.status().is_success() {
             return Err(JwksFetchError::NetworkError);
         }

crates/buzz-auth/src/nip_fi/verifier.rs

@@ -64,7 +64,7 @@ pub(crate) mod sealed {
 /// construction seam: [`verify`] takes no snapshot argument, and this type has
 /// no public constructor, so an external consumer cannot build a snapshot that
 /// labels issuer B's JWKS as issuer A. Building a snapshot (and the source that
-/// serves it) is the trusted configuration act PR 3's JWKS runtime performs at
+/// serves it) is the trusted configuration act the `jwks` runtime performs at
 /// startup, not a per-request or external input.
 ///
 /// The crate-private constructor is a live regression: an external crate that
@@ -90,7 +90,7 @@ impl AssertionKeySet {
     /// generation and a required key-snapshot hard deadline. Rejects a zero
     /// generation, an empty issuer, an empty or oversized key set
     /// ([`MAX_JWKS_KEYS`]), or a non-positive deadline. Crate-private: only the
-    /// trusted in-crate configuration path (PR 3's JWKS runtime) may bind key
+    /// trusted in-crate configuration path (the `jwks` runtime) may bind key
     /// material to an issuer.
     ///
     /// Bounding the key count here is the pre-lookup control (NIP-FI.md:166-171):
@@ -146,7 +146,7 @@ impl fmt::Debug for AssertionKeySet {
 /// instead asks this source for the snapshot bound to the token's
 /// signature-authenticated `iss`. A request-path caller therefore cannot
 /// relabel one issuer's JWKS as another's — the cross-issuer bypass at the old
-/// `verify(token, key_set)` seam. Configuring the source (PR 3's JWKS runtime)
+/// `verify(token, key_set)` seam. Configuring the source (the `jwks` runtime)
 /// is a trusted startup act, not per-request input.
 ///
 /// This trait is sealed via a private supertrait, so it cannot be implemented
@@ -174,7 +174,7 @@ pub trait IssuerKeySource: sealed::Sealed {
 }
 
 /// A fixed issuer→snapshot key source for the in-crate verifier tests,
-/// standing in for PR 3's JWKS runtime. It is `cfg(test)`-only — not behind a
+/// standing in for the `jwks` runtime. It is `cfg(test)`-only — not behind a
 /// downstream-selectable Cargo feature — so no dependent crate can enable it to
 /// reconstruct the authority. An honest source returns only the snapshot bound
 /// to the exact issuer requested, the invariant the real runtime source
@@ -345,7 +345,7 @@ impl<S: IssuerKeySource> FederatedAssertionVerifier<S> {
         // is `evidence_rejected` (403), and this defers a valid one as
         // `authorization_unavailable` (503) so a missing witness never
         // masquerades as rejected evidence, nor invalid input as unavailable
-        // (NIP-FI.md:459-476). PR 3 adds the witness path additively.
+        // (NIP-FI.md:459-476).
         if policy.freshness() == FreshnessClass::CurrentStatus {
             return Err(VerifierError::StatusWitnessUnavailable);
         }
@@ -719,8 +719,8 @@ fn parse_nostr_pubkey_claim(
     }
 }
 
-/// Capture only the claim names the policy reads into a canonical set. For PR 1
-/// the closed set is the `scope` claim, split on ASCII space; unchecked claims
+/// Capture only the claim names the policy reads into a canonical set. The
+/// closed set is the `scope` claim, split on ASCII space; unchecked claims
 /// never enter the result.
 fn capture_capabilities(
     _policy: &IssuerPolicy,