diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 98f3e23..e1f65a5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,3 +36,53 @@ jobs: - name: Test run: nix develop -c cargo test + + # A separate job on purpose. An advisory is published on the RustSec + # timetable, not on this repository's, so a dependency going bad must not + # turn the clippy/test signal red on an unrelated pull request: the two + # answer different questions and are read at different times. + # + # It reads Cargo.lock and the advisory database, and compiles nothing, so + # it needs neither the toolchain cache nor the dev shell's compiler to + # match anything. It does need the network, to fetch the database. + # + # Blocking rather than `continue-on-error`, because an advisory nobody has + # to answer is an advisory nobody reads. + # + # Each `--ignore` below is an advisory in a crate this pod links but does + # not reach, with the path that brings it in and the thing that would + # remove it. They are listed rather than silenced wholesale so that adding + # one is a diff someone reviews, and so a NEW advisory still turns this + # red. Re-check them whenever `oxigraph` or `rudof_lib` moves: + # + # RUSTSEC-2026-0194, RUSTSEC-2026-0195 (quick-xml, both DoS, 7.5) + # Two paths, neither reachable from a request. + # * quick-xml 0.37.5 <- oxrdfxml <- oxrdfio <- oxigraph. This is + # oxigraph's RDF/XML parser, and `rdf::Format::ALL` is closed over + # five media types (Turtle, N-Triples, JSON-LD, TriG, N-Quads) with + # no RDF/XML arm, so nothing in this pod ever constructs one. Adding + # RDF/XML as a supported format makes this advisory live, and the + # `unreachable!()` in `Format::media_type` is what would have to + # change first. + # * quick-xml 0.38.4 <- calamine <- dctap <- rudof_lib. calamine is a + # spreadsheet reader, pulled in because rudof_lib carries DCTAP. + # This pod uses rudof for SHACL validation only, over Turtle. + # Cleared by oxigraph updating oxrdfxml, and by depending on the + # narrower `shacl` crate instead of `rudof_lib`. + # + # RUSTSEC-2026-0235 (rkyv, out-of-bounds read on archive validation) + # rkyv 0.7.46 <- rust_decimal <- rudof_rdf <- rudof_lib. Nothing here + # deserializes an rkyv archive, and no attacker-supplied bytes reach + # one. Cleared by the same narrowing. + audit: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + - uses: DeterminateSystems/nix-installer-action@main + - name: Audit dependencies + run: | + nix develop -c cargo audit \ + --ignore RUSTSEC-2026-0194 \ + --ignore RUSTSEC-2026-0195 \ + --ignore RUSTSEC-2026-0235 diff --git a/README.md b/README.md index 4b5fab2..4bc6b2f 100644 --- a/README.md +++ b/README.md @@ -79,10 +79,13 @@ provider, because this pod has no token endpoint yet. later. The nearest ACL wins outright and does not merge with its ancestors'. The WAC specification requires that, and implementations most often get it wrong. - **Solid-OIDC authentication**: access tokens bound to a key with DPoP (Demonstration of - Proof-of-Possession, RFC 9449), ES256 and RS256 proofs, and the token issuer cross-checked - against the `solid:oidcIssuer` in the WebID profile it claims. A plain `Bearer` credential - is refused: this pod requires the stronger binding, and the cost is that an issuer - configured to hand out non-DPoP tokens will not work against it. + Proof-of-Possession, RFC 9449), ES256 and RS256 for both the proof and the access token's + own signature, and the token issuer cross-checked against the `solid:oidcIssuer` in the + WebID profile it claims. Which algorithm a token is verified under follows the key the + issuer published, never the token's own header, so widening past one algorithm gives a + token no say in how it is checked. A plain `Bearer` credential is refused: this pod + requires the stronger binding, and the cost is that an issuer configured to hand out + non-DPoP tokens will not work against it. - **An SSRF control** on the fetches that happen while a request is still unauthenticated. The token names the URLs, so they are attacker-chosen. The address filter runs inside the DNS resolver, so a name cannot answer public for the check and private for the connection. diff --git a/docs/deployment.md b/docs/deployment.md index 1e4f975..8a121aa 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -184,6 +184,30 @@ match a name the client never typed. With `--op-signing-keys` set it must also be an origin root, since the discovery document it implies hangs off `/.well-known/`. +## Running it under a supervisor + +**Liveness is `GET /.well-known/health`**, unauthenticated, always served, whether or not +the OP is on. It answers `200` with `{"status":"pass"}` as `application/health+json`. + +It reports one thing: this process is up and serving requests. It is deliberately not a +readiness check and touches neither the triple store nor the blob store, because an +unauthenticated probe that queried the store would be a store round trip anyone on the +network could ask for, without a credential and without a rate limit in front of it. A pod +whose store has gone away still answers this route, and fails the requests that need the +store. If you want to alert on the store, alert on those. + +**`SIGTERM` drains.** The pod stops accepting connections and waits for the requests already +in flight before the process exits; `SIGINT` (Ctrl-C) does the same. This matters more here +than it would elsewhere, because a `rocksdb:` directory belongs to one process (below), so +every deployment is a stop and a start with no second replica to cover the gap. Nothing is +corrupted without it, each update sequence is atomic, but a caller learns the outcome of its +own write by having the connection dropped, and a `POST` is not safe to retry blind. + +Give the supervisor a stop timeout longer than your slowest request rather than the default +it ships with (systemd's `TimeoutStopSec` is 90s, Kubernetes' `terminationGracePeriodSeconds` +is 30s), or it will send `SIGKILL` mid-drain and you are back where you started. A second +`SIGTERM` is not special-cased; `SIGKILL` is how you stop waiting. + ## Where the data lives --rdf-store memory (default) triples in this process, gone on restart diff --git a/docs/uri-space.md b/docs/uri-space.md index 49bf6b8..e9ba2d0 100644 --- a/docs/uri-space.md +++ b/docs/uri-space.md @@ -139,13 +139,23 @@ this one is yours: refused by the router: no handler runs, no WAC decision is taken, and a valid credential does not change the answer, the owner's included. It holds whether or not the pod is running as an identity provider. -- **`GET` serves the names the pod implements, and `404`s the rest.** Two names are - implemented, and only while the OP is on (`--op-signing-keys`): - -| Path | Answer | -|---|---| -| `/.well-known/openid-configuration` | the OIDC discovery document, `application/json` | -| `/.well-known/jwks.json` | the public key set, `application/jwk-set+json` | +- **`GET` serves the names the pod implements, and `404`s the rest.** Three names are + implemented. One is served always; the other two only while the OP is on + (`--op-signing-keys`): + +| Path | Answer | When | +|---|---|---| +| `/.well-known/health` | liveness, `{"status":"pass"}` as `application/health+json` | always | +| `/.well-known/openid-configuration` | the OIDC discovery document, `application/json` | OP on | +| `/.well-known/jwks.json` | the public key set, `application/jwk-set+json` | OP on | + + `health` is here rather than at `/health` because `/health` is a name you are entitled to + store a resource at, and a route there would shadow it: the probe would be answered, the + graph would stay, and no write method would reach it. It is not an IANA-registered + well-known name (RFC 8615 §3 asks that names be registered); its shape follows + `draft-inadarei-api-health-check`. It reports that the process is serving and reads + nothing: a pod whose store is unreachable still answers it, and fails the requests that + need the store. Both are served to a request carrying **no credentials at all**, because a verifier reads issuer metadata before it holds anything to present. A request carrying *invalid* credentials diff --git a/flake.nix b/flake.nix index 4038fe2..dafdce7 100644 --- a/flake.nix +++ b/flake.nix @@ -26,6 +26,10 @@ # the whole tree with E0514. clippy rustfmt + # The advisory check CI runs. Here rather than installed in the + # workflow so the version is pinned by flake.lock like every other + # tool, and so `cargo audit` locally is the same command CI runs. + cargo-audit clang libclang pkg-config diff --git a/src/auth/access_token.rs b/src/auth/access_token.rs index 60943a0..9103ebf 100644 --- a/src/auth/access_token.rs +++ b/src/auth/access_token.rs @@ -8,7 +8,7 @@ use base64::engine::general_purpose::URL_SAFE_NO_PAD; use base64::Engine; use josekit::jwk::Jwk; -use josekit::jws::ES256; +use josekit::jws::{JwsVerifier, ES256, RS256}; use josekit::jwt; use serde_json::Value; @@ -28,9 +28,24 @@ pub struct AccessClaims { /// or verification error returns the matching [`AuthError`] rather than a /// silent pass. /// -/// The verification algorithm is pinned to ES256 regardless of what the -/// token's own header claims (its `alg` is never read for this purpose), -/// which forecloses `alg: none` and algorithm-confusion attacks. +/// The verification algorithm comes from the RESOLVED KEY, never from the +/// token's own header, which is what forecloses `alg: none` and +/// algorithm-confusion attacks: a header claiming `none` or `HS256` cannot +/// nominate the verifier it would like to be checked under, and josekit's +/// `decode_with_verifier` separately refuses a token whose header algorithm +/// disagrees with the verifier it was handed. +/// +/// [`verifier_for`] is what maps the key to that algorithm, RS256 for RSA +/// and ES256 for EC P-256, the same two `op::keys::signer_for` signs with. +/// Before that pairing existed this function pinned ES256 outright, which +/// made an RS256 issuer indistinguishable from a forgery and, worse, made +/// this pod reject the tokens its own OP mints from an RSA key: the key set +/// signs RS256 for a key that declares it, publishes `RS256` in the JWKS, +/// and advertises it in `id_token_signing_alg_values_supported`, so the +/// verify side pinning ES256 was a pod that could not read its own +/// signature. ADR-3 settled the identical question one layer down for DPoP +/// proofs ("accepting only ES256 was stricter than the specification +/// without a reason"); this is that decision applied to the access token. pub async fn verify_access_token( token: &str, resolver: &dyn JwksResolver, @@ -56,13 +71,10 @@ pub async fn verify_access_token( let jwk = select_key(&jwks.keys, kid)?; // Verify the JWS signature against the resolved PUBLIC key, with the - // verifier built for the pinned ES256 algorithm, not whatever `alg` - // the header claims. - let verifier = ES256 - .verifier_from_jwk(jwk) - .map_err(|_| AuthError::BadSignature)?; + // verifier the KEY selects, not whatever `alg` the header claims. + let verifier = verifier_for(jwk)?; let (verified_payload, _verified_header) = - jwt::decode_with_verifier(token, &verifier).map_err(|_| AuthError::BadSignature)?; + jwt::decode_with_verifier(token, &*verifier).map_err(|_| AuthError::BadSignature)?; // From here on, every claim comes from `verified_payload`, its // signature has been checked against the issuer's key. @@ -148,9 +160,50 @@ fn decode_segment(segment: &str) -> Result { .map_err(|_| AuthError::Malformed("invalid JSON segment".to_string())) } +/// The verifier for `jwk`, chosen by the key's own type: RS256 for an RSA +/// key, ES256 for an EC P-256 one. The mirror of `op::keys::signer_for`, and +/// the two have to stay in step: a key this pod signs with and cannot verify +/// is a pod that rejects its own tokens. +/// +/// Chosen from the KEY, never from the token header. That is the whole +/// safety argument for widening past one algorithm: the algorithm is a +/// property of the key the issuer published under its own `jwks_uri`, so a +/// token can no more choose it than it can choose the key. A header claiming +/// `none`, `HS256`, or anything else the resolved key does not support is +/// refused by `decode_with_verifier` before a signature is inspected. +/// +/// A key of any other type is [`AuthError::UnsupportedKeyType`] rather than +/// [`AuthError::BadSignature`]: an issuer whose keys this pod cannot handle +/// is a configuration fact an operator can act on, and reporting it as a +/// forgery is exactly what makes it undiagnosable from the log. +fn verifier_for(jwk: &Jwk) -> Result, AuthError> { + let built: Result, _> = match (jwk.key_type(), jwk.curve()) { + ("RSA", _) => RS256 + .verifier_from_jwk(jwk) + .map(|v| Box::new(v) as Box), + ("EC", Some("P-256")) => ES256 + .verifier_from_jwk(jwk) + .map(|v| Box::new(v) as Box), + _ => return Err(AuthError::UnsupportedKeyType), + }; + // A key of a type this pod handles that still will not build a verifier + // (an RSA modulus under josekit's 2048-bit floor, a malformed member) is + // a key nothing can be verified against, which is a refusal, not a + // capability gap. + built.map_err(|_| AuthError::BadSignature) +} + /// Select the JWK to verify against: by `kid` if the header names one, /// else the first signing-capable key (no `use`/`key_ops` restriction, or /// one that explicitly allows verification). +/// +/// Whichever key this returns is also what picks the algorithm +/// ([`verifier_for`]), so a JWKS mixing key types stays coherent: the token +/// is checked under the algorithm of the key it selected, never under one it +/// asked for. The `kid`-less arm is best-effort by nature, and it is the +/// issuer's own JWKS that decides whether it can be: a set holding more than +/// one usable key and a token that names none of them is an issuer not +/// saying which key signed. Every issuer in practice sends `kid`. fn select_key<'a>(keys: &'a [Jwk], kid: Option<&str>) -> Result<&'a Jwk, AuthError> { if let Some(kid) = kid { return keys @@ -295,6 +348,89 @@ mod tests { assert!(verify_access_token(&forged, &resolver, 1_000).await.is_err()); } + /// The regression this pairing exists for. RS256 is the algorithm OIDC + /// Core requires every provider to support, and pinning ES256 rejected + /// every issuer that uses it, as a forged signature. + #[tokio::test] + async fn an_rs256_token_from_an_rsa_issuer_verifies() { + let idp = TestIdp::new_rsa(); + let client = TestClient::new(); + let resolver = StaticJwksResolver::new("https://idp.example/", idp.jwks()); + let jkt = client.jkt(); + let at = idp.mint_access_token("https://alice.example/card#me", &jkt, 9_999_999_999); + + let claims = verify_access_token(&at, &resolver, 1_000).await.unwrap(); + assert_eq!(claims.webid, "https://alice.example/card#me"); + assert_eq!(claims.jkt, jkt); + assert_eq!(claims.issuer, "https://idp.example/"); + } + + /// Widening to a second algorithm must not let a token pick which one it + /// is checked under. The key the resolver hands back decides, so a token + /// signed by the wrong key type is refused rather than routed to a + /// verifier that happens to match its header. + #[tokio::test] + async fn an_es256_token_does_not_verify_against_an_rsa_jwks() { + let ec_idp = TestIdp::new(); + let rsa_idp = TestIdp::new_rsa(); + let client = TestClient::new(); + // The resolver publishes the RSA issuer's key; the token is ES256. + let resolver = StaticJwksResolver::new("https://idp.example/", rsa_idp.jwks()); + let at = ec_idp.mint_access_token("https://alice.example/card#me", &client.jkt(), 9_999_999_999); + + assert!(matches!( + verify_access_token(&at, &resolver, 1_000).await, + Err(AuthError::BadSignature) + )); + } + + /// And the reverse direction, so neither arm is the one carrying the + /// test on its own. + #[tokio::test] + async fn an_rs256_token_does_not_verify_against_an_ec_jwks() { + let ec_idp = TestIdp::new(); + let rsa_idp = TestIdp::new_rsa(); + let client = TestClient::new(); + let resolver = StaticJwksResolver::new("https://idp.example/", ec_idp.jwks()); + let at = rsa_idp.mint_access_token("https://alice.example/card#me", &client.jkt(), 9_999_999_999); + + assert!(matches!( + verify_access_token(&at, &resolver, 1_000).await, + Err(AuthError::BadSignature) + )); + } + + /// A key type this pod cannot verify is a fact about the issuer's + /// configuration, and it must not be reported as a forged signature: + /// that is what makes an operator chase an attack that is not happening. + #[test] + fn an_unsupported_key_type_is_distinguished_from_a_bad_signature() { + // OKP/Ed25519 is a real JWKS key type, and one `verifier_for` has no + // arm for. + let okp = Jwk::generate_ed_key(josekit::jwk::alg::ed::EdCurve::Ed25519).unwrap(); + assert!(matches!( + verifier_for(&okp), + Err(AuthError::UnsupportedKeyType) + )); + + // An EC key on a curve other than P-256 is the same class of answer, + // not an ES256 verification that fails later. + let p384 = Jwk::generate_ec_key(josekit::jwk::alg::ec::EcCurve::P384).unwrap(); + assert!(matches!( + verifier_for(&p384), + Err(AuthError::UnsupportedKeyType) + )); + } + + /// An RSA key below josekit's 2048-bit floor is a key nothing can be + /// verified against, which is a refusal rather than a capability gap: + /// the pod handles RSA, this particular key is unusable. + #[test] + fn an_undersized_rsa_key_is_a_refusal_not_an_unsupported_type() { + let weak = Jwk::generate_rsa_key(1024).unwrap(); + assert!(matches!(verifier_for(&weak), Err(AuthError::BadSignature))); + } + #[test] fn parse_audience_accepts_string_array_or_missing() { assert_eq!( diff --git a/src/auth/authenticate.rs b/src/auth/authenticate.rs index b6f65a8..86faf6d 100644 --- a/src/auth/authenticate.rs +++ b/src/auth/authenticate.rs @@ -101,14 +101,21 @@ pub async fn authenticate( } /// Parse an `Authorization` header of the form `DPoP `, requiring the -/// `DPoP` scheme exactly (a `Bearer` scheme, or any other, is rejected). +/// `DPoP` scheme (a `Bearer` scheme, or any other, is rejected). +/// +/// The scheme is matched case-insensitively, which RFC 9110 §11.1 requires: +/// "the authentication scheme is case-insensitive". Every Solid client in +/// practice sends the `DPoP` spelling, so this refuses nothing it used to +/// accept; it only stops refusing a client that is within its rights. The +/// token itself is case-SENSITIVE and is passed through untouched, it is a +/// compact JWS whose base64url alphabet distinguishes case. fn parse_dpop_scheme(auth_header: Option<&str>) -> Result<&str, AuthError> { let header = auth_header .ok_or_else(|| AuthError::DpopInvalid("missing Authorization header".to_string()))?; let (scheme, token) = header .split_once(' ') .ok_or_else(|| AuthError::Malformed("malformed Authorization header".to_string()))?; - if scheme != "DPoP" { + if !scheme.eq_ignore_ascii_case("DPoP") { return Err(AuthError::DpopInvalid(format!( "unsupported Authorization scheme: {scheme}" ))); @@ -203,6 +210,91 @@ mod tests { .is_err()); } + /// RFC 9110 §11.1: the authentication scheme is case-insensitive. A + /// client spelling it `dpop` or `DPOP` is presenting the same credential + /// and must be answered the same way. + #[tokio::test] + async fn the_dpop_scheme_is_matched_case_insensitively() { + for spelling in ["DPoP", "dpop", "DPOP", "dPoP"] { + let idp = TestIdp::new(); + let client = TestClient::new(); + let resolver = StaticJwksResolver::new("https://idp.example/", idp.jwks()); + let mut webids = StaticWebIdIssuers::new(); + webids.allow("https://alice.example/card#me", "https://idp.example/"); + let cfg = AuthConfig::default(); + let at = idp.mint_access_token( + "https://alice.example/card#me", + &client.jkt(), + 9_999_999_999, + ); + let proof = + client.mint_dpop("https://pod.toph.so/foo", "GET", 1_000, &format!("jti-{spelling}")); + let replay = InMemoryJtiReplayStore::new(); + let deps = AuthDeps { + resolver: &resolver, webid_verifier: &webids, config: &cfg, replay: &replay, + }; + let agent = authenticate(Some(&format!("{spelling} {at}")), Some(&proof), "GET", + "https://pod.toph.so/foo", deps, 1_010).await + .unwrap_or_else(|e| panic!("scheme spelled {spelling:?} must authenticate: {e}")); + assert_eq!(agent, Agent::WebId("https://alice.example/card#me".into())); + } + } + + /// The scheme staying case-insensitive must not make it permissive: + /// `Bearer` is still refused, which is the whole point of requiring + /// proof-of-possession. + #[tokio::test] + async fn a_bearer_scheme_is_still_refused_in_any_spelling() { + for spelling in ["Bearer", "bearer", "BEARER"] { + let idp = TestIdp::new(); + let client = TestClient::new(); + let resolver = StaticJwksResolver::new("https://idp.example/", idp.jwks()); + let mut webids = StaticWebIdIssuers::new(); + webids.allow("https://alice.example/card#me", "https://idp.example/"); + let cfg = AuthConfig::default(); + let at = idp.mint_access_token( + "https://alice.example/card#me", + &client.jkt(), + 9_999_999_999, + ); + let proof = client.mint_dpop("https://pod.toph.so/foo", "GET", 1_000, "jti-bearer"); + let replay = InMemoryJtiReplayStore::new(); + let deps = AuthDeps { + resolver: &resolver, webid_verifier: &webids, config: &cfg, replay: &replay, + }; + let r = authenticate(Some(&format!("{spelling} {at}")), Some(&proof), "GET", + "https://pod.toph.so/foo", deps, 1_010).await; + assert!(matches!(r, Err(AuthError::DpopInvalid(_))), + "{spelling} must not be accepted as a credential"); + } + } + + /// End to end through `authenticate`, not only `verify_access_token`: an + /// issuer signing RS256 is an ordinary OIDC provider and must reach + /// `Agent::WebId` like any other. + #[tokio::test] + async fn an_rs256_issuer_authenticates_end_to_end() { + let idp = TestIdp::new_rsa(); + let client = TestClient::new(); + let resolver = StaticJwksResolver::new("https://idp.example/", idp.jwks()); + let mut webids = StaticWebIdIssuers::new(); + webids.allow("https://alice.example/card#me", "https://idp.example/"); + let cfg = AuthConfig::default(); + let at = idp.mint_access_token( + "https://alice.example/card#me", + &client.jkt(), + 9_999_999_999, + ); + let proof = client.mint_dpop("https://pod.toph.so/foo", "GET", 1_000, "jti-rs256"); + let replay = InMemoryJtiReplayStore::new(); + let deps = AuthDeps { + resolver: &resolver, webid_verifier: &webids, config: &cfg, replay: &replay, + }; + let agent = authenticate(Some(&format!("DPoP {at}")), Some(&proof), "GET", + "https://pod.toph.so/foo", deps, 1_010).await.unwrap(); + assert_eq!(agent, Agent::WebId("https://alice.example/card#me".into())); + } + #[tokio::test] async fn issuer_not_authorized_by_webid_is_rejected() { let idp = crate::auth::testsupport::TestIdp::new(); diff --git a/src/auth/cache.rs b/src/auth/cache.rs new file mode 100644 index 0000000..1a665e6 --- /dev/null +++ b/src/auth/cache.rs @@ -0,0 +1,129 @@ +//! The bound every pre-authentication cache in this module is held to. +//! +//! Both `http_jwks` and `webid_issuer` cache what they resolve, and both are +//! keyed by a string taken off an unverified credential: the token's `iss` +//! for one, its `webid` claim for the other. Neither key is the pod's to +//! choose, and with no `--trusted-issuer` allowlist configured neither is +//! even constrained, anyone running their own IdP writes both. A cache keyed +//! that way and never swept is memory an anonymous caller allocates. +//! +//! One derivation, used by both, because the two got different answers when +//! each held its own: `webid_issuer` was bounded and `http_jwks` was not, and +//! nothing said so. `http_jwks` is also the one reached FIRST, since a JWKS +//! is resolved before the WebID-issuer binding is checked, so the unbounded +//! half was the half in front. + +use std::collections::HashMap; +use std::time::{Duration, Instant}; + +/// Cap on entries in any one of those caches. +/// +/// Sized for the deployment this pod is for rather than for a public +/// federation hub: a pod serves one owner and the handful of issuers and +/// WebIDs that owner's applications actually use, so a thousand live entries +/// is far past the working set and still small enough that a full sweep is +/// nothing. Exceeding it is the signal that the keys are no longer a working +/// set at all. +pub(super) const MAX_CACHE_ENTRIES: usize = 1024; + +/// Insert into a bounded cache, making room first if the map is at +/// [`MAX_CACHE_ENTRIES`]: expired entries go first, and if that frees nothing +/// the least recently fetched entry is evicted. +/// +/// Eviction rather than refusal because the entry being inserted is the one +/// just proven live; the worst case is a cache miss, never unbounded memory. +/// +/// `stamp` reads the `Instant` out of whatever the value happens to be, which +/// is what lets one function serve a positive cache (a payload beside its +/// timestamp) and a negative one (a bare timestamp) without either growing a +/// wrapper type for the sake of this call. +pub(super) fn insert_bounded( + map: &mut HashMap, + key: String, + value: V, + ttl: Duration, + stamp: fn(&V) -> Instant, +) { + if map.len() >= MAX_CACHE_ENTRIES && !map.contains_key(&key) { + map.retain(|_, v| stamp(v).elapsed() < ttl); + + if map.len() >= MAX_CACHE_ENTRIES { + if let Some(oldest) = map + .iter() + .min_by_key(|(_, v)| stamp(v)) + .map(|(k, _)| k.clone()) + { + map.remove(&oldest); + } + } + } + + map.insert(key, value); +} + +#[cfg(test)] +mod tests { + use super::*; + + const TTL: Duration = Duration::from_secs(300); + + /// The property both callers depend on, asserted once here rather than + /// once per caller: the map does not grow past the cap however many + /// distinct keys arrive. + #[test] + fn a_bounded_cache_does_not_grow_past_its_cap() { + let mut map: HashMap = HashMap::new(); + for i in 0..MAX_CACHE_ENTRIES + 50 { + insert_bounded(&mut map, format!("k{i}"), Instant::now(), TTL, |at| *at); + } + assert_eq!(map.len(), MAX_CACHE_ENTRIES); + } + + /// Eviction must not fire on a key already present: refreshing an entry + /// replaces it rather than displacing someone else's. + #[test] + fn refreshing_an_existing_entry_evicts_nothing() { + let mut map: HashMap = HashMap::new(); + for i in 0..MAX_CACHE_ENTRIES { + insert_bounded( + &mut map, + format!("k{i}"), + (0, Instant::now()), + TTL, + |(_, at)| *at, + ); + } + + insert_bounded( + &mut map, + "k0".to_string(), + (7, Instant::now()), + TTL, + |(_, at)| *at, + ); + + assert_eq!(map.len(), MAX_CACHE_ENTRIES); + assert_eq!(map["k0"].0, 7, "the refreshed value is the one held"); + } + + /// An expired entry is what the sweep is supposed to reclaim, so a map + /// full of them makes room without evicting anything live. + #[test] + fn expired_entries_are_reclaimed_before_a_live_one_is_evicted() { + let mut map: HashMap = HashMap::new(); + for i in 0..MAX_CACHE_ENTRIES { + insert_bounded(&mut map, format!("k{i}"), Instant::now(), TTL, |at| *at); + } + // A zero TTL makes every existing entry expired at the next insert, + // which is the sweep's own condition rather than a simulated clock. + insert_bounded( + &mut map, + "fresh".to_string(), + Instant::now(), + Duration::ZERO, + |at| *at, + ); + assert_eq!(map.len(), 1, "the sweep reclaimed the expired entries"); + assert!(map.contains_key("fresh")); + } +} diff --git a/src/auth/http_jwks.rs b/src/auth/http_jwks.rs index 4e466aa..c6f75fa 100644 --- a/src/auth/http_jwks.rs +++ b/src/auth/http_jwks.rs @@ -16,6 +16,7 @@ use josekit::jwk::Jwk; use serde_json::Value; use tokio::sync::RwLock; +use super::cache::insert_bounded; use super::jwks::{Jwks, JwksResolver}; use super::safe_fetch::{guarded_get, FetchPolicy, GuardedClient}; use super::AuthError; @@ -33,6 +34,16 @@ const NEGATIVE_CACHE_TTL: Duration = Duration::from_secs(30); /// An HTTP `JwksResolver` for production use: resolves an issuer's signing /// keys via OIDC discovery, caching the result per issuer for `CACHE_TTL`. +/// +/// Both caches are bounded through [`insert_bounded`], the same derivation +/// `webid_issuer` uses. That matters more here than it does there. Both +/// modules key their caches by a string taken off an unverified credential, +/// but this one's key is the token's `iss`, read by `peek_untrusted_issuer` +/// before any signature is checked, and with no `--trusted-issuer` allowlist +/// configured nothing constrains it at all: a distinct `iss` per request is +/// a fresh entry per request. A JWKS is also resolved *before* the +/// WebID-issuer binding runs, so while this half was unbounded it was the +/// unbounded map sitting in front of the bounded one. pub struct HttpJwksResolver { client: GuardedClient, cache: RwLock>, @@ -126,7 +137,8 @@ impl JwksResolver for HttpJwksResolver { match self.fetch(issuer).await { Ok(jwks) => { - self.cache.write().await.insert( + insert_bounded( + &mut *self.cache.write().await, issuer.to_string(), ( Jwks { @@ -134,15 +146,20 @@ impl JwksResolver for HttpJwksResolver { }, Instant::now(), ), + CACHE_TTL, + |(_, at)| *at, ); self.negative_cache.write().await.remove(issuer); Ok(jwks) } Err(e) => { - self.negative_cache - .write() - .await - .insert(issuer.to_string(), Instant::now()); + insert_bounded( + &mut *self.negative_cache.write().await, + issuer.to_string(), + Instant::now(), + NEGATIVE_CACHE_TTL, + |at| *at, + ); Err(e) } } @@ -229,6 +246,34 @@ mod tests { assert_eq!(jwks_hits.load(Ordering::SeqCst), 1); } + /// The bound, exercised through `resolve` rather than through the helper + /// alone: what an anonymous caller actually drives is this path, and it + /// is the negative cache that fills, one entry per distinct `iss`. + /// + /// The production `FetchPolicy` is what makes this hermetic: an + /// `http://` URL naming a loopback address is refused by the SSRF guard + /// before a socket is opened, so every one of these resolves fails + /// without a network round trip, which is exactly the cheap failure an + /// attacker would be driving. + #[tokio::test] + async fn the_negative_cache_does_not_grow_past_its_cap() { + let resolver = HttpJwksResolver::with_policy(FetchPolicy::default()); + + for i in 0..crate::auth::cache::MAX_CACHE_ENTRIES + 50 { + let issuer = format!("http://127.0.0.1:1/attacker-chosen-{i}/"); + assert!( + resolver.resolve(&issuer).await.is_err(), + "the fetch policy must refuse this without a network call" + ); + } + + assert_eq!( + resolver.negative_cache.read().await.len(), + crate::auth::cache::MAX_CACHE_ENTRIES, + "an unverified `iss` must not be able to allocate without bound" + ); + } + #[tokio::test] async fn unknown_host_is_unknown_issuer() { let resolver = HttpJwksResolver::with_policy(FetchPolicy::permissive()); diff --git a/src/auth/mod.rs b/src/auth/mod.rs index c7cad74..9e6b6b6 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -9,6 +9,7 @@ pub mod access_token; pub mod agent; pub mod authenticate; +mod cache; pub mod config; pub mod dpop; pub mod http_jwks; @@ -52,6 +53,8 @@ pub enum AuthError { Binding, #[error("no signing key available for this token")] MissingKey, + #[error("issuer's signing key is of a type this pod cannot verify")] + UnsupportedKeyType, #[error("blocked outbound fetch: {0}")] FetchBlocked(String), #[error("webid does not authorize this token's issuer")] diff --git a/src/auth/testsupport.rs b/src/auth/testsupport.rs index eddbb39..ce999a9 100644 --- a/src/auth/testsupport.rs +++ b/src/auth/testsupport.rs @@ -15,8 +15,9 @@ use sha2::{Digest, Sha256}; use super::jwks::Jwks; -/// A stand-in external Solid-OIDC IdP: an EC P-256 signing keypair, and a -/// method to mint access tokens as it would issue them. +/// A stand-in external Solid-OIDC IdP: a signing keypair (EC P-256 signing +/// ES256 ([`TestIdp::new`]) or RSA signing RS256 ([`TestIdp::new_rsa`])), and +/// a method to mint access tokens as it would issue them. pub struct TestIdp { private_jwk: Jwk, public_jwk: Jwk, @@ -24,7 +25,22 @@ pub struct TestIdp { impl TestIdp { pub fn new() -> Self { - let private_jwk = Jwk::generate_ec_key(EcCurve::P256).expect("generate IdP EC key"); + Self::from_private(Jwk::generate_ec_key(EcCurve::P256).expect("generate IdP EC key")) + } + + /// An IdP whose signing key is RSA, so its access tokens are signed + /// `RS256`. This is the ordinary shape for an OIDC provider (RS256 is + /// the algorithm OIDC Core requires every implementation to support), + /// and it is also what this pod's own OP mints from an RSA key file, so + /// this is how the verify side is held to reading both. + /// + /// 2048 bits because that is the floor `josekit` enforces on both the + /// signing and the verifying side. + pub fn new_rsa() -> Self { + Self::from_private(Jwk::generate_rsa_key(2048).expect("generate IdP RSA key")) + } + + fn from_private(private_jwk: Jwk) -> Self { let public_jwk = private_jwk.to_public_key().expect("derive IdP public key"); Self { private_jwk, @@ -32,6 +48,23 @@ impl TestIdp { } } + /// The signer this IdP's key type implies, the same pairing + /// `auth::access_token::verifier_for` makes on the other side. + fn signer(&self) -> Box { + match self.public_jwk.key_type() { + "RSA" => Box::new( + RS256 + .signer_from_jwk(&self.private_jwk) + .expect("build IdP RS256 signer"), + ), + _ => Box::new( + ES256 + .signer_from_jwk(&self.private_jwk) + .expect("build IdP ES256 signer"), + ), + } + } + /// The IdP's published (public) keys, as a `JwksResolver` would resolve them. pub fn jwks(&self) -> Jwks { Jwks { @@ -56,9 +89,7 @@ impl TestIdp { exp_unix: i64, aud: &[&str], ) -> String { - let signer = ES256 - .signer_from_jwk(&self.private_jwk) - .expect("build IdP signer"); + let signer = self.signer(); let mut header = JwsHeader::new(); header.set_token_type("JWT"); @@ -84,7 +115,7 @@ impl TestIdp { .expect("set aud claim"); } - jwt::encode_with_signer(&payload, &header, &signer).expect("sign access token") + jwt::encode_with_signer(&payload, &header, &*signer).expect("sign access token") } } diff --git a/src/auth/webid_issuer.rs b/src/auth/webid_issuer.rs index 1cf698f..de8fa32 100644 --- a/src/auth/webid_issuer.rs +++ b/src/auth/webid_issuer.rs @@ -23,6 +23,7 @@ use oxigraph::model::{NamedOrBlankNode, Term}; use tokio::sync::RwLock; use super::safe_fetch::{guarded_get, FetchPolicy, GuardedClient}; +use super::cache::insert_bounded; use super::AuthError; use crate::rdf::Format; @@ -93,11 +94,6 @@ const CACHE_TTL: Duration = Duration::from_secs(120); /// is one fresh outbound fetch to an attacker-chosen address. const NEGATIVE_CACHE_TTL: Duration = Duration::from_secs(30); -/// Cap on entries in either cache. Both are keyed by the token's `webid` -/// claim, which is attacker-influenced under the conditions described at -/// [`NEGATIVE_CACHE_TTL`], so neither may grow without bound. -const MAX_CACHE_ENTRIES: usize = 1024; - /// Production `WebIdIssuerVerifier`: dereferences the WebID's profile /// document over HTTP and checks it for the `solid:oidcIssuer` triple, /// caching each profile's declared issuer list for [`CACHE_TTL`]. @@ -187,36 +183,6 @@ impl HttpWebIdIssuers { } } -/// Insert into a bounded cache, making room first if the map is at -/// [`MAX_CACHE_ENTRIES`]: expired entries go first, and if that frees nothing -/// the least recently fetched entry is evicted. -/// -/// Eviction rather than refusal because the entry being inserted is the one -/// just proven live; the worst case is a cache miss, never unbounded memory. -fn insert_bounded( - map: &mut HashMap, - key: String, - value: V, - ttl: Duration, - stamp: fn(&V) -> Instant, -) { - if map.len() >= MAX_CACHE_ENTRIES && !map.contains_key(&key) { - map.retain(|_, v| stamp(v).elapsed() < ttl); - - if map.len() >= MAX_CACHE_ENTRIES { - if let Some(oldest) = map - .iter() - .min_by_key(|(_, v)| stamp(v)) - .map(|(k, _)| k.clone()) - { - map.remove(&oldest); - } - } - } - - map.insert(key, value); -} - #[async_trait] impl WebIdIssuerVerifier for HttpWebIdIssuers { /// A profile that parsed but does not declare `issuer` is `Ok(false)`; a @@ -444,57 +410,6 @@ mod tests { assert_eq!(hits.load(Ordering::SeqCst), 1); } - /// Both caches are keyed by the token's `webid` claim, which an attacker - /// running their own IdP can set freely when no `trusted_issuers` - /// allowlist is configured. Neither may grow without bound. - #[test] - fn a_bounded_cache_does_not_grow_past_its_cap() { - let mut map: HashMap, Instant)> = HashMap::new(); - - for i in 0..MAX_CACHE_ENTRIES + 50 { - insert_bounded( - &mut map, - format!("https://example.test/{i}#me"), - (vec!["https://idp.example/".to_string()], Instant::now()), - CACHE_TTL, - |(_, at)| *at, - ); - } - - assert_eq!(map.len(), MAX_CACHE_ENTRIES); - } - - /// Eviction must not fire on a key already present: re-fetching the same - /// WebID replaces its entry rather than displacing someone else's. - #[test] - fn refreshing_an_existing_entry_evicts_nothing() { - let mut map: HashMap, Instant)> = HashMap::new(); - - for i in 0..MAX_CACHE_ENTRIES { - insert_bounded( - &mut map, - format!("https://example.test/{i}#me"), - (vec![], Instant::now()), - CACHE_TTL, - |(_, at)| *at, - ); - } - - insert_bounded( - &mut map, - "https://example.test/0#me".to_string(), - (vec!["https://idp.example/".to_string()], Instant::now()), - CACHE_TTL, - |(_, at)| *at, - ); - - assert_eq!(map.len(), MAX_CACHE_ENTRIES); - assert_eq!( - map["https://example.test/0#me"].0, - vec!["https://idp.example/".to_string()] - ); - } - /// Spin up a local profile-document server serving the SAME /// `solid:oidcIssuer` declaration as [`spawn_profile_server`], but as /// **JSON-LD** (expanded form) with `Content-Type: application/ld+json` diff --git a/src/config.rs b/src/config.rs index a040300..8499d73 100644 --- a/src/config.rs +++ b/src/config.rs @@ -20,7 +20,11 @@ use crate::space::{GraphName, SpaceError, StorageSpace}; pub struct InvalidOwnerWebId; #[derive(Parser, Debug, Clone)] -#[command(name = "quadpod", about = "A SPARQL-authoritative Solid pod")] +// `version` reads `CARGO_PKG_VERSION`, so `quadpod --version` answers from +// the manifest rather than from a string someone has to remember to bump. An +// operator filing a bug against a binary they did not build has no other way +// to say which one they are running. +#[command(name = "quadpod", version, about = "A SPARQL-authoritative Solid pod")] pub struct Config { /// Public base URI of this pod. Absolute, with a trailing slash. All /// minted URLs and the DPoP `htu` derive from this, never from the socket. diff --git a/src/http.rs b/src/http.rs index 538d846..66418a6 100644 --- a/src/http.rs +++ b/src/http.rs @@ -88,23 +88,58 @@ pub fn router(state: AppState) -> Router { .with_state(state) } -/// The reserved `/.well-known/` space. Implemented names, OP on: -/// `openid-configuration` (`application/json`) and `jwks.json` -/// (`application/jwk-set+json`, public members only). Everything else, -/// the bare forms included, is 404. +/// The reserved `/.well-known/` space. `health` is served whatever the pod's +/// configuration; with the OP on, `openid-configuration` +/// (`application/json`) and `jwks.json` (`application/jwk-set+json`, public +/// members only) join it. Everything else, the bare forms included, is 404. /// -/// No WAC guard and no store read: both documents are public by design, so -/// there is no target to authorize and nothing here can fail. +/// No WAC guard and no store read: every document here is public by design, +/// so there is no target to authorize and nothing here can fail. +/// +/// **Why liveness lives in the reserved space rather than at `/health`.** +/// A route at `/health` would shadow the resource a user is entitled to +/// store at `/health`: the router would answer the probe, the storage path +/// would keep the graph, and no write method would reach it, so the resource +/// would be unreadable and undeletable. That is the exact hazard +/// `crate::space` already refuses `/.well-known/` for, and inventing a +/// second reserved segment to avoid it costs a name in every pod's URI space +/// forever. Here the segment is already reserved, already GET-only, and +/// already the one exemption `tests/route_coverage.rs` carries, so liveness +/// arrives without widening any of the three. +/// +/// The cost is honest: `health` is not an IANA-registered well-known URI, +/// and RFC 8615 §3 asks that names be registered. The shape follows +/// `draft-inadarei-api-health-check` (a `status` member, the +/// `application/health+json` media type) so the name is at least the one an +/// operator would guess. async fn handle_well_known( State(state): State, rest: Option>, ) -> Response { - let Some(keys) = &state.op_keys else { - return StatusCode::NOT_FOUND.into_response(); - }; // Unwrapped one layer first: `Option>::as_deref` stops at // `&String`, which no `&str` pattern below would match. let name = rest.map(|Path(name)| name); + + // Answered before the OP gate: a verify-only pod is a running pod, and a + // supervisor that reads 404 as "down" would restart it forever. + // + // Deliberately liveness and not readiness: it reports that this process + // is up and serving, and it touches neither store. A probe that queried + // the store would be an unauthenticated store round trip on a route with + // no rate limit in front of it, which is a denial-of-service lever handed + // out for an operational convenience. A pod whose store is unreachable + // fails the requests that need it, with a `500` that says so. + if name.as_deref() == Some("health") { + return ( + [(header::CONTENT_TYPE, "application/health+json")], + r#"{"status":"pass"}"#, + ) + .into_response(); + } + + let Some(keys) = &state.op_keys else { + return StatusCode::NOT_FOUND.into_response(); + }; match name.as_deref() { Some("openid-configuration") => { axum::Json(crate::op::discovery::document(&state.space, keys)).into_response() diff --git a/src/http/tests/well_known.rs b/src/http/tests/well_known.rs index 9077bd5..c15eb67 100644 --- a/src/http/tests/well_known.rs +++ b/src/http/tests/well_known.rs @@ -71,6 +71,53 @@ async fn the_jwks_route_serves_public_members_only_with_its_media_type() { std::fs::remove_file(&p).ok(); } +/// Liveness is served whatever the pod's configuration, and without +/// credentials: a supervisor holds none, and a verify-only pod (no OP) is +/// still a running pod. Answering 404 there is what would make a restart +/// loop out of a healthy process. +#[tokio::test] +async fn health_is_served_unauthenticated_with_the_op_off() { + let f = fixture().await; + let res = get(&f.app, "/.well-known/health").await; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers()[header::CONTENT_TYPE], "application/health+json"); + let bytes = http_body_util::BodyExt::collect(res.into_body()) + .await + .unwrap() + .to_bytes(); + let doc: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(doc["status"], "pass"); +} + +/// And turning the OP on takes nothing away from it: the two discovery +/// documents join `health` rather than displacing it. +#[tokio::test] +async fn health_is_served_with_the_op_on_too() { + let (f, _op, p) = fixture_with_op().await; + let (status, doc) = get_json(&f.app, "/.well-known/health").await; + assert_eq!(status, StatusCode::OK); + assert_eq!(doc.unwrap()["status"], "pass"); + std::fs::remove_file(&p).ok(); +} + +/// The probe reads liveness and nothing else. It must not become a way to +/// ask an unauthenticated question about the store, which is what a +/// readiness check here would be, so its body carries one member and no +/// version, no issuer, no counts. +#[tokio::test] +async fn health_discloses_nothing_beyond_liveness() { + let (f, _op, p) = fixture_with_op().await; + let (_, doc) = get_json(&f.app, "/.well-known/health").await; + let obj = doc.unwrap(); + let obj = obj.as_object().expect("a JSON object"); + assert_eq!( + obj.keys().collect::>(), + vec!["status"], + "liveness says only that this process is serving" + ); + std::fs::remove_file(&p).ok(); +} + #[tokio::test] async fn an_unimplemented_name_is_404_and_the_bare_forms_too() { let (f, _op, p) = fixture_with_op().await; @@ -121,6 +168,7 @@ async fn every_write_is_405( "/.well-known/oauth-authorization-server", "/.well-known/openid-configuration", "/.well-known/jwks.json", + "/.well-known/health", "/.well-known", "/.well-known/", ] { diff --git a/src/main.rs b/src/main.rs index ff941dc..aa0afeb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -88,5 +88,40 @@ async fn main() { ).await.expect("provision root ACL"); let listener = tokio::net::TcpListener::bind(cfg.listen).await.unwrap(); tracing::info!("quadpod listening on {}", cfg.listen); - axum::serve(listener, router(state)).await.unwrap(); + axum::serve(listener, router(state)) + .with_graceful_shutdown(shutdown_signal()) + .await + .unwrap(); + tracing::info!("quadpod stopped"); +} + +/// Resolves on `SIGTERM` or `SIGINT`, which is what lets `axum::serve` stop +/// accepting connections and then wait for the requests already in flight. +/// +/// It matters more here than it would elsewhere. ADR-7 gives a `rocksdb:` +/// directory to exactly one process, so every deployment is a stop and a +/// start with no second replica to absorb the gap, and without this the +/// supervisor's `SIGTERM` kills in-flight requests at whatever `.await` they +/// were parked on. Nothing is corrupted by that (ADR-2 makes each update +/// sequence atomic, so a killed write either took effect whole or not at +/// all), but a caller learns the outcome of its own `PUT` by having the +/// connection dropped, which for a non-idempotent `POST` it cannot safely +/// retry. Draining turns that into an ordinary response. +/// +/// `SIGTERM` is what a supervisor sends (systemd, Kubernetes, `docker +/// stop`); `SIGINT` is Ctrl-C in a terminal. A second signal is not handled, +/// and deliberately: the runtime's default disposition is restored for +/// neither, so an operator who wants to stop waiting can send `SIGKILL`, +/// which no process can trap anyway. +async fn shutdown_signal() { + use tokio::signal::unix::{signal, SignalKind}; + + let mut term = signal(SignalKind::terminate()).expect("install SIGTERM handler"); + let mut int = signal(SignalKind::interrupt()).expect("install SIGINT handler"); + + let received = tokio::select! { + _ = term.recv() => "SIGTERM", + _ = int.recv() => "SIGINT", + }; + tracing::info!("{received} received, draining in-flight requests"); } diff --git a/src/op/keys.rs b/src/op/keys.rs index 62d71f3..fda3c76 100644 --- a/src/op/keys.rs +++ b/src/op/keys.rs @@ -235,6 +235,20 @@ fn signer_for(jwk: &josekit::jwk::Jwk) -> Result