diff --git a/bobbin/crates/xrpc/src/auth.rs b/bobbin/crates/xrpc/src/auth.rs index acd0402e..fbebc2b3 100644 --- a/bobbin/crates/xrpc/src/auth.rs +++ b/bobbin/crates/xrpc/src/auth.rs @@ -1,3 +1,25 @@ +//! Inbound atproto service-auth verification. +//! +//! This is deliberately hand-rolled rather than `jacquard-axum`'s `ExtractServiceAuth`, which +//! cannot serve bobbin as of 0.12.1. What has to land upstream before we can swap: +//! +//! - `DidDocResponse::parse` drops a slingshot mini-doc's `signingKey` (`verification_method: +//! None`), and jacquard-axum errors when there are no verification methods. Since our resolver +//! uses `PlcSource::Slingshot`, that is every `did:plc` issuer. See [`signing_key`]. +//! - `require_lxm` only checks that `lxm` is *present*; nothing binds it to the route, so a token +//! minted for one method is accepted at another. See [`jwt_check_claims`]. +//! - `ServiceAuthClaims::validate` reads `chrono::Utc::now()` directly, so there is no way to +//! inject a clock, and it checks only `exp` — no skew, no `iat`, no maximum lifetime. +//! - No policy hook for the issuer host, so `iss` can steer resolution at private addresses. +//! See [`ServiceAuth::guard_issuer_host`]. +//! - `InMemoryReplayStore` is an LRU-capped cache, so an evicted `jti` becomes replayable again. +//! Ours fails closed. See [`ServiceAuth::record_jti`]. +//! - `ServiceAuthError` has no variant for a replayed token, a resolution failure that should be +//! a 502 rather than a 401, a non-bearer authorization header, or the three lifetime checks +//! above; those stay [`XrpcError`] strings here. +//! - `ServiceAuthClaims::require_method` takes `&Nsid`, which is awkward to reach from +//! `Nsid`; it wants to be generic over `BosStr`. + use std::sync::Arc; use axum::extract::{FromRequestParts, MatchedPath}; @@ -5,17 +27,23 @@ use axum::http::HeaderMap; use axum::http::header::AUTHORIZATION; use axum::http::request::Parts; use bobbin_runtime::{Clock, RuntimeHasher, private_host_reason}; -use jacquard_common::DefaultStr; -use jacquard_common::service_auth::{self, ParsedJwt}; +use jacquard_common::service_auth::{self, ParsedJwt, ServiceAuthError}; use jacquard_common::types::crypto::{KeyCodec, PublicKey as CryptoKey}; use jacquard_common::types::did::Did; use jacquard_common::types::nsid::Nsid; -use jacquard_identity::resolver::{IdentityResolver, MiniDoc}; +use jacquard_common::types::string::DidService; +use jacquard_common::{CowStr, DefaultStr, format_smolstr}; +use jacquard_identity::resolver::{DidDocResponse, IdentityResolver, MiniDoc}; use scc::HashMap as SccMap; use url::Url; use crate::{AppState, Directory, XrpcError}; +/// A failure jacquard has no variant for, so it cannot survive the eventual swap as-is. +fn malformed(message: &'static str) -> ServiceAuthError { + ServiceAuthError::MalformedToken(CowStr::new_static(message)) +} + const CLOCK_SKEW_SECS: i64 = 60; const MAX_TOKEN_LIFETIME_SECS: i64 = 300; const MAX_NONCE_BYTES: usize = 256; @@ -29,7 +57,9 @@ pub struct ServiceAuth { audience: Did, clock: Arc, directory: Arc, - /// (issuer, jti) -> unix seconds after which the entry may be pruned. + /// (issuer, jti) -> unix seconds after which the entry may be pruned. This is what should be + /// handed to `ServiceAuthConfig::with_replay_store` if we ever adopt jacquard-axum; its own + /// store fails open. seen_jti: SccMap<(Did, String), i64, RuntimeHasher>, issuer_jti_counts: SccMap, usize, RuntimeHasher>, } @@ -56,15 +86,17 @@ impl ServiceAuth { } /// Returns the verified issuer, or the error the caller should see. - async fn verify_service_jwt( + /// + /// Claim checks run before the network resolve, so an unauthenticated caller cannot make us + /// fetch on a token that was never going to pass. jacquard-axum resolves first. + async fn verify_service_auth( &self, token: &str, method: &Nsid, ) -> Result, XrpcError> { - let parsed = service_auth::parse_jwt(token) - .map_err(|e| XrpcError::AuthRequired(format!("malformed token: {e}")))?; + let parsed = service_auth::parse_jwt(token)?; if !parsed.header().typ.as_str().eq_ignore_ascii_case("JWT") { - return Err(XrpcError::AuthRequired("token is not a JWT".into())); + return Err(malformed("token typ is not JWT").into()); } let issuer = parsed.claims().iss.clone(); let now_micros = self.clock.now_unix_micros(); @@ -75,8 +107,7 @@ impl ServiceAuth { self.guard_issuer_host(&issuer)?; let key = issuer_key(&self.directory, &issuer).await?; - service_auth::verify_signature(&parsed, &key) - .map_err(|e| XrpcError::AuthRequired(format!("signature: {e}")))?; + service_auth::verify_signature(&parsed, &key)?; // always guard by single use self.record_jti(&issuer, jti, parsed.claims().exp, now)?; @@ -176,8 +207,20 @@ async fn issuer_key( issuer: &Did, ) -> Result { let resp = directory.resolve_did_doc(issuer).await.map_err(|e| { + // an unreachable plc directory is not the caller's fault; jacquard-axum returns 401 here XrpcError::UpstreamUnavailable(format!("resolving issuer {}: {e}", issuer.as_str())) })?; + signing_key(&resp, issuer) +} + +/// The issuer's verifying key, from an already-fetched document. +/// +/// Split out from [`issuer_key`] so the mini-doc branch below is testable without a resolver: it +/// is the one thing jacquard-axum gets wrong, so it must not regress silently. +fn signing_key( + resp: &DidDocResponse, + issuer: &Did, +) -> Result { // parse_validated enforces doc.id == issuer, which also covers the mini-doc branch below let doc = resp .parse_validated() @@ -203,80 +246,91 @@ async fn issuer_key( KeyCodec::Secp256k1 => service_auth::PublicKey::from_k256_bytes(&key.bytes), KeyCodec::P256 => service_auth::PublicKey::from_p256_bytes(&key.bytes), other => { - return Err(XrpcError::AuthRequired(format!( + return Err(ServiceAuthError::Crypto(CowStr::Owned(format_smolstr!( "unsupported issuer key codec {other:?}" - ))); + ))) + .into()); } } - .map_err(|e| XrpcError::AuthRequired(format!("issuer key: {e}"))) + .map_err(XrpcError::from) } +/// `now_unix` is passed in rather than read from a clock so this stays pure and testable, which +/// is also why we don't call `ServiceAuthClaims::validate`. fn jwt_check_claims( parsed: &ParsedJwt, audience: &Did, method: &Nsid, now_unix: i64, -) -> Result<(), XrpcError> { +) -> Result<(), ServiceAuthError> { let claims = parsed.claims(); if claims.aud.audience().as_str() != audience.as_str() { - return Err(XrpcError::AuthRequired(format!( - "token addressed to {}, expected {}", - claims.aud.as_str(), - audience.as_str(), - ))); + return Err(ServiceAuthError::AudienceMismatch { + expected: audience.clone(), + actual: DidService::new_owned(claims.aud.as_str()) + .map_err(|_| malformed("token aud is not a did"))?, + }); } if claims.exp.saturating_add(CLOCK_SKEW_SECS) < now_unix { - return Err(XrpcError::AuthRequired(format!( - "token expired at {}, now {now_unix}", - claims.exp, - ))); + return Err(ServiceAuthError::Expired { + exp: claims.exp, + now: now_unix, + }); } if claims.iat.saturating_sub(CLOCK_SKEW_SECS) > now_unix { - return Err(XrpcError::AuthRequired(format!( - "token issued at {}, now {now_unix}", - claims.iat, - ))); + return Err(malformed("token was issued in the future")); } if claims.exp < claims.iat { - return Err(XrpcError::AuthRequired("token expires before issue".into())); + return Err(malformed("token expires before it was issued")); } if claims.exp.saturating_sub(claims.iat) > MAX_TOKEN_LIFETIME_SECS { - return Err(XrpcError::AuthRequired(format!( - "token lifetime exceeds {MAX_TOKEN_LIFETIME_SECS}s", - ))); + return Err(malformed("token lifetime exceeds the 300s maximum")); } - let bound = claims.lxm.as_ref().map(|lxm| lxm.as_str()); - if bound != Some(method.as_str()) { - return Err(XrpcError::AuthRequired(format!( - "token bound to {}, expected {}", - bound.unwrap_or("nothing"), - method.as_str(), - ))); + // binding to the route is what jacquard-axum is missing; `require_lxm` only checks presence + if claims.lxm.as_ref().map(|lxm| lxm.as_str()) != Some(method.as_str()) { + return Err(ServiceAuthError::MethodMismatch { + expected: method.clone(), + actual: claims.lxm.clone(), + }); } Ok(()) } -fn jwt_nonce(parsed: &ParsedJwt) -> Result { +fn jwt_nonce(parsed: &ParsedJwt) -> Result { let jti = parsed .claims() .jti .as_ref() - .ok_or_else(|| XrpcError::AuthRequired("token has no jti".into()))?; + .ok_or(ServiceAuthError::MissingField("jti"))?; if jti.as_str().len() > MAX_NONCE_BYTES { - return Err(XrpcError::AuthRequired("token jti is oversized".into())); + return Err(malformed("token jti is oversized")); } Ok(jti.as_str().to_owned()) } -fn bearer(headers: &HeaderMap) -> Option<&str> { - let value = headers.get(AUTHORIZATION)?.to_str().ok()?; - let (scheme, token) = value.split_once(' ')?; - scheme - .eq_ignore_ascii_case("Bearer") - .then(|| token.trim()) - .filter(|token| !token.is_empty()) +/// `Ok(None)` is anonymous. A header that is present but not a bearer token is an error rather +/// than a silent downgrade, matching jacquard-axum; unlike it, we accept any scheme casing. +fn bearer(headers: &HeaderMap) -> Result, XrpcError> { + let Some(value) = headers.get(AUTHORIZATION) else { + return Ok(None); + }; + let not_bearer = + || XrpcError::AuthRequired("authorization header is not a bearer token".into()); + let (scheme, token) = value + .to_str() + .map_err(|_| not_bearer())? + .split_once(' ') + .ok_or_else(not_bearer)?; + if !scheme.eq_ignore_ascii_case("Bearer") { + return Err(not_bearer()); + } + let token = token.trim(); + if token.is_empty() { + return Err(not_bearer()); + } + Ok(Some(token)) } /// The route path is the lexicon method, so there is no per-handler nsid constant to drift. @@ -293,7 +347,7 @@ fn matched_nsid(parts: &Parts) -> Result, XrpcError> { /// The verified caller, if it presented a token. `None` is anonymous, which stays a valid way to /// read public data; a token that fails verification is a 401 rather than a silent downgrade. -pub(crate) struct Viewer(pub(crate) Option>); +pub struct Viewer(pub Option>); impl FromRequestParts for Viewer { type Rejection = XrpcError; @@ -302,13 +356,13 @@ impl FromRequestParts for Viewer { parts: &mut Parts, state: &AppState, ) -> Result { - let Some(token) = bearer(&parts.headers) else { + let Some(token) = bearer(&parts.headers)? else { return Ok(Self(None)); }; let method = matched_nsid(parts)?; state .service_auth - .verify_service_jwt(token, &method) + .verify_service_auth(token, &method) .await .map(|did| { tracing::debug!(viewer = %did.as_str(), method = %method.as_str(), "service auth ok"); @@ -326,7 +380,7 @@ mod tests { use super::*; - fn auth(allow_private: bool) -> ServiceAuth { + fn auth() -> ServiceAuth { let hasher = RuntimeHasher::from_entropy(&SeededEntropy::new(7)); ServiceAuth::new( "bobbin.example", @@ -337,8 +391,10 @@ mod tests { .unwrap() } - /// `check_claims` never reaches the signature, so an unsigned token is enough for it. - fn addressed_to(aud: &str) -> ParsedJwt { + const METHOD: &str = "sh.tangled.feed.getTimeline"; + + /// `jwt_check_claims` never reaches the signature, so an unsigned token is enough for it. + fn token(aud: &str, iat: i64, exp: i64) -> ParsedJwt { let b64 = |value: &serde_json::Value| { URL_SAFE_NO_PAD.encode(serde_json::to_vec(value).expect("test claims serialize")) }; @@ -346,21 +402,70 @@ mod tests { let claims = b64(&json!({ "iss": "did:plc:issuer", "aud": aud, - "iat": 1_000, - "exp": 1_060, + "iat": iat, + "exp": exp, "jti": "nonce-1", - "lxm": "sh.tangled.feed.getTimeline", + "lxm": METHOD, })); let signature = URL_SAFE_NO_PAD.encode([0u8; 64]); service_auth::parse_jwt(&format!("{header}.{claims}.{signature}")) .expect("hand-built jwt parses") } + fn addressed_to(aud: &str) -> ParsedJwt { + token(aud, 1_000, 1_060) + } + + fn method() -> Nsid { + Nsid::new_owned(METHOD).unwrap() + } + + fn issuer() -> Did { + Did::new_owned("did:plc:issuer").unwrap() + } + + /// A resolver response, so the key path is testable without touching the network. + fn doc_response(body: serde_json::Value, requested: &Did) -> DidDocResponse { + DidDocResponse { + buffer: axum::body::Bytes::from(serde_json::to_vec(&body).unwrap()), + status: http::StatusCode::OK, + requested: Some(requested.clone()), + } + } + + /// A real secp256k1 multikey, borrowed from jacquard's own service-auth tests. + const K256_MULTIKEY: &str = "zQ3shNBS3N4EB3vX5G1HoxFkS8tDLFXUHaV85rHQZgVM88rM5"; + const ED25519_MULTIKEY: &str = "z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK"; + + fn full_doc(did: &str, key_type: &str, multikey: &str) -> serde_json::Value { + json!({ + "@context": ["https://www.w3.org/ns/did/v1"], + "id": did, + "verificationMethod": [{ + "id": format!("{did}#atproto"), + "type": key_type, + "controller": did, + "publicKeyMultibase": multikey, + }], + }) + } + + fn mini_doc(did: &str, signing_key: &str) -> serde_json::Value { + json!({ + "did": did, + // `.example` is a disallowed tld, so a handle there fails to deserialize + "handle": "issuer.test", + "pds": "https://pds.test", + "signingKey": signing_key, + }) + } + #[test] fn a_service_id_fragment_still_addresses_us() { - let auth = auth(false); - let method = Nsid::new_owned("sh.tangled.feed.getTimeline").unwrap(); - let check = |aud: &str| jwt_check_claims(&addressed_to(aud), &auth.audience, &method, 1_000); + let auth = auth(); + let method = method(); + let check = + |aud: &str| jwt_check_claims(&addressed_to(aud), &auth.audience, &method, 1_000); check("did:web:bobbin.example").unwrap(); check("did:web:bobbin.example#bobbin_appview") @@ -372,9 +477,149 @@ mod tests { ); } + #[test] + fn claim_lifetimes_are_bounded_in_both_directions() { + let auth = auth(); + let method = method(); + let check = |iat, exp, now| { + jwt_check_claims( + &token("did:web:bobbin.example", iat, exp), + &auth.audience, + &method, + now, + ) + }; + + check(1_000, 1_060, 1_030).expect("a live token in its window"); + check(1_000, 1_060, 1_000 - CLOCK_SKEW_SECS).expect("issued within the skew tolerance"); + check(1_000, 1_060, 1_060 + CLOCK_SKEW_SECS).expect("expired within the skew tolerance"); + + assert!( + check(1_000, 1_060, 1_060 + CLOCK_SKEW_SECS + 1).is_err(), + "expired beyond the skew tolerance" + ); + assert!( + check(9_000, 9_060, 1_000).is_err(), + "issued too far in the future" + ); + assert!(check(1_060, 1_000, 1_030).is_err(), "expires before issue"); + assert!( + check(1_000, 1_000 + MAX_TOKEN_LIFETIME_SECS + 1, 1_030).is_err(), + "lifetime beyond the maximum" + ); + } + + #[test] + fn a_token_must_be_bound_to_the_route_it_arrives_on() { + let auth = auth(); + let check = |method: &str| { + jwt_check_claims( + &addressed_to("did:web:bobbin.example"), + &auth.audience, + &Nsid::new_owned(method).unwrap(), + 1_000, + ) + }; + + check(METHOD).unwrap(); + assert!( + check("sh.tangled.repo.getRepo").is_err(), + "a token minted for one method must not be accepted at another; this is the check \ + jacquard-axum is missing" + ); + } + + #[test] + fn a_nonce_is_single_use_per_issuer() { + let auth = auth(); + let issuer = issuer(); + let other = Did::new_owned("did:plc:other").unwrap(); + + auth.record_jti(&issuer, "nonce-1".into(), 1_060, 1_000) + .expect("first use"); + assert!( + auth.record_jti(&issuer, "nonce-1".into(), 1_060, 1_000) + .is_err(), + "replayed nonce" + ); + auth.record_jti(&other, "nonce-1".into(), 1_060, 1_000) + .expect("two issuers may legally pick the same nonce"); + } + + #[test] + fn one_issuer_cannot_fill_the_replay_store() { + let auth = auth(); + let issuer = issuer(); + for n in 0..MAX_JTI_PER_ISSUER { + auth.record_jti(&issuer, format!("nonce-{n}"), 1_060, 1_000) + .expect("within the issuer's share"); + } + + // fails closed: saturation must not become a way to disable replay checks + let err = auth + .record_jti(&issuer, "one-too-many".into(), 1_060, 1_000) + .expect_err("beyond the issuer's share"); + assert!(matches!(err, XrpcError::Overloaded), "got {err:?}"); + + // ...but the entries are prunable once their tokens are long expired + auth.record_jti(&issuer, "after-the-horizon".into(), 9_999, 5_000) + .expect("pruning past the horizon frees the issuer's share"); + } + + #[test] + fn signing_keys_come_from_full_docs_and_slingshot_mini_docs() { + let issuer = issuer(); + let did = issuer.as_str(); + + signing_key( + &doc_response(full_doc(did, "Multikey", K256_MULTIKEY), &issuer), + &issuer, + ) + .expect("a full did doc with an atproto multikey"); + + signing_key( + &doc_response(mini_doc(did, K256_MULTIKEY), &issuer), + &issuer, + ) + .expect( + "a slingshot mini-doc carries signingKey but no verification methods; jacquard drops \ + it, and this is the path every did:plc issuer takes", + ); + } + + #[test] + fn signing_keys_are_refused_when_absent_unusable_or_for_another_did() { + let issuer = issuer(); + let did = issuer.as_str(); + + let no_key = json!({ "@context": ["https://www.w3.org/ns/did/v1"], "id": did }); + assert!( + signing_key(&doc_response(no_key, &issuer), &issuer).is_err(), + "a doc publishing no signing key" + ); + assert!( + signing_key( + &doc_response(full_doc(did, "Multikey", ED25519_MULTIKEY), &issuer), + &issuer + ) + .is_err(), + "ed25519 is not a service-auth signing algorithm" + ); + + let other = Did::new_owned("did:plc:other").unwrap(); + assert!( + signing_key( + &doc_response(full_doc(did, "Multikey", K256_MULTIKEY), &other), + &other + ) + .is_err(), + "a doc that answers for a different did than the one we asked about" + ); + } + #[test] fn private_issuer_hosts_are_refused() { - let strict = auth(false); + let strict = auth(); let guard = |host: &str| strict.guard_issuer_host(&did_web(host).unwrap()); guard("bobbin.example").unwrap(); @@ -385,11 +630,6 @@ mod tests { assert!(guard("127.0.0.1").is_err()); assert!(guard("10.0.0.1").is_err()); assert!(guard("169.254.169.254").is_err(), "cloud metadata endpoint"); - - let localinfra = auth(true); - localinfra - .guard_issuer_host(&did_web("localhost:8090").unwrap()) - .expect("localinfra issues did:web:localhost"); } #[test] @@ -415,11 +655,21 @@ mod tests { #[test] fn bearer_is_scheme_insensitive_and_rejects_empties() { let mut headers = HeaderMap::new(); + assert_eq!( + bearer(&headers).unwrap(), + None, + "no header at all is anonymous" + ); + headers.insert(AUTHORIZATION, "bearer tok.en.sig ".parse().unwrap()); - assert_eq!(bearer(&headers), Some("tok.en.sig")); + assert_eq!(bearer(&headers).unwrap(), Some("tok.en.sig")); + + // present but unusable is a 401, not a silent downgrade to anonymous headers.insert(AUTHORIZATION, "Bearer ".parse().unwrap()); - assert_eq!(bearer(&headers), None); + assert!(bearer(&headers).is_err()); headers.insert(AUTHORIZATION, "Basic abc".parse().unwrap()); - assert_eq!(bearer(&headers), None); + assert!(bearer(&headers).is_err()); + headers.insert(AUTHORIZATION, "nonsense".parse().unwrap()); + assert!(bearer(&headers).is_err()); } } diff --git a/bobbin/crates/xrpc/src/lib.rs b/bobbin/crates/xrpc/src/lib.rs index ada97c58..719ffd90 100644 --- a/bobbin/crates/xrpc/src/lib.rs +++ b/bobbin/crates/xrpc/src/lib.rs @@ -106,7 +106,7 @@ mod feed; mod filter; mod recordpath; -pub use auth::ServiceAuth; +pub use auth::{ServiceAuth, Viewer}; pub use backpressure::{ HeavyLimiter, HeavyPermit, MaxInFlight, PerRequestAnonBytes, PressureVerdict, ReservedFloor, }; @@ -747,6 +747,10 @@ pub enum XrpcError { InvalidParams(String), #[error("authentication required: {0}")] AuthRequired(String), + /// A service-auth token failed verification. Typed so the jwt-level checks speak jacquard's + /// vocabulary; adopting `jacquard-axum` later replaces the producer, not this variant. + #[error(transparent)] + Auth(#[from] jacquard_common::service_auth::ServiceAuthError), #[error("record not found")] NotFound, #[error("upstream unavailable: {0}")] @@ -777,7 +781,9 @@ impl IntoResponse for XrpcError { fn into_response(self) -> Response { let (status, error) = match &self { Self::InvalidParams(_) => (StatusCode::BAD_REQUEST, "InvalidRequest"), - Self::AuthRequired(_) => (StatusCode::UNAUTHORIZED, "AuthenticationRequired"), + Self::AuthRequired(_) | Self::Auth(_) => { + (StatusCode::UNAUTHORIZED, "AuthenticationRequired") + } Self::NotFound => (StatusCode::NOT_FOUND, "RecordNotFound"), Self::UpstreamUnavailable(_) => (StatusCode::BAD_GATEWAY, "UpstreamFailed"), Self::UpstreamGone(_) => (StatusCode::BAD_GATEWAY, "UpstreamGone"), @@ -1635,7 +1641,10 @@ fn drop_unhydratable( } }, Err( - err @ (XrpcError::Internal(_) | XrpcError::Overloaded | XrpcError::AuthRequired(_)), + err @ (XrpcError::Internal(_) + | XrpcError::Overloaded + | XrpcError::AuthRequired(_) + | XrpcError::Auth(_)), ) => Err(err), } }