From 3aa5780ecd944ab474f6a9c6b36aa94d2165e201 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:08 -0400 Subject: [PATCH 01/32] fix(trogon-aauth-verify): make a verified signature cover the body it was sent with The signature bound only the Content-Digest header string, so the digest a peer signed was never checked against the bytes that arrived. Signed-off-by: Yordis Prieto --- .../aauth/trogon-aauth-verify/src/http_pop.rs | 82 ++++++++++++- .../trogon-aauth-verify/src/http_pop/tests.rs | 116 ++++++++++++++---- 2 files changed, 173 insertions(+), 25 deletions(-) diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs index 7114cb7b5..12e9b6df5 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs @@ -25,6 +25,17 @@ //! - `aauth-mission` (OPTIONAL): the raw `AAuth-Mission` header field value, //! required to be covered only when the request carries that header. //! +//! ## Body integrity +//! +//! Covering `content-digest` binds the *header value* to the signature; on +//! its own that says nothing about the bytes in `body`. Whenever the header +//! is present this verifier therefore also recomputes SHA-256 over the +//! request body and compares, exactly as [`crate::nats_pop`] does for its +//! own profile. Without that second half a captured request could have its +//! body replaced (or removed entirely, which drops it out of the +//! has-a-body branch) while the untouched digest header kept the signature +//! valid. +//! //! `Signature-Input`, `Signature`, and `Signature-Key` are parsed only in the //! single-member Dictionary shape the draft's examples use throughout //! (`sig=(...)`, `sig=:...:`, `sig=jwt;jwt="..."`) -- this verifier does not @@ -48,7 +59,10 @@ //! NATS PoP profile ([`crate::nats_pop`]), which mints an explicit nonce //! header -- the HTTP profile has none to reuse. +use base64::Engine; +use base64::engine::general_purpose::{STANDARD, STANDARD_NO_PAD, URL_SAFE_NO_PAD}; use jsonwebtoken::{Algorithm, DecodingKey, crypto::verify, jwk::Jwk}; +use sha2::{Digest, Sha256}; use trogon_identity_types::aauth::headers; use crate::constants::HTTP_SECURITY_HEADERS; @@ -159,6 +173,15 @@ pub enum HttpPopError { /// versa the header is covered but absent. #[error("content-digest header required but missing")] MissingContentDigest, + /// `Content-Digest` was present but carried no `sha-256` entry, or that + /// entry was not a decodable Byte Sequence. Refused rather than skipped, + /// because skipping an unparseable digest is indistinguishable from + /// having no body integrity at all. + #[error("content-digest header has no decodable sha-256 entry")] + UnsupportedContentDigest, + /// `Content-Digest` did not match SHA-256 over the request body. + #[error("content-digest does not match the request body")] + ContentDigestMismatch, /// The `AAuth-Mission` header is covered by the signature but absent /// from the request, or present but absent from coverage while a mission /// claim is expected -- see [`crate::mission`] for claim-level checks. @@ -257,8 +280,9 @@ impl HttpPopVerifier { /// (Server)" (#verification): extracts `Signature-Key`, `Signature-Input`, /// `Signature`; verifies required component coverage; verifies the JWT /// layer of the presented token (agent or auth); verifies the HTTP - /// Message Signature against the token's `cnf.jwk`; and enforces - /// freshness plus the replay tuple from (#freshness-and-replay). + /// Message Signature against the token's `cnf.jwk`; recomputes + /// `Content-Digest` over the body; and enforces freshness plus the replay + /// tuple from (#freshness-and-replay). pub async fn verify(&self, req: &HttpRequest) -> Result { if self.max_skew_secs < 0 { return Err(HttpPopError::NegativeMaxSkew(self.max_skew_secs)); @@ -326,6 +350,7 @@ impl HttpPopVerifier { let base = build_signature_base(req, &parsed_input)?; verify_signature_with_jwk(&cnf_jwk, base.as_bytes(), &sig_b64)?; + verify_content_digest(req)?; let replay_key = format!( "http-pop:{jkt}:{}:{}:{}:{}", @@ -502,6 +527,59 @@ fn verify_covered_components(req: &HttpRequest, components: &[String]) -> Result Ok(()) } +/// Recomputes SHA-256 over the request body and compares it against the +/// `Content-Digest` header. +/// +/// Keyed on the header being *present* rather than on +/// [`HttpRequest::has_body`], so that stripping the body from a captured +/// request does not slip past by falling out of the has-a-body branch. The +/// header cannot itself be stripped: [`verify_covered_components`] requires it +/// to be covered whenever a body is present, and [`build_signature_base`] +/// fails to reconstruct the base for a covered field that is missing. +fn verify_content_digest(req: &HttpRequest) -> Result<(), HttpPopError> { + let Some(raw) = req.header(headers::CONTENT_DIGEST) else { + return Ok(()); + }; + let supplied = parse_sha256_content_digest(raw).ok_or(HttpPopError::UnsupportedContentDigest)?; + let expected = Sha256::digest(req.body.as_deref().unwrap_or(&[])); + if supplied.as_slice() != expected.as_slice() { + return Err(HttpPopError::ContentDigestMismatch); + } + Ok(()) +} + +/// Extracts the raw `sha-256` digest bytes from an RFC 9530 `Content-Digest` +/// value, a Structured Fields Dictionary of algorithm keys to Byte Sequences +/// (`sha-256=::`). Other algorithm entries are skipped rather than +/// rejected, since a peer is free to send additional ones. +fn parse_sha256_content_digest(raw: &str) -> Option> { + for member in raw.split(',') { + let Some((algorithm, encoded)) = member.split_once('=') else { + continue; + }; + if !algorithm.trim().eq_ignore_ascii_case("sha-256") { + continue; + } + let inner = encoded.trim().strip_prefix(':')?.strip_suffix(':')?; + return decode_base64_any_alphabet(inner); + } + None +} + +/// Decodes a digest that may arrive in any of the base64 alphabets seen in +/// practice. RFC 8941 Byte Sequences are standard padded base64, which is +/// what a conformant third-party client sends, while +/// [`crate::nats_pop::content_digest_sha256`] emits URL-safe unpadded. Both +/// have to decode to the same 32 bytes, so comparison happens on the decoded +/// digest rather than on the header string. +fn decode_base64_any_alphabet(encoded: &str) -> Option> { + STANDARD + .decode(encoded) + .or_else(|_| STANDARD_NO_PAD.decode(encoded)) + .or_else(|_| URL_SAFE_NO_PAD.decode(encoded)) + .ok() +} + /// Builds the RFC 9421 canonical signature base for the supported component /// subset (see module docs). For each covered component identifier, in the /// order given by `Signature-Input`, appends `"": \n`; then a diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs index 7585dc5c3..73f3db31a 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs @@ -170,37 +170,107 @@ async fn verify_accepts_ed25519_agent_presenter() { assert!(matches!(result, VerifiedPresenter::Agent(_))); } +/// Signs a request carrying `body` with a matching, covered `Content-Digest`. +fn signed_body_request( + fixture: &crate::test_support::EcFixture, + jwt: &str, + body: &[u8], + digest: String, +) -> HttpRequest { + let mut req = base_request(&signature_key_header(jwt)); + req.body = Some(body.to_vec()); + req.headers.push((headers::CONTENT_DIGEST.to_string(), digest)); + let mut components = REQUIRED_COMPONENTS.to_vec(); + components.push("content-digest"); + sign_request(fixture, &mut req, 1000, &components); + req +} + #[tokio::test(flavor = "current_thread")] -async fn verify_rejects_tampered_body_content_digest_not_recomputed() { +async fn verify_accepts_body_matching_content_digest() { let fixture = p256_fixture("k1"); let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); let verifier = verifier_at(jwks, 1000, "resource.example"); - let body = br#"{"scope":"data.read"}"#.to_vec(); - let digest = crate::nats_pop::content_digest_sha256(&body); - let mut req = base_request(&signature_key_header(&jwt)); - req.body = Some(body); - req.headers.push((headers::CONTENT_DIGEST.to_string(), digest)); - let mut components = REQUIRED_COMPONENTS.to_vec(); - components.push("content-digest"); - sign_request(&fixture, &mut req, 1000, &components); + let body = br#"{"scope":"data.read"}"#; + let digest = crate::nats_pop::content_digest_sha256(body); + let req = signed_body_request(&fixture, &jwt, body, digest); + + let result = verifier.verify(&req).await.expect("matching digest verifies"); + assert!(matches!(result, VerifiedPresenter::Agent(_))); +} + +#[tokio::test(flavor = "current_thread")] +async fn verify_accepts_rfc9530_padded_base64_content_digest() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // RFC 8941 Byte Sequences are standard padded base64, which is what a + // conformant third-party client sends; our own emitter uses URL-safe + // unpadded. Both encode the same digest and both must be accepted. + let body = br#"{"scope":"data.read"}"#; + let padded = format!("sha-256=:{}:", STANDARD.encode(Sha256::digest(body))); + let req = signed_body_request(&fixture, &jwt, body, padded); + + verifier.verify(&req).await.expect("padded base64 digest verifies"); +} - // Tamper with the body after signing without updating content-digest. - // The digest header itself is covered and unmodified, so the signature - // still verifies -- catching a body/digest mismatch is the caller's - // responsibility (recompute and compare), since #covered-components only - // requires that content-digest be *covered*, not that this crate - // recompute it against a body it is never given as a signed component. +#[tokio::test(flavor = "current_thread")] +async fn verify_rejects_tampered_body_against_covered_content_digest() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + let body = br#"{"scope":"data.read"}"#; + let digest = crate::nats_pop::content_digest_sha256(body); + let mut req = signed_body_request(&fixture, &jwt, body, digest); + + // Swap the body after signing, leaving the covered digest header intact. + // The signature still verifies over the untouched header, so only + // recomputing the digest against the body catches this. req.body = Some(br#"{"scope":"data.write"}"#.to_vec()); - let recomputed = crate::nats_pop::content_digest_sha256(req.body.as_ref().unwrap()); - let supplied = req.header(headers::CONTENT_DIGEST).unwrap().to_string(); - assert_ne!(recomputed, supplied, "tampering must be visible via digest mismatch"); - - verifier - .verify(&req) - .await - .expect("signature over untouched digest header still verifies"); + + let err = verifier.verify(&req).await.unwrap_err(); + assert!(matches!(err, HttpPopError::ContentDigestMismatch)); +} + +#[tokio::test(flavor = "current_thread")] +async fn verify_rejects_stripped_body_against_covered_content_digest() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + let body = br#"{"scope":"data.read"}"#; + let digest = crate::nats_pop::content_digest_sha256(body); + let mut req = signed_body_request(&fixture, &jwt, body, digest); + + // Dropping the body entirely falls out of the has-a-body branch, so a + // coverage-only check would let it through. + req.body = None; + + let err = verifier.verify(&req).await.unwrap_err(); + assert!(matches!(err, HttpPopError::ContentDigestMismatch)); +} + +#[tokio::test(flavor = "current_thread")] +async fn verify_rejects_content_digest_without_sha256_entry() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // Only algorithms this verifier cannot check: refused rather than + // skipped, since skipping is indistinguishable from no body integrity. + let body = br#"{"scope":"data.read"}"#; + let req = signed_body_request(&fixture, &jwt, body, "sha-512=:YWJj:".to_string()); + + let err = verifier.verify(&req).await.unwrap_err(); + assert!(matches!(err, HttpPopError::UnsupportedContentDigest)); } #[tokio::test(flavor = "current_thread")] From d7362d9ebebbd1446864b8b9385715319e7c7942 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:08 -0400 Subject: [PATCH 02/32] fix(trogon-aauth-verify): honor what a publisher says its key is for A JWKS publisher that sets use, key_ops, or alg has stated the key's purpose, and ignoring that lets a key be conscripted into a job it was never offered for. Signed-off-by: Yordis Prieto --- .../aauth/trogon-aauth-verify/src/token.rs | 60 +++++++++++- .../trogon-aauth-verify/src/token/tests.rs | 94 +++++++++++++++++++ 2 files changed, 152 insertions(+), 2 deletions(-) diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs index edc96446d..2a7b742a6 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs @@ -2,7 +2,9 @@ use jsonwebtoken::{ Algorithm, DecodingKey, Validation, decode, decode_header, - jwk::{AlgorithmParameters, EllipticCurve, Jwk, JwkSet}, + jwk::{ + AlgorithmParameters, CommonParameters, EllipticCurve, Jwk, JwkSet, KeyAlgorithm, KeyOperations, PublicKeyUse, + }, }; use serde::Deserialize; use trogon_identity_types::aauth::{AgentClaims, AuthClaims, ResourceClaims, TYP_AGENT, TYP_AUTH, TYP_RESOURCE}; @@ -406,8 +408,62 @@ fn pick_jwk<'a>(set: &'a JwkSet, alg: Algorithm, kid: Option<&str>) -> Option<&' None } +/// A JWK may verify a signature under `alg` only when its key material matches +/// that algorithm's family *and* the key's own advertised purpose permits +/// verification. +/// +/// RFC 7517 makes `use` (section 4.2), `key_ops` (section 4.3), and `alg` +/// (section 4.4) optional, so an absent member stays permissive: the AAuth +/// draft does not require them and a federated deployment resolves JWKS +/// documents this platform did not publish. But a publisher that *does* set +/// them has declared what the key is for, and honoring that declaration stops +/// a key published for encryption, or pinned to a different algorithm, from +/// being conscripted into signature verification merely because its curve +/// lines up. fn jwk_compatible_with_alg(jwk: &Jwk, alg: Algorithm) -> bool { - match (&jwk.algorithm, alg) { + jwk_purpose_permits_verification(&jwk.common) + && jwk_declared_alg_matches(&jwk.common, alg) + && jwk_material_matches_alg(&jwk.algorithm, alg) +} + +/// Rejects a key whose `use` names something other than signatures, or whose +/// `key_ops` enumerates operations without including `verify`. RFC 7517 says +/// the two members SHOULD NOT appear together; when a publisher sets both +/// anyway, each is checked independently and either one can reject. +fn jwk_purpose_permits_verification(common: &CommonParameters) -> bool { + if let Some(public_key_use) = &common.public_key_use + && !matches!(public_key_use, PublicKeyUse::Signature) + { + return false; + } + if let Some(key_operations) = &common.key_operations + && !key_operations.iter().any(|op| matches!(op, KeyOperations::Verify)) + { + return false; + } + true +} + +/// Rejects a key whose own `alg` names a different algorithm than the one the +/// JWT header claims, closing the gap where a publisher pins a key to one +/// algorithm and a token asserts another from the same key family. +fn jwk_declared_alg_matches(common: &CommonParameters, alg: Algorithm) -> bool { + let Some(declared) = common.key_algorithm else { + return true; + }; + matches!( + (declared, alg), + (KeyAlgorithm::ES256, Algorithm::ES256) + | (KeyAlgorithm::ES384, Algorithm::ES384) + | (KeyAlgorithm::EdDSA, Algorithm::EdDSA) + ) +} + +/// The key-material check: `kty`/`crv` must be the pair the algorithm is +/// defined over, which is what blocks cross-family confusion between the +/// three algorithms [`parse_typ`] admits. +fn jwk_material_matches_alg(parameters: &AlgorithmParameters, alg: Algorithm) -> bool { + match (parameters, alg) { (AlgorithmParameters::EllipticCurve(ec), Algorithm::ES256) => ec.curve == EllipticCurve::P256, (AlgorithmParameters::EllipticCurve(ec), Algorithm::ES384) => ec.curve == EllipticCurve::P384, (AlgorithmParameters::OctetKeyPair(okp), Algorithm::EdDSA) => okp.curve == EllipticCurve::Ed25519, diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/token/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/token/tests.rs index ba1e7e97d..436620ffd 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/token/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/token/tests.rs @@ -95,6 +95,100 @@ fn jwk_compatible_with_alg_rejects_mismatched_family() { assert!(!jwk_compatible_with_alg(&ec.jwk, Algorithm::EdDSA)); } +#[test] +fn jwk_compatible_with_alg_rejects_encryption_use() { + let mut ec = p256_fixture("p256-k1"); + ec.jwk.common.public_key_use = Some(jsonwebtoken::jwk::PublicKeyUse::Encryption); + assert!( + !jwk_compatible_with_alg(&ec.jwk, Algorithm::ES256), + "a key published for encryption must not verify signatures" + ); +} + +#[test] +fn jwk_compatible_with_alg_rejects_unrecognized_use() { + let mut ec = p256_fixture("p256-k1"); + ec.jwk.common.public_key_use = Some(jsonwebtoken::jwk::PublicKeyUse::Other("tls".into())); + assert!(!jwk_compatible_with_alg(&ec.jwk, Algorithm::ES256)); +} + +#[test] +fn jwk_compatible_with_alg_rejects_key_ops_without_verify() { + let mut ec = p256_fixture("p256-k1"); + ec.jwk.common.public_key_use = None; + ec.jwk.common.key_operations = Some(vec![jsonwebtoken::jwk::KeyOperations::Encrypt]); + assert!(!jwk_compatible_with_alg(&ec.jwk, Algorithm::ES256)); +} + +#[test] +fn jwk_compatible_with_alg_accepts_key_ops_containing_verify() { + let mut ec = p256_fixture("p256-k1"); + ec.jwk.common.public_key_use = None; + ec.jwk.common.key_operations = Some(vec![ + jsonwebtoken::jwk::KeyOperations::Verify, + jsonwebtoken::jwk::KeyOperations::Encrypt, + ]); + assert!(jwk_compatible_with_alg(&ec.jwk, Algorithm::ES256)); +} + +#[test] +fn jwk_compatible_with_alg_rejects_declared_alg_mismatch() { + // Material lines up (EC/P-256 is exactly what ES256 is defined over), so + // only the key's own `alg` member can catch this mislabeling. + let mut ec = p256_fixture("p256-k1"); + ec.jwk.common.key_algorithm = Some(jsonwebtoken::jwk::KeyAlgorithm::ES384); + assert!(!jwk_compatible_with_alg(&ec.jwk, Algorithm::ES256)); +} + +#[test] +fn jwk_compatible_with_alg_rejects_declared_encryption_alg() { + // `alg: "ECDH-ES"` is a key-agreement algorithm; jsonwebtoken folds any + // algorithm it does not model into UNKNOWN_ALGORITHM, which must not be + // treated as "compatible with whatever the header claims". + let mut ec = p256_fixture("p256-k1"); + ec.jwk.common.key_algorithm = Some(jsonwebtoken::jwk::KeyAlgorithm::UNKNOWN_ALGORITHM); + assert!(!jwk_compatible_with_alg(&ec.jwk, Algorithm::ES256)); +} + +#[test] +fn jwk_compatible_with_alg_accepts_absent_purpose_members() { + // `use`, `key_ops` and `alg` are all optional in RFC 7517; a JWKS that + // omits them stays verifiable, which is what keeps third-party issuers + // working. + let mut ec = p256_fixture("p256-k1"); + ec.jwk.common.public_key_use = None; + ec.jwk.common.key_operations = None; + ec.jwk.common.key_algorithm = None; + assert!(jwk_compatible_with_alg(&ec.jwk, Algorithm::ES256)); +} + +#[tokio::test(flavor = "current_thread")] +async fn verify_resource_rejects_key_published_for_encryption() { + // End-to-end: an issuer whose only P-256 key is marked `use: enc` has no + // key eligible to verify with, even though the curve matches. + let mut fixture = p256_fixture("p256-k1"); + fixture.jwk.common.public_key_use = Some(jsonwebtoken::jwk::PublicKeyUse::Encryption); + let mut header = jsonwebtoken::Header::new(Algorithm::ES256); + header.typ = Some(TYP_RESOURCE.into()); + header.kid = Some("p256-k1".into()); + let claims = serde_json::json!({ + "iss": "iss.example", + "aud": "ps.example", + "jti": "j1", + "iat": 1000, + "exp": 9999999999_i64, + "dwk": "aa-resource", + "agent": "agent-1", + "agent_jkt": "abc", + "scope": "read", + }); + let jwt = jsonwebtoken::encode(&header, &claims, &fixture.signing).expect("encode"); + let jwks = jwks_with_key("iss.example", fixture.jwk); + let v = TokenVerifier::new(jwks, SystemTimeSource); + let err = v.verify_resource(&jwt, "ps.example").await.unwrap_err(); + assert!(matches!(err, TokenError::NoCompatibleJwk), "got {err:?}"); +} + #[tokio::test(flavor = "current_thread")] async fn assert_freshness_uses_supplied_clock() { let now = Arc::new(std::sync::atomic::AtomicI64::new(1000)); From 160af04dc139de2be6457fea75a218a5ad29837e Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:08 -0400 Subject: [PATCH 03/32] chore(trogon-aauth-verify): describe the replay store that exists, not one that does not The docs promised a JetStream-backed production store in trogon-aauth-person; no such implementation ships, which understates the multi-replica gap. Signed-off-by: Yordis Prieto --- .../aauth/trogon-aauth-verify/Cargo.toml | 6 +++-- .../aauth/trogon-aauth-verify/src/replay.rs | 24 +++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/Cargo.toml b/rsworkspace/crates/aauth/trogon-aauth-verify/Cargo.toml index 841110cc9..d0c2bf5f9 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/Cargo.toml +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/Cargo.toml @@ -18,8 +18,10 @@ async-trait = { workspace = true } base64 = { workspace = true } # AAuth tokens rely on JWS shapes (esp. EC `kid`-based selection) that the # crate's parser only models cleanly from jsonwebtoken 10.x; keep the -# version pin local to this crate so main's other jsonwebtoken 9.3 -# consumers (auth-callout, etc.) are not forced to upgrade in lockstep. +# version pin local to this crate so the workspace's other jsonwebtoken +# consumers (auth-callout, on 11.x) are not forced to move in lockstep. +# The cost is two majors linked into a2a-gateway, which also means +# verification hardening cannot be shared between the two paths as code. jsonwebtoken = { version = "=10.4.0", features = ["rust_crypto"] } # rustls-tls only, never native-tls, per ADR#0015. reqwest = { workspace = true, features = ["json", "rustls-tls"] } diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/replay.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/replay.rs index 5a9e3ce8e..772472250 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/replay.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/replay.rs @@ -1,8 +1,14 @@ //! Replay-protection store: dedup `jti` and PoP nonces with TTLs. //! -//! The production-grade implementation lives in `trogon-aauth-person` backed by -//! NATS JetStream KV (TTL per key). This module provides the trait + an in-memory -//! variant used by the gateway when no shared store is configured. +//! This module provides the [`ReplayStore`] trait and [`InMemoryReplayStore`], +//! its only implementation today. The in-memory store is process-local, so a +//! multi-replica gateway accepts the same nonce once per replica: replay +//! protection is complete for a single-process deployment and partial for any +//! other. A shared backend (NATS JetStream KV, keyed with a per-key TTL, is +//! the intended one) has not been built yet; [`ReplayError::Backend`] exists +//! so it can be added without changing this trait, and `AAuthIngress` is +//! already generic over `S: ReplayStore` so wiring one in is a construction +//! change rather than a signature change. use std::collections::HashMap; use std::sync::Mutex; @@ -24,14 +30,18 @@ pub enum ReplayError { /// failed-closed. #[error("replay store mutex poisoned")] MutexPoisoned, - /// Pluggable backends (NATS JetStream KV, Redis, etc.) surface their own - /// typed source error here instead of being flattened to a String. + /// Reserved for shared backends (NATS JetStream KV, Redis, etc.) so they + /// surface their own typed source error here instead of being flattened + /// to a String. No such backend ships today. #[error("replay store backend failure")] Backend(#[source] Box), } -/// Best-effort in-memory replay protection. Suitable for a single-process gateway -/// or unit tests. Multi-instance deployments should use the JetStream-backed store. +/// Best-effort in-memory replay protection: complete for a single-process +/// gateway or unit tests, and partial for a multi-replica deployment, where +/// each replica keeps its own map and a nonce is therefore accepted once per +/// replica. There is no shared-store implementation to fall back to yet; see +/// the module docs. pub struct InMemoryReplayStore { inner: Mutex>, clock: Box i64 + Send + Sync>, From bae20cff6f17b09fa816aa9ed9d2bbef007c29da Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:20 -0400 Subject: [PATCH 04/32] fix(trogon-identity-types): keep private key material out of a published confirmation claim Three mint sites embedded a caller-supplied JWK verbatim into cnf and signed it, so one careless caller could publish a private key in a token. Signed-off-by: Yordis Prieto --- .../crates/aauth/trogon-aauth-as/src/error.rs | 2 + .../crates/aauth/trogon-aauth-as/src/mint.rs | 4 +- .../aauth/trogon-aauth-person/src/error.rs | 2 + .../aauth/trogon-aauth-person/src/mint.rs | 4 +- .../trogon-identity-types/src/aauth/mod.rs | 47 +++++++++++++ .../trogon-identity-types/src/aauth/tests.rs | 70 +++++++++++++++++++ .../trogon-identity-types/src/constants.rs | 11 +++ .../trogon-jwks-publisher/src/provider.rs | 13 ++-- .../src/provider/tests.rs | 23 ++++++ 9 files changed, 165 insertions(+), 11 deletions(-) diff --git a/rsworkspace/crates/aauth/trogon-aauth-as/src/error.rs b/rsworkspace/crates/aauth/trogon-aauth-as/src/error.rs index f8befee3b..05ca39b79 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-as/src/error.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-as/src/error.rs @@ -82,6 +82,8 @@ pub enum MintError { Encode(#[source] jsonwebtoken::errors::Error), #[error("ttl overflowed i64 when added to iat ({iat} + {ttl_secs})")] TtlOverflow { iat: i64, ttl_secs: i64 }, + #[error("confirmation key rejected: {0}")] + Cnf(#[from] trogon_identity_types::aauth::CnfError), } /// Top-level error for one AS token-endpoint evaluation, per "Token Endpoint diff --git a/rsworkspace/crates/aauth/trogon-aauth-as/src/mint.rs b/rsworkspace/crates/aauth/trogon-aauth-as/src/mint.rs index 32670bfa0..993a718b3 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-as/src/mint.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-as/src/mint.rs @@ -129,9 +129,7 @@ pub fn mint_auth_jwt( consent_id: inputs.consent_id.map(str::to_string), resource: inputs.resource.map(str::to_string), act: inputs.act.clone(), - cnf: Some(Cnf { - jwk: inputs.cnf_jwk.clone(), - }), + cnf: Some(Cnf::public(inputs.cnf_jwk.clone())?), }; let mut header = Header::new(alg); diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/error.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/error.rs index 6cd9f69c6..b0ecb29c2 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/error.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/error.rs @@ -73,6 +73,8 @@ pub enum MintError { Encode(#[source] jsonwebtoken::errors::Error), #[error("ttl overflowed i64 when added to iat ({iat} + {ttl_secs})")] TtlOverflow { iat: i64, ttl_secs: i64 }, + #[error("confirmation key rejected: {0}")] + Cnf(#[from] trogon_identity_types::aauth::CnfError), } /// Failures operating on a pending request, per "Clarification Chat" and diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/mint.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/mint.rs index bcfb0d812..7171e9bd4 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/mint.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/mint.rs @@ -133,9 +133,7 @@ pub fn mint_auth_jwt( consent_id: inputs.consent_id.map(str::to_string), resource: inputs.resource.map(str::to_string), act: inputs.act.clone(), - cnf: Some(Cnf { - jwk: inputs.cnf_jwk.clone(), - }), + cnf: Some(Cnf::public(inputs.cnf_jwk.clone())?), }; let mut header = Header::new(alg); diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs index 8d09681a2..87e5d9daa 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs @@ -35,6 +35,53 @@ pub struct Cnf { pub jwk: Value, } +impl Cnf { + /// Build a confirmation claim, refusing any JWK that carries private or + /// symmetric key material. + /// + /// Issuers must go through this rather than constructing [`Cnf`] + /// literally. A `cnf` claim is embedded in a signed token that is handed + /// to resource servers by design, so a caller that passes a full keypair + /// instead of its public half publishes the private key to every party + /// that sees the token, with a valid signature over it. That mistake is + /// easy to make (JWK serializers include `d` unless asked not to) and + /// impossible to walk back once a token is issued, which is why it is + /// checked at the one place every issuer passes through. + /// + /// The field stays public so the verifier side can still deserialize an + /// inbound token: what a peer chose to put in its own `cnf` is not ours + /// to reject here, and verification reads only the public parameters. + pub fn public(jwk: Value) -> Result { + let Some(members) = jwk.as_object() else { + return Err(CnfError::NotAnObject); + }; + if members + .get("kty") + .and_then(Value::as_str) + .is_some_and(|kty| kty.eq_ignore_ascii_case(crate::constants::KTY_OCT)) + { + return Err(CnfError::SymmetricKey); + } + for member in crate::constants::JWK_PRIVATE_MEMBERS { + if members.contains_key(member) { + return Err(CnfError::PrivateKeyMaterial { member }); + } + } + Ok(Self { jwk }) + } +} + +/// Rejections from [`Cnf::public`]. +#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)] +pub enum CnfError { + #[error("cnf.jwk must be a JSON object")] + NotAnObject, + #[error("cnf.jwk must not be a symmetric key")] + SymmetricKey, + #[error("cnf.jwk carries private key material in member {member:?}")] + PrivateKeyMaterial { member: &'static str }, +} + /// Claims for an `aa-agent+jwt`. Issued by an Agent Provider at bootstrap. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct AgentClaims { diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs index db2bed62b..3aab5c994 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs @@ -238,3 +238,73 @@ fn split_header_skips_empty_segments_from_stray_semicolons() { let raw = "requirement=clarification;; ;"; assert_eq!(Requirement::parse(raw), Requirement::Clarification); } + +#[test] +fn cnf_public_accepts_an_ec_public_jwk() { + let jwk = serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"}); + let cnf = Cnf::public(jwk.clone()).expect("public jwk accepted"); + assert_eq!(cnf.jwk, jwk); +} + +#[test] +fn cnf_public_rejects_ec_private_scalar() { + // The mistake this guards: serializing a keypair instead of its public + // half puts `d` in a token that is handed to every resource server. + let jwk = serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB", "d": "SECRET"}); + assert_eq!( + Cnf::public(jwk).unwrap_err(), + CnfError::PrivateKeyMaterial { member: "d" } + ); +} + +#[test] +fn cnf_public_rejects_rsa_crt_parameters() { + for member in ["p", "q", "dp", "dq", "qi", "oth"] { + let mut jwk = serde_json::json!({"kty": "RSA", "n": "AAA", "e": "AQAB"}); + jwk[member] = serde_json::json!("SECRET"); + assert_eq!( + Cnf::public(jwk).unwrap_err(), + CnfError::PrivateKeyMaterial { member }, + "{member} must be refused" + ); + } +} + +#[test] +fn cnf_public_rejects_okp_private_scalar() { + let jwk = serde_json::json!({"kty": "OKP", "crv": "Ed25519", "x": "AAA", "d": "SECRET"}); + assert_eq!( + Cnf::public(jwk).unwrap_err(), + CnfError::PrivateKeyMaterial { member: "d" } + ); +} + +#[test] +fn cnf_public_rejects_symmetric_keys_by_kty() { + // Checked before the member scan so an `oct` key is refused even when + // `k` is absent: there is no public half of a symmetric key to carry. + let jwk = serde_json::json!({"kty": "oct"}); + assert_eq!(Cnf::public(jwk).unwrap_err(), CnfError::SymmetricKey); + let upper = serde_json::json!({"kty": "OCT", "k": "SECRET"}); + assert_eq!(Cnf::public(upper).unwrap_err(), CnfError::SymmetricKey); +} + +#[test] +fn cnf_public_rejects_non_object_jwk() { + for value in [ + serde_json::json!("not-a-jwk"), + serde_json::json!(null), + serde_json::json!([{"kty": "EC"}]), + ] { + assert_eq!(Cnf::public(value).unwrap_err(), CnfError::NotAnObject); + } +} + +#[test] +fn cnf_still_deserializes_a_peer_supplied_confirmation_claim() { + // The verifier read path must stay lenient: rejecting a peer's own `cnf` + // at parse time is not this type's call, and the guard is issuer-side. + let raw = r#"{"jwk":{"kty":"EC","crv":"P-256","x":"AAA","y":"BBB","d":"THEIRS"}}"#; + let cnf: Cnf = serde_json::from_str(raw).expect("inbound cnf parses"); + assert!(cnf.jwk.get("d").is_some()); +} diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs b/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs index 207cd365d..c0b0c4c7a 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs @@ -4,6 +4,17 @@ /// Maximum number of entries allowed in an `act` delegation chain. pub const MAX_ACT_CHAIN_DEPTH: usize = 8; +/// JWK members that carry private key material, across every key type AAuth +/// can encounter: `d` for EC (RFC 7518 Section 6.2.2), RSA (Section 6.3.2), +/// and OKP (RFC 8037 Section 2); the remaining RSA CRT parameters; and `k`, +/// which *is* the secret for a symmetric `oct` key (Section 6.4.1). +pub const JWK_PRIVATE_MEMBERS: [&str; 8] = ["d", "p", "q", "dp", "dq", "qi", "oth", "k"]; + +/// `kty` value for a symmetric key. Never valid in a confirmation claim: a +/// proof-of-possession key that both parties must hold is not a proof of +/// possession, and publishing it in a token discloses it. +pub const KTY_OCT: &str = "oct"; + /// `typ` header value identifying an agent identity token. pub const TYP_AGENT: &str = "aa-agent+jwt"; /// `typ` header value identifying a resource challenge token. diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider.rs index a2e4997af..14e9acca8 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider.rs @@ -235,10 +235,11 @@ pub enum PersonServerUrlError { /// public confirmation key. pub struct AgentTokenRequest { pub sub: AgentIdentifier, - /// Agent's public JWK, embedded verbatim into `cnf.jwk` per RFC 7800. - /// Kept as `serde_json::Value` to match `Cnf::jwk`'s type -- this crate - /// does not validate JWK shape beyond what the caller already produced - /// (e.g. via `trogon-aauth-sdk`'s public key derivation). + /// Agent's public JWK, embedded into `cnf.jwk` per RFC 7800. Kept as + /// `serde_json::Value` to match `Cnf::jwk`'s type; [`Cnf::public`] + /// rejects private and symmetric key material at mint time, but the + /// remaining shape is whatever the caller produced (e.g. via + /// `trogon-aauth-sdk`'s public key derivation). pub agent_jwk: serde_json::Value, pub ttl: TokenTtl, pub ps: Option, @@ -251,6 +252,8 @@ pub enum ProviderError { Encode(#[source] jsonwebtoken::errors::Error), #[error("system clock is before unix epoch")] ClockBeforeEpoch, + #[error("confirmation key rejected: {0}")] + Cnf(#[from] trogon_identity_types::aauth::CnfError), } /// Agent Provider: mints `aa-agent+jwt` tokens under a fixed signing key, @@ -288,7 +291,7 @@ impl AgentProvider { iat, exp, dwk: DWK_AGENT.to_string(), - cnf: Cnf { jwk: req.agent_jwk }, + cnf: Cnf::public(req.agent_jwk)?, ps: req.ps.map(PersonServerUrl::into_inner), }; diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs index bd3aa1171..07fc0f7a6 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs @@ -238,3 +238,26 @@ fn base64_decode(segment: &str) -> Vec { .decode(segment) .expect("valid base64url") } + +#[test] +fn mint_refuses_to_publish_private_key_material_in_cnf() { + // Passing the full keypair instead of its public half would sign the + // agent's private scalar into a token every resource server receives. + let key = AgentProviderKey::new( + test_encoding_key(), + KeyId::new("ap-key-1").expect("kid"), + ProviderIssuer::new("https://ap.example").expect("iss"), + ); + let provider = AgentProvider::new(key); + let mut jwk = test_agent_jwk(); + jwk["d"] = serde_json::json!("evZzL1gdAFr88hb2OF_2NxApJCzGCEDdfSp6VQO30hw"); + let req = AgentTokenRequest { + sub: AgentIdentifier::new("aauth:assistant-v2@agent.example").expect("sub"), + agent_jwk: jwk, + ttl: TokenTtl::new(3600).expect("ttl"), + ps: None, + }; + + let err = provider.mint(req).expect_err("private key material refused"); + assert!(matches!(err, ProviderError::Cnf(_)), "got {err:?}"); +} From bad87b2bc20f9c43c92b8961cf95f02fb2c88c8b Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:20 -0400 Subject: [PATCH 05/32] fix(trogon-gateway): make webhook dedup survive a replayed delivery Dedup keyed on an unsigned header an attacker can vary freely, and the stream had no duplicate window, so the guarantee was absent rather than weak. Signed-off-by: Yordis Prieto --- .../src/source/gitlab/constants.rs | 2 ++ .../src/source/gitlab/server.rs | 31 ++++++++++++++----- .../src/source/gitlab/server/tests.rs | 23 ++++++++++++-- .../src/source/gitlab/signature.rs | 5 ++- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/constants.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/constants.rs index 35e0b40ab..ca2b29bcf 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/constants.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/constants.rs @@ -16,6 +16,8 @@ pub const HEADER_IDEMPOTENCY_KEY: &str = "idempotency-key"; pub const HEADER_INSTANCE: &str = "x-gitlab-instance"; pub const NATS_HEADER_EVENT: &str = "X-GitLab-Event"; +pub const NATS_HEADER_WEBHOOK_ID: &str = "Webhook-Id"; +pub const NATS_HEADER_IDEMPOTENCY_KEY: &str = "Idempotency-Key"; pub const NATS_HEADER_WEBHOOK_UUID: &str = "X-GitLab-Webhook-UUID"; pub const NATS_HEADER_EVENT_UUID: &str = "X-GitLab-Event-UUID"; pub const NATS_HEADER_INSTANCE: &str = "X-GitLab-Instance"; diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs index 6e7982c11..75d5aba98 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs @@ -5,8 +5,8 @@ use super::GitLabSigningToken; use super::config::GitlabConfig; use super::constants::{ HEADER_EVENT, HEADER_EVENT_UUID, HEADER_IDEMPOTENCY_KEY, HEADER_INSTANCE, HEADER_WEBHOOK_UUID, HTTP_BODY_SIZE_MAX, - NATS_HEADER_EVENT, NATS_HEADER_EVENT_UUID, NATS_HEADER_INSTANCE, NATS_HEADER_REJECT_REASON, - NATS_HEADER_WEBHOOK_UUID, + NATS_HEADER_EVENT, NATS_HEADER_EVENT_UUID, NATS_HEADER_IDEMPOTENCY_KEY, NATS_HEADER_INSTANCE, + NATS_HEADER_REJECT_REASON, NATS_HEADER_WEBHOOK_ID, NATS_HEADER_WEBHOOK_UUID, }; use super::signature; use axum::{ @@ -40,12 +40,15 @@ impl RejectReason { async fn publish_unroutable( publisher: &ClaimCheckPublisher, subject_prefix: &NatsToken, + verified: &signature::VerifiedWebhook, reason: RejectReason, body: Bytes, ack_timeout: NonZeroDuration, ) -> StatusCode { let subject = format!("{subject_prefix}.unroutable"); let mut headers = async_nats::HeaderMap::new(); + headers.insert(async_nats::header::NATS_MESSAGE_ID, verified.webhook_id.as_str()); + headers.insert(NATS_HEADER_WEBHOOK_ID, verified.webhook_id.as_str()); headers.insert(NATS_HEADER_REJECT_REASON, reason.as_str()); let outcome = publisher @@ -84,6 +87,7 @@ pub async fn provision(js: &C, config: &GitlabConfig) -> Re js.get_or_create_stream(async_nats::jetstream::stream::Config { name: config.stream_name.as_str().to_owned(), subjects: vec![format!("{}.>", config.subject_prefix)], + duplicate_window: config.timestamp_tolerance.into(), max_age: config.stream_max_age.into(), ..Default::default() }) @@ -135,16 +139,20 @@ async fn handle_webhook_inner( headers: HeaderMap, body: Bytes, ) -> StatusCode { - if let Err(e) = signature::verify(&headers, &body, &state.signing_token, state.timestamp_tolerance) { - warn!(reason = %e, "GitLab webhook signature validation failed"); - return StatusCode::UNAUTHORIZED; - } + let verified = match signature::verify(&headers, &body, &state.signing_token, state.timestamp_tolerance) { + Ok(verified) => verified, + Err(e) => { + warn!(reason = %e, "GitLab webhook signature validation failed"); + return StatusCode::UNAUTHORIZED; + } + }; let Some(raw_event) = headers.get(HEADER_EVENT).and_then(|v| v.to_str().ok()) else { warn!("Missing X-GitLab-Event header"); return publish_unroutable( &state.publisher, &state.subject_prefix, + &verified, RejectReason::MissingEventHeader, body, state.nats_ack_timeout, @@ -163,6 +171,7 @@ async fn handle_webhook_inner( return publish_unroutable( &state.publisher, &state.subject_prefix, + &verified, RejectReason::InvalidEventToken, body, state.nats_ack_timeout, @@ -187,8 +196,14 @@ async fn handle_webhook_inner( let mut nats_headers = async_nats::HeaderMap::new(); nats_headers.insert(NATS_HEADER_EVENT, raw_event); - if let Some(id) = idempotency_key { - nats_headers.insert(async_nats::header::NATS_MESSAGE_ID, id); + // Dedup on the signed `webhook-id`, not on `idempotency-key`. Both + // identify a delivery, but only the former is inside the Standard + // Webhooks signed content, so it is the only one an attacker replaying a + // captured request cannot vary to defeat the JetStream duplicate window. + nats_headers.insert(async_nats::header::NATS_MESSAGE_ID, verified.webhook_id.as_str()); + nats_headers.insert(NATS_HEADER_WEBHOOK_ID, verified.webhook_id.as_str()); + if let Some(key) = idempotency_key { + nats_headers.insert(NATS_HEADER_IDEMPOTENCY_KEY, key); } if let Some(uuid) = webhook_uuid { nats_headers.insert(NATS_HEADER_WEBHOOK_UUID, uuid); diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs index d9a1ba777..201b586a1 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs @@ -168,6 +168,7 @@ async fn provision_creates_stream() { assert_eq!(streams[0].name, "GITLAB"); assert_eq!(streams[0].subjects, vec!["gitlab.>"]); assert_eq!(streams[0].max_age, Duration::from_secs(3600)); + assert_eq!(streams[0].duplicate_window, Duration::from_secs(300)); } #[tokio::test] @@ -216,6 +217,14 @@ async fn valid_webhook_publishes_to_nats_and_returns_200() { .headers .get(async_nats::header::NATS_MESSAGE_ID) .map(|v| v.as_str()), + Some("msg_123"), + ); + assert_eq!( + messages[0].headers.get(NATS_HEADER_WEBHOOK_ID).map(|v| v.as_str()), + Some("msg_123"), + ); + assert_eq!( + messages[0].headers.get(NATS_HEADER_IDEMPOTENCY_KEY).map(|v| v.as_str()), Some("idem-key-test"), ); } @@ -359,7 +368,7 @@ async fn empty_body_publishes_successfully() { } #[tokio::test] -async fn missing_idempotency_key_skips_dedup_id() { +async fn dedup_id_comes_from_signed_webhook_id_not_idempotency_key() { let _guard = tracing_guard(); let publisher = MockJetStreamPublisher::new(); @@ -385,9 +394,17 @@ async fn missing_idempotency_key_skips_dedup_id() { assert_eq!(resp.status(), StatusCode::OK); let messages = publisher.published_messages(); + assert_eq!( + messages[0] + .headers + .get(async_nats::header::NATS_MESSAGE_ID) + .map(|v| v.as_str()), + Some("msg_123"), + "dedup must not depend on the unsigned Idempotency-Key header" + ); assert!( - messages[0].headers.get(async_nats::header::NATS_MESSAGE_ID).is_none(), - "should not set Nats-Msg-Id when Idempotency-Key is absent" + messages[0].headers.get(NATS_HEADER_IDEMPOTENCY_KEY).is_none(), + "absent Idempotency-Key is forwarded as absent" ); } diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/signature.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/signature.rs index 50109599a..7566f59a5 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/signature.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/signature.rs @@ -5,7 +5,7 @@ use super::GitLabSigningToken; use super::constants::HEADER_NAMES; use crate::source::standard_webhooks; -pub use crate::source::standard_webhooks::SignatureError; +pub use crate::source::standard_webhooks::{SignatureError, VerifiedWebhook}; #[cfg(test)] pub use crate::source::standard_webhooks::{WebhookId, WebhookTimestamp}; @@ -14,7 +14,7 @@ pub fn verify( body: &[u8], signing_token: &GitLabSigningToken, timestamp_tolerance: NonZeroDuration, -) -> Result<(), SignatureError> { +) -> Result { standard_webhooks::verify( headers, body, @@ -22,7 +22,6 @@ pub fn verify( timestamp_tolerance, HEADER_NAMES, ) - .map(|_| ()) } #[cfg(test)] From d17554da5770a38a080c103f0441c08648054df7 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:20 -0400 Subject: [PATCH 06/32] fix(a2a-gateway): refuse to run unverified bundles when verification was asked for A typo in the signing key parsed to None and read as verification-not-wanted, so the misconfiguration silently cost the very check it was meant to enable. Signed-off-by: Yordis Prieto --- .../crates/a2a/a2a-gateway/src/runtime/env.rs | 55 +++++++++++++++---- .../a2a/a2a-gateway/src/runtime/env/tests.rs | 30 +++++++--- .../a2a-gateway/src/runtime/policy_stack.rs | 25 ++++++--- .../src/runtime/policy_stack/tests.rs | 25 +++++++++ 4 files changed, 110 insertions(+), 25 deletions(-) diff --git a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env.rs b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env.rs index 92227d337..6d607347a 100644 --- a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env.rs +++ b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env.rs @@ -5,6 +5,11 @@ //! (returns the safer disabled / shorter-deadline default) when the //! env value is missing or malformed -- callers should branch on the //! resulting state rather than re-parse the env at the dispatch site. +//! +//! [`gateway_tier3_signing_pubkey`] is the exception, and deliberately +//! so: for a code-signing key the disabled default is *not* the safer +//! one, so it reports a malformed value as its own state instead of +//! folding it into "not configured". use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -34,26 +39,56 @@ pub fn gateway_audit_publish_enabled(env: &E) -> bool { parse_bool_flag(env, ENV_GATEWAY_AUDIT_PUBLISH) } -/// Tier-3 wasm bundle signing public key, if configured. Returns -/// `None` for unset / empty / unparseable values -- the gateway -/// then refuses to verify signatures rather than half-trusting an -/// invalid pubkey. -pub fn gateway_tier3_signing_pubkey(env: &E) -> Option { +/// What `A2A_GATEWAY_TIER3_SIGNING_PUBKEY` says about bundle signing. +/// +/// The three states are kept distinct because two of them look alike +/// and mean opposite things. "Unset" is an operator who never opted +/// into signing, and unsigned bundles are the expected posture. +/// "Invalid" is an operator who *did* opt in and mistyped the key; +/// collapsing that into "no pubkey configured" would silently execute +/// unverified wasm on a deployment that asked for verification. +#[derive(Debug)] +pub enum Tier3SigningKey { + /// Var unset or blank: bundle signing was never requested. + NotConfigured, + /// A usable verifying key. + Configured(Ed25519PublicKey), + /// Var set to something that is not a valid ed25519 pubkey. + Invalid, +} + +impl Tier3SigningKey { + /// The key to hand the wasm substrate, or `None` when signing was + /// never configured. [`Self::Invalid`] has no such projection on + /// purpose: callers have to handle it before they can get here. + pub fn into_configured(self) -> Option { + match self { + Self::Configured(pubkey) => Some(pubkey), + Self::NotConfigured | Self::Invalid => None, + } + } +} + +/// Reads the tier-3 wasm bundle signing public key. +/// +/// Unlike the other helpers in this module, an unusable value here is +/// not folded into the disabled default: see [`Tier3SigningKey`]. +pub fn gateway_tier3_signing_pubkey(env: &E) -> Tier3SigningKey { let Ok(raw) = env.var(ENV_GATEWAY_TIER3_SIGNING_PUBKEY) else { - return None; + return Tier3SigningKey::NotConfigured; }; let trimmed = raw.trim(); if trimmed.is_empty() { - return None; + return Tier3SigningKey::NotConfigured; } match Ed25519PublicKey::from_hex(trimmed) { - Ok(pubkey) => Some(pubkey), + Ok(pubkey) => Tier3SigningKey::Configured(pubkey), Err(err) => { warn!( error = %err, - "{ENV_GATEWAY_TIER3_SIGNING_PUBKEY} invalid; tier-3 bundle signing disabled", + "{ENV_GATEWAY_TIER3_SIGNING_PUBKEY} is not a valid ed25519 pubkey", ); - None + Tier3SigningKey::Invalid } } } diff --git a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env/tests.rs b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env/tests.rs index ca105dea2..b8fb8bb95 100644 --- a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env/tests.rs +++ b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/env/tests.rs @@ -30,37 +30,51 @@ fn audit_publish_enables_on_truthy_value() { } #[test] -fn tier3_signing_pubkey_returns_none_when_unset() { +fn tier3_signing_pubkey_is_not_configured_when_unset() { let env = InMemoryEnv::new(); - assert!(gateway_tier3_signing_pubkey(&env).is_none()); + assert!(matches!( + gateway_tier3_signing_pubkey(&env), + Tier3SigningKey::NotConfigured + )); } #[test] -fn tier3_signing_pubkey_returns_none_for_empty_string() { +fn tier3_signing_pubkey_is_not_configured_for_empty_string() { // An empty value must surface as "no pubkey configured" rather // than a half-trusted invalid pubkey. Operators clearing the // env var to disable signing rely on this. let env = InMemoryEnv::new(); env.set(ENV_GATEWAY_TIER3_SIGNING_PUBKEY, " "); - assert!(gateway_tier3_signing_pubkey(&env).is_none()); + assert!(matches!( + gateway_tier3_signing_pubkey(&env), + Tier3SigningKey::NotConfigured + )); } #[test] -fn tier3_signing_pubkey_returns_none_for_invalid_hex() { +fn tier3_signing_pubkey_is_invalid_not_unconfigured_for_bad_hex() { + // The distinction that matters: an operator who typo'd the key + // asked for verification and must not silently get none. let env = InMemoryEnv::new(); env.set(ENV_GATEWAY_TIER3_SIGNING_PUBKEY, "not-hex"); - assert!(gateway_tier3_signing_pubkey(&env).is_none()); + assert!(matches!(gateway_tier3_signing_pubkey(&env), Tier3SigningKey::Invalid)); +} + +#[test] +fn tier3_signing_pubkey_invalid_has_no_configured_projection() { + assert!(Tier3SigningKey::Invalid.into_configured().is_none()); + assert!(Tier3SigningKey::NotConfigured.into_configured().is_none()); } #[test] fn tier3_signing_pubkey_parses_valid_hex() { // Test ed25519 pubkey from the a2a-redaction fixtures (32-byte - // hex). Asserts the success path produces a `Some(_)` without + // hex). Asserts the success path produces a usable key without // hard-coding the inner type's debug shape. let env = InMemoryEnv::new(); let hex = "abababababababababababababababababababababababababababababababab"; env.set(ENV_GATEWAY_TIER3_SIGNING_PUBKEY, hex); - assert!(gateway_tier3_signing_pubkey(&env).is_some()); + assert!(gateway_tier3_signing_pubkey(&env).into_configured().is_some()); } #[test] diff --git a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack.rs b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack.rs index bb615cabe..b541b785b 100644 --- a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack.rs +++ b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack.rs @@ -22,9 +22,11 @@ use crate::policy::tier2_cel::{RealTier2CelEvaluator, Tier2CompiledBundle}; use crate::policy::tier3_redaction::load_tier3_manifests_from_bundle; use crate::policy::wasmtime_substrate::{Tier2State, WasmtimeSubstrate}; use crate::policy::{NoopTier3RedactionGate, RealTier3RedactionGate, Tier3RedactionGate, Tier3SkillManifest}; -use crate::runtime::env::{gateway_tier2_cel_enabled, gateway_tier3_signing_pubkey}; +use crate::runtime::env::{Tier3SigningKey, gateway_tier2_cel_enabled, gateway_tier3_signing_pubkey}; -use crate::constants::{ENV_POLICY_BUNDLE_DIR, ENV_POLICY_SKILLS, ENV_TIER3_REDACTION_ENABLED}; +use crate::constants::{ + ENV_GATEWAY_TIER3_SIGNING_PUBKEY, ENV_POLICY_BUNDLE_DIR, ENV_POLICY_SKILLS, ENV_TIER3_REDACTION_ENABLED, +}; /// The dispatch path's view of the policy stack. Each field is /// independently-replaceable in tests: @@ -63,10 +65,11 @@ impl GatewayPolicyStack { /// Boot the policy stack from environment variables. /// /// Returns the Noop stack when no bundle directory is configured, -/// when the directory is empty, or when the Wasmtime substrate fails -/// to load -- in every failure path the gateway still serves traffic -/// but applies no policy, with a warning logged so operators can -/// diagnose the misconfiguration. +/// when the directory is empty, when the configured bundle signing +/// pubkey is unusable, or when the Wasmtime substrate fails to load +/// -- in every failure path the gateway still serves traffic but +/// applies no policy, with a warning logged so operators can diagnose +/// the misconfiguration. /// /// Reads: /// - `A2A_GATEWAY_POLICY_BUNDLE_DIR` -- root of the policy bundle layout @@ -92,7 +95,15 @@ pub fn gateway_policy_stack_from_env(env: &E) -> GatewayPolicyStack } let bundle_path = WasmBundlePath::new(dir); - let tier3_signing_pubkey = gateway_tier3_signing_pubkey(env); + let signing_key = gateway_tier3_signing_pubkey(env); + if matches!(signing_key, Tier3SigningKey::Invalid) { + // Booting without the pubkey would run the very bundles this + // operator asked to have verified, unverified. A typo in the + // key must cost policy enforcement, not bundle authenticity. + warn!("{ENV_GATEWAY_TIER3_SIGNING_PUBKEY} is set but unusable; refusing to load unverified bundles"); + return GatewayPolicyStack::noop(); + } + let tier3_signing_pubkey = signing_key.into_configured(); let tier2_cel_active = gateway_tier2_cel_enabled(env); let tier2 = load_tier2_state(&bundle_path, tier2_cel_active); diff --git a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack/tests.rs b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack/tests.rs index d2679615b..b65c5b6a7 100644 --- a/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack/tests.rs +++ b/rsworkspace/crates/a2a/a2a-gateway/src/runtime/policy_stack/tests.rs @@ -57,3 +57,28 @@ fn from_env_returns_substrate_when_bundle_dir_set_to_any_path() { assert!(stack.substrate.is_some()); assert!(stack.tier3_manifests.is_empty()); } + +#[test] +fn from_env_returns_noop_when_signing_pubkey_is_unusable() { + // A bundle dir plus a mistyped signing pubkey must not boot a + // substrate that would execute those bundles unverified. Losing + // policy enforcement is the correct cost of the typo. + let env = InMemoryEnv::new(); + env.set(ENV_POLICY_BUNDLE_DIR, "/tmp/policy-bundle-for-boot-test"); + env.set(ENV_GATEWAY_TIER3_SIGNING_PUBKEY, "not-hex"); + let stack = gateway_policy_stack_from_env(&env); + assert!(stack.substrate.is_none()); + assert!(stack.tier3_manifests.is_empty()); +} + +#[test] +fn from_env_returns_substrate_when_signing_pubkey_is_valid() { + let env = InMemoryEnv::new(); + env.set(ENV_POLICY_BUNDLE_DIR, "/tmp/policy-bundle-for-boot-test"); + env.set( + ENV_GATEWAY_TIER3_SIGNING_PUBKEY, + "abababababababababababababababababababababababababababababababab", + ); + let stack = gateway_policy_stack_from_env(&env); + assert!(stack.substrate.is_some()); +} From 837fb0b551c916c092a30763bce5b298ef0e755d Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:20 -0400 Subject: [PATCH 07/32] fix(a2a-redaction): hold bundle signatures to the strict check Code signing is the wrong place to accept the permissive default, which lets one signature validate under more than one public key. Signed-off-by: Yordis Prieto --- .../crates/a2a/a2a-redaction/src/signed_bundle/verify.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rsworkspace/crates/a2a/a2a-redaction/src/signed_bundle/verify.rs b/rsworkspace/crates/a2a/a2a-redaction/src/signed_bundle/verify.rs index 6ecca25a5..dd7780e39 100644 --- a/rsworkspace/crates/a2a/a2a-redaction/src/signed_bundle/verify.rs +++ b/rsworkspace/crates/a2a/a2a-redaction/src/signed_bundle/verify.rs @@ -1,4 +1,3 @@ -use ed25519_dalek::Verifier; use sha2::{Digest, Sha256}; use super::digest::Sha256Digest; @@ -88,8 +87,13 @@ pub fn verify_signed_bundle( let signature = envelope.signature_bytes(&skill_id)?; let message = sign_bundle_digest(envelope.version, &skill_id, expected_manifest, expected_wasm); + // `verify_strict` rather than `verify`: this is a code-signing + // decision, so the permissive checks are the wrong default. Strict + // rejects small-order and non-canonically-encoded key/nonce points, + // which is what closes the gap where one signature validates under + // more than one public key. verifying_key - .verify(&message, &signature.dalek_signature()?) + .verify_strict(&message, &signature.dalek_signature()?) .map_err(|_| SignatureVerificationError::SignatureVerificationFailed { skill_id: skill_id.to_string(), }) From d00735c891b1b49516dba651e0e3ce0b07ae5e46 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:20 -0400 Subject: [PATCH 08/32] fix(a2a-auth-callout): stop a token from choosing the algorithm that verifies it The validator was built from the token's own header, so its algorithm check compared the header against itself and admitted whatever was asserted. Signed-off-by: Yordis Prieto --- .../a2a/a2a-auth-callout/src/constants.rs | 16 ++ .../a2a-auth-callout/src/credentials/oidc.rs | 61 +++++- .../src/credentials/oidc/tests.rs | 179 +++++++++++++++++- 3 files changed, 253 insertions(+), 3 deletions(-) diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/constants.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/constants.rs index 3d79dfb75..c53de0b29 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/constants.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/constants.rs @@ -30,3 +30,19 @@ pub const NATS_JWT_PREFIX: &[u8] = b"eyJ"; /// Default minted user JWT TTL, in seconds, for the `a2a-auth-callout` binary. pub const DEFAULT_USER_JWT_TTL_SECS: u64 = 300; + +/// Signature algorithms an inbound OIDC ID token may assert. +/// +/// Scoped to the RSA family because [`crate::credentials::oidc`] only builds +/// decoding keys from RSA JWK components; an allowlist that admitted anything +/// else would name algorithms that cannot verify here anyway. The list exists +/// so the algorithm is a deployment decision rather than something the token +/// under verification chooses for itself. +pub(crate) const OIDC_ALLOWED_ALGORITHMS: [jsonwebtoken::Algorithm; 6] = [ + jsonwebtoken::Algorithm::RS256, + jsonwebtoken::Algorithm::RS384, + jsonwebtoken::Algorithm::RS512, + jsonwebtoken::Algorithm::PS256, + jsonwebtoken::Algorithm::PS384, + jsonwebtoken::Algorithm::PS512, +]; diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs index ff14d4c7b..8ea4417cb 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs @@ -1,12 +1,13 @@ use std::sync::Arc; +use crate::constants::OIDC_ALLOWED_ALGORITHMS; use crate::error::{AuthCalloutError, CredentialError}; use crate::jwt::{ AudienceAccount, ExternalSubject, UserJwtClaims, derive_caller_id, spicedb_principal_from_oidc_claims, }; use crate::permissions::IssuedPermissions; -use jsonwebtoken::jwk::{AlgorithmParameters, JwkSet}; -use jsonwebtoken::{DecodingKey, Validation, decode, decode_header}; +use jsonwebtoken::jwk::{AlgorithmParameters, JwkSet, KeyAlgorithm, KeyOperations, PublicKeyUse}; +use jsonwebtoken::{Algorithm, DecodingKey, Validation, decode, decode_header}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct OidcIssuerUrl(String); @@ -189,6 +190,46 @@ fn url_origin(s: &str) -> Option<(String, String, Option)> { Some((scheme, host.to_ascii_lowercase(), port)) } +/// A JWK may verify an OIDC ID token under `alg` only when the key's own +/// advertised purpose permits verification and its declared algorithm, if any, +/// is the one asserted. +/// +/// RFC 7517 makes `use` (section 4.2), `key_ops` (section 4.3), and `alg` +/// (section 4.4) optional, so an absent member stays permissive: an external +/// IdP's JWKS is not ours to constrain beyond what it states. But a publisher +/// that does set them has declared what the key is for, and honoring that stops +/// a key published for encryption, or pinned to one RSA algorithm, from being +/// conscripted into verifying another. +/// +/// Mirrors `jwk_compatible_with_alg` in `trogon-aauth-verify`'s token +/// verifier. Kept separate rather than shared because the two crates compile +/// against different major versions of `jsonwebtoken`, so the `Jwk` types are +/// distinct. +fn jwk_permits_verification_with(jwk: &jsonwebtoken::jwk::Jwk, alg: jsonwebtoken::Algorithm) -> bool { + if let Some(public_key_use) = &jwk.common.public_key_use + && !matches!(public_key_use, PublicKeyUse::Signature) + { + return false; + } + if let Some(key_operations) = &jwk.common.key_operations + && !key_operations.iter().any(|op| matches!(op, KeyOperations::Verify)) + { + return false; + } + let Some(declared) = jwk.common.key_algorithm else { + return true; + }; + matches!( + (declared, alg), + (KeyAlgorithm::RS256, Algorithm::RS256) + | (KeyAlgorithm::RS384, Algorithm::RS384) + | (KeyAlgorithm::RS512, Algorithm::RS512) + | (KeyAlgorithm::PS256, Algorithm::PS256) + | (KeyAlgorithm::PS384, Algorithm::PS384) + | (KeyAlgorithm::PS512, Algorithm::PS512) + ) +} + impl JwksOidcVerifier { pub(crate) async fn fetch_jwks(&self) -> Result { match &self.jwks { @@ -239,7 +280,23 @@ impl JwksOidcVerifier { let jwk = jwks .find(kid) .ok_or_else(|| CredentialError::InvalidCredentials(format!("no JWK for kid {kid}")))?; + if !OIDC_ALLOWED_ALGORITHMS.contains(&header.alg) { + return Err(CredentialError::InvalidCredentials(format!( + "unsupported OIDC token algorithm {:?}", + header.alg + )) + .into()); + } + if !jwk_permits_verification_with(jwk, header.alg) { + return Err(CredentialError::InvalidCredentials(format!( + "JWK for kid {kid} is not published for verifying {:?} signatures", + header.alg + )) + .into()); + } let auds: Vec<&str> = self.expected_id_token_audiences.iter().map(String::as_str).collect(); + // Built from the header only after the allowlist above has vetted it, + // so the token cannot nominate its own algorithm. let mut validation = Validation::new(header.alg); validation.set_issuer(&[self.issuer.as_str()]); validation.set_audience(&auds); diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs index 58b925dcc..59b444dab 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs @@ -38,7 +38,9 @@ fn test_jwks_and_encoding_key(rng: &mut OsRng) -> (JwkSet, jsonwebtoken::Encodin let jwk = Jwk { common: CommonParameters { public_key_use: Some(PublicKeyUse::Signature), - key_operations: Some(vec![KeyOperations::Sign]), + // `verify`, not `sign`: this is the *public* half of the pair, and + // RFC 7517 section 4.3 scopes `key_ops` to what this key can do. + key_operations: Some(vec![KeyOperations::Verify]), key_id: Some("test-kid".into()), x509_url: None, x509_chain: None, @@ -406,3 +408,178 @@ async fn oidc_verifier_trait_delegates_to_verify_internal() { .expect("verify via trait"); assert_eq!(claims.sub.as_str(), "user-1"); } + +/// Signs `claims` as an RS256 token naming `test-kid`, the shape every guard +/// test below starts from before varying one thing about the JWK. +fn rs256_token_for(issuer: &OidcIssuerUrl, enc: &jsonwebtoken::EncodingKey) -> String { + #[derive(Serialize)] + struct IdClaims { + sub: String, + iss: String, + aud: String, + exp: u64, + } + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + let id = IdClaims { + sub: "user-guard".into(), + iss: issuer.as_str().to_owned(), + aud: "a2a-client".into(), + exp: now + 600, + }; + let mut header = jsonwebtoken::Header::new(jsonwebtoken::Algorithm::RS256); + header.kid = Some("test-kid".into()); + jsonwebtoken::encode(&header, &id, enc).expect("encode") +} + +fn jwks_with_common(jwks: &JwkSet, mutate: impl FnOnce(&mut CommonParameters)) -> JwkSet { + let mut jwk = jwks.keys[0].clone(); + mutate(&mut jwk.common); + JwkSet { keys: vec![jwk] } +} + +#[tokio::test] +async fn verify_rejects_algorithm_outside_the_allowlist() { + let rng = &mut OsRng; + let (jwks, _) = test_jwks_and_encoding_key(rng); + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let verifier = JwksOidcVerifier::with_static_jwks(issuer.clone(), vec!["a2a-client".into()], jwks); + + #[derive(Serialize)] + struct IdClaims { + sub: String, + iss: String, + aud: String, + exp: u64, + } + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + let id = IdClaims { + sub: "user-hs".into(), + iss: issuer.as_str().to_owned(), + aud: "a2a-client".into(), + exp: now + 600, + }; + let mut header = jsonwebtoken::Header::new(jsonwebtoken::Algorithm::HS256); + header.kid = Some("test-kid".into()); + let token = + jsonwebtoken::encode(&header, &id, &jsonwebtoken::EncodingKey::from_secret(b"shared")).expect("encode hs256"); + + let err = verifier + .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) + .await + .unwrap_err(); + let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(msg)) = err else { + panic!("expected InvalidCredentials, got {err:?}"); + }; + assert!( + msg.contains("unsupported OIDC token algorithm"), + "unexpected message: {msg}" + ); +} + +#[tokio::test] +async fn verify_rejects_a_jwk_published_for_encryption() { + let rng = &mut OsRng; + let (jwks, enc) = test_jwks_and_encoding_key(rng); + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let token = rs256_token_for(&issuer, &enc); + let jwks = jwks_with_common(&jwks, |common| { + common.public_key_use = Some(PublicKeyUse::Encryption); + common.key_operations = None; + }); + let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["a2a-client".into()], jwks); + + let err = verifier + .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) + .await + .unwrap_err(); + let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(msg)) = err else { + panic!("expected InvalidCredentials, got {err:?}"); + }; + assert!(msg.contains("not published for verifying"), "unexpected message: {msg}"); +} + +#[tokio::test] +async fn verify_rejects_a_jwk_whose_key_ops_omit_verify() { + let rng = &mut OsRng; + let (jwks, enc) = test_jwks_and_encoding_key(rng); + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let token = rs256_token_for(&issuer, &enc); + let jwks = jwks_with_common(&jwks, |common| { + common.key_operations = Some(vec![KeyOperations::Encrypt]); + }); + let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["a2a-client".into()], jwks); + + let err = verifier + .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) + .await + .unwrap_err(); + assert!(matches!( + err, + AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(_)) + )); +} + +#[tokio::test] +async fn verify_rejects_a_jwk_pinned_to_a_different_rsa_algorithm() { + let rng = &mut OsRng; + let (jwks, enc) = test_jwks_and_encoding_key(rng); + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let token = rs256_token_for(&issuer, &enc); + let jwks = jwks_with_common(&jwks, |common| { + common.key_algorithm = Some(jsonwebtoken::jwk::KeyAlgorithm::PS512); + }); + let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["a2a-client".into()], jwks); + + let err = verifier + .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) + .await + .unwrap_err(); + let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(msg)) = err else { + panic!("expected InvalidCredentials, got {err:?}"); + }; + assert!(msg.contains("not published for verifying"), "unexpected message: {msg}"); +} + +#[tokio::test] +async fn verify_accepts_a_jwk_that_declares_the_asserted_algorithm() { + let rng = &mut OsRng; + let (jwks, enc) = test_jwks_and_encoding_key(rng); + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let token = rs256_token_for(&issuer, &enc); + let jwks = jwks_with_common(&jwks, |common| { + common.key_algorithm = Some(jsonwebtoken::jwk::KeyAlgorithm::RS256); + }); + let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["a2a-client".into()], jwks); + + let claims = verifier + .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) + .await + .expect("declared alg matches the asserted one"); + assert_eq!(claims.sub.as_str(), "user-guard"); +} + +#[tokio::test] +async fn verify_accepts_a_jwk_that_declares_no_purpose_at_all() { + let rng = &mut OsRng; + let (jwks, enc) = test_jwks_and_encoding_key(rng); + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let token = rs256_token_for(&issuer, &enc); + let jwks = jwks_with_common(&jwks, |common| { + common.public_key_use = None; + common.key_operations = None; + common.key_algorithm = None; + }); + let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["a2a-client".into()], jwks); + + let claims = verifier + .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) + .await + .expect("absent RFC 7517 members stay permissive"); + assert_eq!(claims.sub.as_str(), "user-guard"); +} From 8cf3fd8e558c0d26894a8802fb16d06b3a08f634 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:11:20 -0400 Subject: [PATCH 09/32] chore(adr): record the external federation surface and correct what the corpus claims exists Signed-off-by: Yordis Prieto --- docs/adr/0017-aauth-agent-authentication.md | 47 +++- .../0036-agent-self-certifying-identity.md | 5 + docs/adr/0038-agent-identity-crypto-suite.md | 52 +++- docs/adr/0049-revocation-latency-target.md | 7 + .../0053-external-oidc-federation-surface.md | 227 ++++++++++++++++++ docs/adr/index.md | 1 + 6 files changed, 333 insertions(+), 6 deletions(-) create mode 100644 docs/adr/0053-external-oidc-federation-surface.md diff --git a/docs/adr/0017-aauth-agent-authentication.md b/docs/adr/0017-aauth-agent-authentication.md index 0b34b4d4a..1e25523b8 100644 --- a/docs/adr/0017-aauth-agent-authentication.md +++ b/docs/adr/0017-aauth-agent-authentication.md @@ -197,6 +197,30 @@ the tier-1 SpiceDB layer already uses. Optional tuning when unset, but an unparseable value is still a startup error rather than a silently-ignored default. +### 7. Discovery mode accepts any HTTPS issuer unless an allowlist is pinned + +`iss` is read from an unverified JWT claim in order to locate the key that +will verify it, so in discovery mode it is attacker-influenced input that +drives an outbound fetch. `HttpJwksResolver` bounds the damage structurally: +HTTPS only, DNS hosts only (IP literals and loopback names rejected outright +as SSRF attempts), a request timeout, and a streamed response-size cap that +holds even when `Content-Length` is absent or lies. + +What it does not do by default is bound *which* issuers may be resolved at +all. `A2A_GATEWAY_AAUTH_JWKS_ALLOWED_ISSUERS` takes a comma-separated exact +list (trailing slashes ignored) and rejects any other `iss` before a network +call is made; when it is unset, any issuer that is HTTPS-reachable and serves +a parseable well-known document can mint a token this gateway will +successfully verify. That default is deliberate and follows from the draft's +self-sovereign premise, that an agent needs no pre-registration, but it means +**verification success carries no admission decision on its own**. The +authority plane of [ADR#0037](./0037-agent-identity-governance.md) is what +decides whether a verified-but-unknown agent may act, and a deployment that +knows its federation partners should pin them here rather than rely on that +plane alone. This variable is part of the fail-loud inventory above only in +the sense that a malformed value is rejected; being unset is a valid, +documented posture, not a misconfiguration. + ## Consequences - Agents authenticate to the gateway with a self-sovereign, key-bound identity @@ -219,11 +243,24 @@ silently-ignored default. alongside whatever auth token the Person Server issued. - `-32118` is now reserved on the JSON-RPC-over-NATS error surface for AAuth denials specifically; no other gateway error path may reuse it. -- Replay protection today is `InMemoryReplayStore`, process-local. A - multi-node gateway deployment can have the same nonce accepted once per - node until a shared store (NATS [JetStream](../glossary/jetstream) KV, per the doc comment in - `trogon-aauth-person`) is wired in; single-node deployments are fully - protected, multi-node deployments are not yet. +- Replay protection today is `InMemoryReplayStore`, process-local, and it is + the *only* `ReplayStore` implementation in the workspace. A multi-node + gateway deployment has the same nonce accepted once per node; single-node + deployments are fully protected, multi-node deployments are not. A shared + store (NATS [JetStream](../glossary/jetstream) KV keyed with a per-key TTL + is the intended backend) has not been built. `AAuthIngress` is generic over + `S: ReplayStore` and `ReplayError::Backend` is reserved for it, so adding + one is a construction change rather than a signature change. +- Verification success is not admission. In discovery mode with no issuer + allowlist pinned (Decision 7), any HTTPS-reachable issuer can mint a token + that verifies. Deployments that rely on AAuth as an authorization boundary + rather than an authentication one are misreading it; that boundary is + [ADR#0037](./0037-agent-identity-governance.md)'s authority plane. +- The three algorithms this ADR admits (ES256, ES384, EdDSA) are the whole + verifier allowlist, and none of them is RS256. Federating this platform's + identities *out* to an external OIDC-consuming IdP is therefore not + reachable from this surface; that boundary is + [ADR#0053](./0053-external-oidc-federation-surface.md). - JWKS resolution is env-selected between a static file (`StaticJwks`) and live `.well-known/{dwk}` discovery (`HttpJwksResolver`, HTTPS-only, size- and timeout-capped, wrapped in `CachedJwksResolver`). Static deployments diff --git a/docs/adr/0036-agent-self-certifying-identity.md b/docs/adr/0036-agent-self-certifying-identity.md index 0b3079449..8762a36ff 100644 --- a/docs/adr/0036-agent-self-certifying-identity.md +++ b/docs/adr/0036-agent-self-certifying-identity.md @@ -94,6 +94,10 @@ These do not change the genesis anchor and are added when needed: replicated substrate (relays, a DID method, a transparency log) so parties outside this system can resolve it. Until then the agent's event stream is the authoritative key record. + [ADR#0053](./0053-external-oidc-federation-surface.md) settles the narrower + external-verifier case with an OIDC issuer and JWKS, which is what + non-AAuth parties actually consume; it does not close this layer, because + resolving the *identifier* is the part that still needs a DID method. - **Key rotation**: a later signed event authorized by the current key, with the identifier held stable by the resolution layer above. Pure self-certifying identifiers are immutable, so rotation and resolution arrive together. @@ -129,5 +133,6 @@ These do not change the genesis anchor and are added when needed: - [ADR#0037: Agent Identity Governance: Decentralized Verification under Governed Authority](./0037-agent-identity-governance.md) - [ADR#0038: Agent Identity Cryptographic Suite and Crypto-Agility](./0038-agent-identity-crypto-suite.md) - [ADR#0039: Self-Authenticating Event Provenance](./0039-self-authenticating-event-provenance.md) +- [ADR#0053: External OIDC Federation Surface for Agent Identity](./0053-external-oidc-federation-surface.md) - [Buzz: Nostr-based agent identity](https://github.com/block/buzz) - [ADR index](./index.md) diff --git a/docs/adr/0038-agent-identity-crypto-suite.md b/docs/adr/0038-agent-identity-crypto-suite.md index 7ba3bc804..d3c9c211a 100644 --- a/docs/adr/0038-agent-identity-crypto-suite.md +++ b/docs/adr/0038-agent-identity-crypto-suite.md @@ -155,8 +155,43 @@ Decision 3, never as a replacement for the Ed25519 root: is a governance question, recorded as a stance in [ADR#0037](./0037-agent-identity-governance.md), not a decision this ADR makes on its own. +- **RSA-2048 with RS256**, only where an external identity provider's + federation surface refuses everything else. This trigger is already met in + practice rather than hypothetical: Microsoft Entra's workload identity + federation supports only RS256-signed issuers, and additionally requires + that the published JWK set contain *nothing but* RSA signing keys, so EC + and OKP keys cannot merely sit alongside an added RSA one. Every algorithm + Decision 1 and the two profiles above name is excluded by that constraint. + This profile is adopted for one purpose only, signing assertions presented + to a third-party IdP, and it never becomes an agent's root anchor; the + "only RSA keys" requirement also forces a *separate* published key set + rather than an extra key in the AAuth well-known documents, which is why + the surface itself is decided in + [ADR#0053](./0053-external-oidc-federation-surface.md) rather than here. -### 5. Post-quantum path +### 5. Where the implementation currently stands, and where it diverges + +Decision 1 names Ed25519 as the default and the two curves above as +conditional. The shipped code does not match that yet, and the divergence is +recorded here rather than left to be discovered: + +- `trogon-aauth-verify` admits `ES256 | ES384 | EdDSA`, which is the + allowlist Decision 3 requires and a superset of the default. Its PoP path + derives the algorithm from the confirmed key's own `kty`/`crv` rather than + from the presented header, so algorithm confusion is structurally + unavailable there, not merely filtered. +- `trogon-jwks-publisher` is P-256 only. `jwk_from_ec_pkcs8_pem` hardcodes + the curve and the Agent Provider mints with `ES256`. The Agent Provider can + therefore issue the *conditional* profile and cannot issue the declared + default. Closing this means adding Ed25519 issuance to the publisher, not + changing the decision above: P-256 issuance stays supported under its + hardware-custody trigger. + +Nothing about this divergence is load-bearing for the anchor, precisely +because Decision 3 makes every key and signature name its own algorithm. It +is recorded as a gap to close, not as an amendment to the default. + +### 6. Post-quantum path Every elliptic-curve scheme named above, Ed25519, P-256, and secp256k1 alike, falls to Shor's algorithm on a cryptographically relevant quantum computer; @@ -182,6 +217,19 @@ threat timeline, not an emergency response to an already-broken algorithm. presumes one fixed curve in the identifier itself, are rejected. Identifiers and keys in this platform are always algorithm-tagged, so an identifier never has to be reinterpreted if the algorithm it names is later retired. +- The verifier's allowlist is now enforced on three axes, not one: the key's + material (`kty`/`crv`) must match the algorithm's family, and where a + publisher declares `use`, `key_ops`, or `alg` on a JWK + ([RFC 7517](https://www.rfc-editor.org/rfc/rfc7517) sections 4.2 to 4.4) + those declarations are honored, so a key published for encryption or pinned + to a different algorithm is not conscripted into signature verification + because its curve happens to line up. Absent members stay permissive, since + they are optional in the RFC and a federated deployment resolves key sets + this platform did not publish. +- Adopting the RSA profile of Decision 4 means operating a second published + key set with its own rotation, because the IdP constraint that triggers it + forbids mixing key types in one document. That cost is the reason the + profile is conditional rather than default. - Private key custody is unaffected by this ADR and remains on the security plane defined by [ADR#0023](./0023-secret-management-and-key-custody-direction.md) and [ADR#0033](./0033-two-tier-key-custody-product-model.md). This ADR @@ -197,8 +245,10 @@ threat timeline, not an emergency response to an already-broken algorithm. - [ADR#0036: Agent Self-Certifying Cryptographic Identity](./0036-agent-self-certifying-identity.md) - [ADR#0037: Agent Identity Governance](./0037-agent-identity-governance.md) - [ADR#0039: Self-Authenticating Event Provenance](./0039-self-authenticating-event-provenance.md) +- [ADR#0053: External OIDC Federation Surface for Agent Identity](./0053-external-oidc-federation-surface.md) - [RFC 6979: Deterministic Usage of DSA and ECDSA](https://www.rfc-editor.org/rfc/rfc6979) - [RFC 7515: JSON Web Signature (JWS)](https://www.rfc-editor.org/rfc/rfc7515) +- [RFC 7517: JSON Web Key (JWK)](https://www.rfc-editor.org/rfc/rfc7517) - [RFC 7638: JSON Web Key (JWK) Thumbprint](https://www.rfc-editor.org/rfc/rfc7638) - [RFC 8032: Edwards-Curve Digital Signature Algorithm (EdDSA)](https://www.rfc-editor.org/rfc/rfc8032) - [RFC 8037: CFRG Elliptic Curve Diffie-Hellman and Signatures in JOSE](https://www.rfc-editor.org/rfc/rfc8037) diff --git a/docs/adr/0049-revocation-latency-target.md b/docs/adr/0049-revocation-latency-target.md index 69fdb0e1a..004a1617d 100644 --- a/docs/adr/0049-revocation-latency-target.md +++ b/docs/adr/0049-revocation-latency-target.md @@ -49,6 +49,13 @@ in dashboards. - The cache TTL plus jitter (at most 330 seconds) is the hard upper bound on staleness when the event path fails entirely; the alert on the event path exists precisely so the backstop is never the operative mechanism. +- The target is scoped to credentials this platform resolves. It does not + extend to standing that has been federated to an external identity + provider, which never consults this platform's revocation state; that + boundary is bounded by assertion TTL plus the remote provider's own cache + and is decided in + [ADR#0053](./0053-external-oidc-federation-surface.md). An offboarding that + must hold on both planes is not complete when this histogram says it is. - The numbers are working values. They are revisited once production stream metrics exist, and any change lands as an amendment to this ADR. diff --git a/docs/adr/0053-external-oidc-federation-surface.md b/docs/adr/0053-external-oidc-federation-surface.md new file mode 100644 index 000000000..a2c1be3b2 --- /dev/null +++ b/docs/adr/0053-external-oidc-federation-surface.md @@ -0,0 +1,227 @@ +--- +number: "0053" +slug: external-oidc-federation-surface +status: draft +date: 2026-08-07 +--- + +# ADR#0053: External OIDC Federation Surface for Agent Identity + +## Context + +[ADR#0036](./0036-agent-self-certifying-identity.md) anchors an +[agent](../glossary/agent) to a self-certifying key and defers a *global +resolution* layer, "publishing the identifier-to-current-key mapping to a +replicated substrate so parties outside this system can resolve it", with +root-key rotation deferred alongside it because the two arrive together. +[ADR#0038](./0038-agent-identity-crypto-suite.md) pins the suite and makes +crypto-agility structural. [ADR#0017](./0017-aauth-agent-authentication.md) +ships the verification path and a well-known publisher. None of them says how +an agent identity is presented to a party that is not this platform and does +not speak AAuth. + +That gap is now load-bearing, because the shape of the answer is not the one +the deferred layer implies. Every cloud identity provider that accepts an +external workload assertion, Microsoft Entra's workload identity federation +being the concrete case examined here, consumes exactly one thing: an OIDC +discovery document plus a JWKS, over HTTPS, at a stable issuer URL. It does +not resolve DIDs, read transparency logs, or fetch AAuth well-known +documents. The pragmatic substrate for +[ADR#0036](./0036-agent-self-certifying-identity.md)'s deferred layer is the +boring one that already has universal client support. + +Three facts about that surface make it a decision rather than an +implementation detail: + +1. **It is not the surface we already publish.** + `trogon-jwks-publisher` serves `GET /.well-known/{dwk}` for the four + filenames the AAuth draft registers, and each response is a bare `JwkSet`. + There is no `/.well-known/openid-configuration`, no `issuer` field, and no + `jwks_uri`. A key set and a discovery document both publish trust material + over HTTPS, and they are still different protocol surfaces: an IdP handed + an AAuth `dwk` URL as an issuer will fail discovery before it ever looks at + a key. + +2. **Our entire algorithm allowlist is excluded.** Entra's federation supports + only RS256-signed issuers and additionally requires the published key set + to contain *nothing but* RSA signing keys. `trogon-aauth-verify` admits + `ES256 | ES384 | EdDSA`; `trogon-jwks-publisher` mints ES256; + [ADR#0038](./0038-agent-identity-crypto-suite.md) makes Ed25519 the default + root. Every one of those is refused. The "nothing but RSA" half is the + sharper constraint, because it means an RSA key cannot simply be added + alongside the existing ones in a shared document. + +3. **The mapping, not the token, is where the trust decision lives.** An IdP + binds an external assertion to a governed identity through an exact, + case-sensitive match on `iss`, `sub`, and `aud`, with no wildcards and a + hard cap on how many such bindings one identity may hold. A valid assertion + proves which workload is running; it grants nothing. If the mapping is + wrong, a short-lived, correctly-signed token only lets the wrong workload + become the wrong principal faster. + +The platform also already holds a position this surface must not quietly +contradict. [ADR#0037](./0037-agent-identity-governance.md) requires that "the +signature is valid" and "the agent is currently authorized" never collapse +into one check, and [ADR#0049](./0049-revocation-latency-target.md) commits to +a p99 five-second revocation-to-invalidation target. Neither survives a +federation boundary unamended: a third-party IdP will not call this +platform's status list. + +## Decision + +### 1. The federation surface is a separate issuer, not an added key + +External OIDC federation is served by its own issuer origin, publishing its +own `/.well-known/openid-configuration` and its own `jwks_uri`, distinct from +the AAuth well-known documents of +[ADR#0017](./0017-aauth-agent-authentication.md). The two are not merged and +the AAuth `dwk` documents are never advertised as an OIDC issuer. + +This is forced, not stylistic. The IdP constraint that the key set contain +only RSA keys is incompatible with an AAuth document holding the EC and OKP +keys the mesh verifies against, so one document cannot serve both. Keeping +them separate also keeps the blast radius separate: the federation issuer is +internet-facing and consumed by parties outside our control, while the AAuth +documents serve the mesh. + +### 2. RS256 only on that surface, under the conditional profile + +Assertions minted for external federation are signed RS256 over RSA-2048 +keys, adopted as the conditional profile +[ADR#0038](./0038-agent-identity-crypto-suite.md) Decision 4 records. This +profile exists for this purpose and no other. It is never an agent's root +anchor, never enters the `did:key` identifier, and never widens +`trogon-aauth-verify`'s inbound allowlist: this platform *signs* RS256 here +and continues to refuse to *verify* it on the mesh. + +### 3. The binding is a first-class resource with exact-match semantics + +The mapping from a platform-attested identity to the external principal it may +assume is modeled as an explicit, enumerable, auditable resource, anchored in +the project hierarchy of +[ADR#0046](./0046-project-anchored-resource-hierarchy.md), with exact-match +semantics on issuer, subject, and audience. Not a pattern, not a policy +expression, not a wildcard. + +Pattern matching is rejected for this binding even where an IdP offers it. It +reduces the number of objects to manage and changes the risk model in the same +move: a subject pattern makes the security of the whole federation depend on +how tightly the upstream registration policy constrains what subjects can be +minted, which relocates the trust decision somewhere it is much harder to +audit. Enumerable bindings are the property worth paying object count for. + +### 4. One audience per assertion, minted at the moment of exchange + +An assertion minted for an external IdP carries exactly one audience, the one +that IdP requires. Tokens minted for any internal purpose are never presented +as external assertions and vice versa: a multi-audience bearer token is +replayable to every recipient it names. + +Because such an assertion is a bearer credential for its whole validity +window, it is requested only when an exchange is about to happen, never +cached; never logged, traced, or written to an event; and never placed +anywhere an agent's model context can reach it, which includes prompts, tool +arguments, and tool results. What *is* cached is the resource token the +exchange returns, keyed by its own lifetime, never the assertion that bought +it. + +This is a weaker posture than the platform holds for its own ingress, where +[ADR#0051](./0051-fully-bound-request-signing.md) binds a request token to its +target and payload with a nonce, and that asymmetry is inherent: the external +IdP's protocol defines what it accepts, and it accepts a bearer assertion. +The mitigations above are what is available, so they are requirements rather +than hardening. + +### 5. Revocation across the boundary is TTL plus unbinding, and it is slower + +[ADR#0049](./0049-revocation-latency-target.md)'s five-second target is +in-platform: it measures a revocation event reaching this platform's own +runtime projection. It does not extend across a federation boundary, because +the external IdP does not consult this platform's status list and +[ADR#0037](./0037-agent-identity-governance.md)'s mandatory status check +cannot be pushed onto it. + +Standing across the boundary is therefore bounded by two things only: the +assertion TTL, which is kept short and is the primary lever, and deleting the +binding of Decision 3, which propagates asynchronously through the IdP's own +caches and is not immediate. Both are measured and neither is presented as +equivalent to in-platform revocation. Offboarding an agent must revoke on both +planes; revoking only in-platform leaves a window in which an already-issued +external resource token still works. + +### 6. Assertion validity is scoped, and delegated authority is not implied + +An assertion presented on this surface authenticates a *client*: it says which +platform-attested identity is calling. It never carries delegated user +authority. Where a downstream call acts on a person's behalf, that +authority comes from the person-linked principal +[ADR#0017](./0017-aauth-agent-authentication.md) Decision 5 already makes +authoritative, carried separately. This surface is not a shortcut around +delegated consent, and an integration that treats a successful federation +exchange as sufficient for an on-behalf-of call has widened authority without +anyone deciding to. + +### 7. Deferred, and named so it is not assumed + +Two things this decision deliberately does not settle: + +- **Which principal the downstream provider sees.** Today + [ADR#0032](./0032-model-route-and-credential-binding.md) Decision 4 brokers + hosted model access through a session-scoped proxy holding platform + credentials, so the provider's own audit log names the platform, not the + agent. Exchanging a per-agent assertion so the provider attributes calls to + the agent is the strictly better shape and is not built. Until it is, agent + attribution at a hosted provider is a platform-side record only. +- **Workload attestation.** Proof of possession + ([ADR#0017](./0017-aauth-agent-authentication.md), + [ADR#0036](./0036-agent-self-certifying-identity.md)) proves control of a + key, not that the process holding it is the runtime the platform expects; a + leaked private key satisfies it. Binding key use to an attested workload is + the deferred operational-key tier of + [ADR#0036](./0036-agent-self-certifying-identity.md), and this surface + inherits that limitation rather than fixing it. + +## Consequences + +- [ADR#0036](./0036-agent-self-certifying-identity.md)'s deferred global + resolution layer gains a concrete, unglamorous shape for the external case: + an OIDC issuer with a JWKS. It does not close the layer, because a DID + resolution story is still what makes the *identifier* portable; it closes + the narrower question of how a non-AAuth party verifies us today. +- A second published key set exists, with its own rotation schedule, its own + overlap window, and its own monitoring, and it must be coordinated with + SPIRE-style key rotation on the signing side and the IdP's cache on the + consuming side. Four rotation responsibilities that must not drift apart is + the standing operational cost of this decision. +- Federation failures need to be distinguishable in telemetry + ([ADR#0008](./0008-opentelemetry-observability.md)): a missing binding, an + unreachable or mismatched issuer, and IdP cache lag after a binding change + have different remediations, and the last class is expected to require + retry rather than a fix. +- The platform now signs with an algorithm it refuses to verify. That is + intentional and worth stating plainly, because the asymmetry looks like a + bug to anyone reading only one side of it. +- Adopting this surface expands what an attacker gains from compromising the + signing path: an assertion minted here is accepted by a third party under + the mapping's authority, and revoking it is slower than revoking anything + in-platform. + +## References + +- [ADR#0008: OpenTelemetry Observability](./0008-opentelemetry-observability.md) +- [ADR#0017: AAuth Agent Authentication over a Trogon NATS PoP Binding](./0017-aauth-agent-authentication.md) +- [ADR#0032: Model Route and Credential Binding](./0032-model-route-and-credential-binding.md) +- [ADR#0036: Agent Self-Certifying Cryptographic Identity](./0036-agent-self-certifying-identity.md) +- [ADR#0037: Agent Identity Governance: Decentralized Verification under Governed Authority](./0037-agent-identity-governance.md) +- [ADR#0038: Agent Identity Cryptographic Suite and Crypto-Agility](./0038-agent-identity-crypto-suite.md) +- [ADR#0046: Project-Anchored Resource Hierarchy for the Credential Platform](./0046-project-anchored-resource-hierarchy.md) +- [ADR#0049: Revocation Propagation Latency Target](./0049-revocation-latency-target.md) +- [ADR#0051: Fully Bound Per-Request Signing Contract](./0051-fully-bound-request-signing.md) +- [RFC 7517: JSON Web Key (JWK)](https://www.rfc-editor.org/rfc/rfc7517) +- [RFC 8414: OAuth 2.0 Authorization Server Metadata](https://www.rfc-editor.org/rfc/rfc8414) +- [RFC 8615: Well-Known Uniform Resource Identifiers](https://www.rfc-editor.org/rfc/rfc8615) +- [OpenID Connect Discovery 1.0](https://openid.net/specs/openid-connect-discovery-1_0.html) +- [Microsoft Entra: workload identity federation considerations](https://github.com/MicrosoftDocs/entra-docs/blob/main/docs/workload-id/workload-identity-federation-considerations.md) +- [SPIFFE Workload Identity and Entra Agent ID: the trust gap](https://dev.to/astaykov/your-spiffe-workload-can-authenticate-as-an-entra-agent-id-but-mind-the-trust-gap-3969) +- [From JWT-SVID to Entra Agent ID: a working SPIFFE PoC](https://dev.to/astaykov/from-jwt-svid-to-entra-agent-id-a-working-spiffe-poc-ic4) +- [ADR index](./index.md) diff --git a/docs/adr/index.md b/docs/adr/index.md index 6ea74c18c..52e58b78a 100644 --- a/docs/adr/index.md +++ b/docs/adr/index.md @@ -58,3 +58,4 @@ future implementation work. - [ADR#0050: Signed Proof-of-Possession as the Strongly Recommended Caller Authentication](./0050-signed-first-caller-authentication.md) - [ADR#0051: Fully Bound Per-Request Signing Contract](./0051-fully-bound-request-signing.md) - [ADR#0052: Cloud KMS Auto-Unseal Is Mandatory for Production OpenBao](./0052-cloud-kms-production-seal.md) +- [ADR#0053: External OIDC Federation Surface for Agent Identity (Draft)](./0053-external-oidc-federation-surface.md) From 67e4f5c9ece62a6a03a250c3d1bf06d0f46bba3a Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:40:08 -0400 Subject: [PATCH 10/32] fix(a2a-identity-types): stop a replayable credential from reaching a log A bearer token in a log is a usable credential for the rest of its lifetime, and these two carried a redacted Display next to a derived Debug, which is the form tracing actually records. Signed-off-by: Yordis Prieto --- .../crates/a2a/a2a-identity-types/src/jwt.rs | 16 ++++++++++++++-- .../a2a/a2a-identity-types/src/jwt/tests.rs | 13 +++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/rsworkspace/crates/a2a/a2a-identity-types/src/jwt.rs b/rsworkspace/crates/a2a/a2a-identity-types/src/jwt.rs index f6d97fab8..aebe74e8b 100644 --- a/rsworkspace/crates/a2a/a2a-identity-types/src/jwt.rs +++ b/rsworkspace/crates/a2a/a2a-identity-types/src/jwt.rs @@ -10,9 +10,15 @@ use crate::error::JwtError; /// Compact JWT string suitable for header transport. Validates shape on /// construction (3 dotted segments) without verifying signature. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub struct CallerJwtHeaderValue(String); +impl fmt::Debug for CallerJwtHeaderValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("CallerJwtHeaderValue").field(&"").finish() + } +} + impl CallerJwtHeaderValue { /// Builds a header value from a [`MintedUserJwt`]. The minted JWT is already /// shape-validated at construction, so this is infallible. @@ -54,9 +60,15 @@ impl fmt::Display for CallerJwtHeaderValue { /// User JWT minted for bridge/gateway consumption; carried as the inner /// `nats.jwt` on wire responses. Validates shape on construction but does not /// verify the signature — that lives gateway-side. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub struct MintedUserJwt(String); +impl fmt::Debug for MintedUserJwt { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("MintedUserJwt").field(&"").finish() + } +} + impl MintedUserJwt { /// Constructs a minted JWT from an already-shape-valid compact JWT string. /// Returns an error if the input is not three non-empty `.`-separated diff --git a/rsworkspace/crates/a2a/a2a-identity-types/src/jwt/tests.rs b/rsworkspace/crates/a2a/a2a-identity-types/src/jwt/tests.rs index c75c911e3..036d3a827 100644 --- a/rsworkspace/crates/a2a/a2a-identity-types/src/jwt/tests.rs +++ b/rsworkspace/crates/a2a/a2a-identity-types/src/jwt/tests.rs @@ -125,3 +125,16 @@ fn ensure_fresh_rejects_future_nbf() { let err = MintedUserJwt::new(token).unwrap().ensure_fresh().unwrap_err(); assert!(matches!(err, JwtError::Decode(ref msg) if msg.contains("not yet valid"))); } + +#[test] +fn debug_does_not_leak_bearer_credentials() { + let minted = MintedUserJwt::new("hhh.ppp.sss").expect("valid shape"); + let minted_dbg = format!("{minted:?}"); + assert!(!minted_dbg.contains("ppp"), "{minted_dbg}"); + assert!(minted_dbg.contains(""), "{minted_dbg}"); + + let header = CallerJwtHeaderValue::from_minted(&minted); + let header_dbg = format!("{header:?}"); + assert!(!header_dbg.contains("ppp"), "{header_dbg}"); + assert!(header_dbg.contains(""), "{header_dbg}"); +} From 6181ed43faaa91eeb9329c034ed2761b1607b672 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:40:09 -0400 Subject: [PATCH 11/32] fix(a2a-auth-callout): stop a replayable credential from reaching a log The neighbouring wire claim type already hand-redacts what it prints; the inbound assertion, the minted user JWT, and the signed response carrying it did not, and each is replayable for as long as it is valid. Signed-off-by: Yordis Prieto --- .../a2a/a2a-auth-callout/src/caller_jwt_header.rs | 8 +++++++- .../src/caller_jwt_header/tests.rs | 8 ++++++++ .../a2a/a2a-auth-callout/src/credentials/oidc.rs | 9 ++++++++- .../a2a-auth-callout/src/credentials/oidc/tests.rs | 9 +++++++++ .../crates/a2a/a2a-auth-callout/src/jwt/mod.rs | 8 +++++++- .../crates/a2a/a2a-auth-callout/src/jwt/tests.rs | 8 ++++++++ .../src/wire/callout_auth_response_claims.rs | 14 +++++++++++++- .../src/wire/callout_auth_response_claims/tests.rs | 14 ++++++++++++++ 8 files changed, 74 insertions(+), 4 deletions(-) diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header.rs index b6b297fa8..4e65c7ec4 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header.rs @@ -2,9 +2,15 @@ use std::fmt; use crate::jwt::{JwtError, MintedUserJwt}; -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub struct CallerJwtHeaderValue(String); +impl fmt::Debug for CallerJwtHeaderValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("CallerJwtHeaderValue").field(&"").finish() + } +} + impl CallerJwtHeaderValue { pub fn from_minted(jwt: &MintedUserJwt) -> Self { Self(jwt.as_str().to_owned()) diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header/tests.rs index 1ff330edd..1366eee06 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/caller_jwt_header/tests.rs @@ -41,3 +41,11 @@ fn display_redacts_value() { let header = CallerJwtHeaderValue::parse("a.b.c").unwrap(); assert_eq!(header.to_string(), ""); } + +#[test] +fn debug_does_not_leak_the_token() { + let header = CallerJwtHeaderValue::parse("hhh.ppp.sss").unwrap(); + let dbg = format!("{header:?}"); + assert!(!dbg.contains("ppp"), "{dbg}"); + assert!(dbg.contains(""), "{dbg}"); +} diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs index 8ea4417cb..a63106d69 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs @@ -1,3 +1,4 @@ +use std::fmt; use std::sync::Arc; use crate::constants::OIDC_ALLOWED_ALGORITHMS; @@ -51,9 +52,15 @@ impl OidcClientId { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub struct BearerToken(String); +impl fmt::Debug for BearerToken { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("BearerToken").field(&"").finish() + } +} + impl BearerToken { pub fn new(token: impl Into) -> Self { Self(token.into()) diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs index 59b444dab..ce15ccf9b 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs @@ -583,3 +583,12 @@ async fn verify_accepts_a_jwk_that_declares_no_purpose_at_all() { .expect("absent RFC 7517 members stay permissive"); assert_eq!(claims.sub.as_str(), "user-guard"); } + +#[test] +fn bearer_token_debug_does_not_leak_the_assertion() { + let token = BearerToken::new("hhh.ppp.sss"); + let dbg = format!("{token:?}"); + assert!(!dbg.contains("ppp"), "{dbg}"); + assert!(dbg.contains(""), "{dbg}"); + assert_eq!(token.as_str(), "hhh.ppp.sss"); +} diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/mod.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/mod.rs index 9877879b2..95dc3d623 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/mod.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/mod.rs @@ -166,9 +166,15 @@ impl SpiceDbPrincipal { } /// HS256 User JWT minted for bridge/gateway consumption (inner `nats.jwt` on wire responses). -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub struct MintedUserJwt(String); +impl fmt::Debug for MintedUserJwt { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("MintedUserJwt").field(&"").finish() + } +} + impl MintedUserJwt { /// Wrap a token after validating compact-JWT shape (three non-empty /// dot-separated segments, non-empty input after trimming). Mirrors the diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/tests.rs index e5070da7e..a2ceda5e2 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/jwt/tests.rs @@ -417,3 +417,11 @@ fn ensure_fresh_rejects_not_yet_valid_token() { let err = token.ensure_fresh().unwrap_err(); assert!(matches!(err, JwtError::Decode(ref m) if m.contains("not yet valid"))); } + +#[test] +fn minted_user_jwt_debug_does_not_leak_the_token() { + let minted = MintedUserJwt::new("hhh.ppp.sss").expect("valid shape"); + let dbg = format!("{minted:?}"); + assert!(!dbg.contains("ppp"), "{dbg}"); + assert!(dbg.contains(""), "{dbg}"); +} diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims.rs index 8b90fe292..80abe2aca 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims.rs @@ -1,3 +1,5 @@ +use std::fmt; + use nats_jwt_rs::authorization::AuthResponse; use nkeys::{KeyPair, XKey}; @@ -6,11 +8,21 @@ use crate::error::AuthCalloutError; use crate::jwt::MintedUserJwt; /// Callout-signed authorization **response** JWT for `$SYS.REQ.USER.AUTH` reply. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct CalloutAuthResponseClaims { encoded: String, } +/// `encoded` carries the minted user JWT inside `nats.jwt`, so printing it +/// would put a usable credential wherever the response is traced. +impl fmt::Debug for CalloutAuthResponseClaims { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("CalloutAuthResponseClaims") + .field("encoded", &"") + .finish() + } +} + impl CalloutAuthResponseClaims { pub fn success( request: &ServerAuthRequestClaims, diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims/tests.rs index fc811ae34..3d40843eb 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/wire/callout_auth_response_claims/tests.rs @@ -111,3 +111,17 @@ fn into_wire_bytes_errors_when_server_xkey_but_no_account_xkey() { let err = resp.into_wire_bytes(&req, None).unwrap_err(); assert!(matches!(err, AuthCalloutError::WireFormat(_))); } + +#[test] +fn debug_does_not_leak_the_encoded_response() { + let (req, callout) = fixture_request(); + let resp = CalloutAuthResponseClaims::success( + &req, + &MintedUserJwt::new("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ4In0.c2ln").unwrap(), + &callout, + ) + .unwrap(); + let dbg = format!("{resp:?}"); + assert!(!dbg.contains(resp.as_jwt_str()), "{dbg}"); + assert!(dbg.contains(""), "{dbg}"); +} From 0487fb19cf8767cf832fb28c409df6dc5cc2af62 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 18:40:09 -0400 Subject: [PATCH 12/32] fix(trogon-jwks-publisher): refuse a key set no consumer can select from The builder already exists so misconfiguration fails at startup rather than remotely, but it vetted only the filename, leaving a rotation that reuses or omits a kid to surface as a verification failure at the consumer with nothing visible on this side. Signed-off-by: Yordis Prieto --- .../trogon-jwks-publisher/src/publisher.rs | 45 ++++++++++++- .../src/publisher/tests.rs | 64 +++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs index d1417be9c..faf5a188d 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs @@ -7,7 +7,7 @@ //! host service mounts at its own root so `GET /.well-known/{dwk}` resolves //! against a configured set of `JwkSet`s. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use axum::Router; use axum::extract::{Path, State}; @@ -54,6 +54,10 @@ pub enum PublisherError { UnknownDwk(String), #[error("dwk filename {0:?} was registered more than once")] DuplicateDwk(String), + #[error("dwk {dwk:?} publishes key id {kid:?} more than once")] + DuplicateKeyId { dwk: String, kid: String }, + #[error("dwk {dwk:?} publishes {keys} keys and at least one omits `kid`; only a single-key set may omit it")] + MissingKeyId { dwk: String, keys: usize }, #[error("invalid EC PKCS8 PEM for kid {kid:?}: {source}")] InvalidPem { kid: String, @@ -71,6 +75,44 @@ fn is_known_dwk(dwk: &str) -> bool { known_dwk_filenames().contains(&dwk) } +/// A published set is selected from by `kid`: [`JwkSet::find`] matches the JWT +/// header's `kid` against `common.key_id`, and `trogon-aauth-verify`'s +/// `pick_jwk` does the same, falling back to the sole compatible key only when +/// the set holds exactly one. +/// +/// That makes two shapes unusable. Keys sharing a `kid` resolve silently to +/// whichever copy is enumerated first, so a rotation that reuses the previous +/// `kid` keeps every consumer pinned to the retired key. A key with no `kid` +/// cannot be selected at all once the set holds more than one, which is exactly +/// the state a rotation overlap creates. Both fail at the consumer, remotely, +/// with nothing to see on this side, so they are refused at startup instead -- +/// the same reason the dwk filename is validated here rather than left to +/// surface as a 404. +fn validate_selectable_by_kid(dwk: &str, set: &JwkSet) -> Result<(), PublisherError> { + let multi_key = set.keys.len() > 1; + let mut seen: HashSet<&str> = HashSet::new(); + for jwk in &set.keys { + match jwk.common.key_id.as_deref() { + Some(kid) => { + if !seen.insert(kid) { + return Err(PublisherError::DuplicateKeyId { + dwk: dwk.to_owned(), + kid: kid.to_owned(), + }); + } + } + None if multi_key => { + return Err(PublisherError::MissingKeyId { + dwk: dwk.to_owned(), + keys: set.keys.len(), + }); + } + None => {} + } + } + Ok(()) +} + /// Build a public EC P-256 JWK from a PKCS8 PEM private key, mirroring /// `trogon-aauth-sdk`'s `public_jwk` helper but returning a typed /// `jsonwebtoken::jwk::Jwk` (this crate already depends on `jsonwebtoken` for @@ -136,6 +178,7 @@ impl JwksPublisherConfigBuilder { if self.entries.contains_key(&dwk) { return Err(PublisherError::DuplicateDwk(dwk)); } + validate_selectable_by_kid(&dwk, &set)?; self.entries.insert(dwk, set); Ok(self) } diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs index 6c8502838..16ce3ca47 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs @@ -97,3 +97,67 @@ fn cache_max_age_renders_header_value() { assert_eq!(max_age.header_value(), "max-age=300"); assert_eq!(max_age.as_secs(), 300); } + +fn keyed(kid: &str) -> Jwk { + jwk_from_ec_pkcs8_pem(TEST_PEM, kid).expect("valid pem") +} + +fn unkeyed() -> Jwk { + let mut jwk = keyed("k1"); + jwk.common.key_id = None; + jwk +} + +#[test] +fn builder_accepts_a_rotation_overlap_with_distinct_key_ids() { + let cfg = JwksPublisherConfigBuilder::new(CacheMaxAge::new(60)) + .with_jwk_set( + DWK_AGENT, + JwkSet { + keys: vec![keyed("current"), keyed("previous")], + }, + ) + .expect("distinct kids stay selectable") + .build(); + assert_eq!(cfg.entries.get(DWK_AGENT).expect("present").keys.len(), 2); +} + +#[test] +fn builder_rejects_a_set_that_repeats_a_key_id() { + let err = JwksPublisherConfigBuilder::new(CacheMaxAge::new(60)) + .with_jwk_set( + DWK_AGENT, + JwkSet { + keys: vec![keyed("same"), keyed("same")], + }, + ) + .unwrap_err(); + assert!( + matches!(&err, PublisherError::DuplicateKeyId { kid, .. } if kid.as_str() == "same"), + "{err}" + ); +} + +#[test] +fn builder_rejects_a_multi_key_set_holding_an_unidentified_key() { + let err = JwksPublisherConfigBuilder::new(CacheMaxAge::new(60)) + .with_jwk_set( + DWK_AGENT, + JwkSet { + keys: vec![keyed("current"), unkeyed()], + }, + ) + .unwrap_err(); + assert!(matches!(&err, PublisherError::MissingKeyId { keys: 2, .. }), "{err}"); +} + +#[test] +fn builder_allows_a_lone_key_without_a_key_id() { + // `pick_jwk` resolves a one-key set without consulting `kid`, and RFC 7517 + // section 4.5 leaves the member optional, so this shape stays publishable. + let cfg = JwksPublisherConfigBuilder::new(CacheMaxAge::new(60)) + .with_jwk_set(DWK_AGENT, JwkSet { keys: vec![unkeyed()] }) + .expect("a sole key needs no kid") + .build(); + assert_eq!(cfg.entries.get(DWK_AGENT).expect("present").keys.len(), 1); +} From 87123bca885b634845bd8040e65cb3922f52bed1 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 19:12:49 -0400 Subject: [PATCH 13/32] fix(trogon-gateway): keep a replay bound from lapsing on an already-provisioned stream A stream created before the dedup window was paired with the signature timestamp tolerance kept JetStream's default window, so the declared bound never took effect where it mattered most: a deployment that had already been running. Signed-off-by: Yordis Prieto --- .../src/source/gitlab/server.rs | 6 ++- .../src/source/gitlab/server/tests.rs | 22 +++++++++++ .../trogon-nats/src/jetstream/client.rs | 8 ++++ .../trogon-nats/src/jetstream/mocks.rs | 37 ++++++++++++++----- .../trogon-nats/src/jetstream/traits.rs | 13 +++++++ 5 files changed, 75 insertions(+), 11 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs index 75d5aba98..5774567c3 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs @@ -84,7 +84,11 @@ struct AppState { } pub async fn provision(js: &C, config: &GitlabConfig) -> Result<(), C::Error> { - js.get_or_create_stream(async_nats::jetstream::stream::Config { + // Reconciled rather than created-if-absent: `duplicate_window` is the + // replay bound paired with the signature timestamp tolerance below, so a + // stream provisioned before that pairing existed would otherwise keep + // JetStream's default window and leave replays live past it. + js.create_or_update_stream(async_nats::jetstream::stream::Config { name: config.stream_name.as_str().to_owned(), subjects: vec![format!("{}.>", config.subject_prefix)], duplicate_window: config.timestamp_tolerance.into(), diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs index 201b586a1..c63b8c7da 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs @@ -171,6 +171,28 @@ async fn provision_creates_stream() { assert_eq!(streams[0].duplicate_window, Duration::from_secs(300)); } +#[tokio::test] +async fn provision_widens_the_dedup_window_on_an_already_provisioned_stream() { + let _guard = tracing_guard(); + let js = MockJetStreamContext::new(); + // A deployment that provisioned GITLAB before the dedup window was paired + // with the signature tolerance: JetStream's own default, not ours. + js.get_or_create_stream(async_nats::jetstream::stream::Config { + name: "GITLAB".to_owned(), + duplicate_window: Duration::from_secs(120), + ..Default::default() + }) + .await + .unwrap(); + + provision(&js, &test_config()).await.unwrap(); + + let streams = js.created_streams(); + assert_eq!(streams.len(), 1, "reconciled in place rather than duplicated"); + assert_eq!(streams[0].name, "GITLAB"); + assert_eq!(streams[0].duplicate_window, Duration::from_secs(300)); +} + #[tokio::test] async fn provision_propagates_error() { let _guard = tracing_guard(); diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs index deec432e9..14d4b997c 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs @@ -38,6 +38,14 @@ impl JetStreamContext for NatsJetStreamClient { ) -> Result { self.context.get_or_create_stream(config).await } + + async fn create_or_update_stream + Send>( + &self, + config: S, + ) -> Result<(), async_nats::jetstream::context::CreateStreamError> { + self.context.create_or_update_stream(config.into()).await?; + Ok(()) + } } pub type PublishError = async_nats::jetstream::context::PublishError; diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs index 405363086..86242184b 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs @@ -231,6 +231,16 @@ impl MockJetStreamContext { pub fn fail_next(&self) { *self.should_fail.lock().unwrap() = true; } + + fn take_failure(&self) -> bool { + let mut flag = self.should_fail.lock().unwrap(); + if *flag { + *flag = false; + true + } else { + false + } + } } impl Default for MockJetStreamContext { @@ -245,21 +255,28 @@ impl JetStreamContext for MockJetStreamContext { async fn get_or_create_stream + Send>(&self, config: S) -> Result<(), MockError> { let config = config.into(); - let should_fail = { - let mut flag = self.should_fail.lock().unwrap(); - if *flag { - *flag = false; - true - } else { - false - } - }; - if should_fail { + if self.take_failure() { return Err(MockError("simulated stream creation failure".to_string())); } self.created_streams.lock().unwrap().push(config); Ok(()) } + + /// Upserts by stream name, mirroring the server-side reconcile the real + /// context performs, so a test can tell it apart from + /// [`Self::get_or_create_stream`] leaving an existing stream alone. + async fn create_or_update_stream + Send>(&self, config: S) -> Result<(), MockError> { + let config = config.into(); + if self.take_failure() { + return Err(MockError("simulated stream creation failure".to_string())); + } + let mut streams = self.created_streams.lock().unwrap(); + match streams.iter_mut().find(|existing| existing.name == config.name) { + Some(existing) => *existing = config, + None => streams.push(config), + } + Ok(()) + } } #[derive(Clone, Debug)] diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs index ea11df3f1..700433fa0 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs @@ -20,6 +20,19 @@ pub trait JetStreamContext: Send + Sync + Clone + 'static { &self, config: S, ) -> impl Future> + Send; + + /// Reconcile a stream to `config`, creating it when it does not exist. + /// + /// [`Self::get_or_create_stream`] returns an already-existing stream + /// untouched, so a setting that carries a security property -- a + /// `duplicate_window` sized to a signature timestamp tolerance, say -- + /// would keep whatever value the stream was first created with and the + /// declared config would never take effect. Use this where the config has + /// to hold on an existing deployment rather than only on a fresh one. + fn create_or_update_stream + Send>( + &self, + config: S, + ) -> impl Future> + Send; } pub trait JetStreamKeyValueStatus: Send + Sync + Clone + 'static { From 94a917202ec91b1915d1839f586cc886232f29a8 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 19:12:53 -0400 Subject: [PATCH 14/32] chore(adr): keep the federation issuer key out of agent key management Recording the RSA profile under a blanket agent-binding claim invited an implementation that binds one issuer key per agent, when the identity belongs in the assertion claims and the key's custody belongs to the issuer. Signed-off-by: Yordis Prieto --- docs/adr/0038-agent-identity-crypto-suite.md | 28 +++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/docs/adr/0038-agent-identity-crypto-suite.md b/docs/adr/0038-agent-identity-crypto-suite.md index d3c9c211a..0d52fa42e 100644 --- a/docs/adr/0038-agent-identity-crypto-suite.md +++ b/docs/adr/0038-agent-identity-crypto-suite.md @@ -133,12 +133,13 @@ decision this ADR makes: it is what lets a broken algorithm be survived by rebinding a new key and rotating verifier policy rather than by redesigning the identity format itself. -### 4. Conditional profiles, added as bound keys, never as root replacements +### 4. Conditional profiles, each with its trigger, never as root replacements -Two further curves are recorded as conditional profiles, each with the -trigger that would justify adopting it. Either arrives as an additional key -bound to the agent's identity through the stream-attested binding of -Decision 3, never as a replacement for the Ed25519 root: +Three further profiles are recorded as conditional, each with the trigger that +would justify adopting it. None of them replaces the Ed25519 root. The two +curve profiles arrive as an additional key bound to the agent's identity +through the stream-attested binding of Decision 3; the third is not agent-bound +at all, and its entry records where it lives instead: - **P-256 (ES256)**, carried in [COSE](https://www.rfc-editor.org/rfc/rfc9052) ([RFC 9052](https://www.rfc-editor.org/rfc/rfc9052) and @@ -163,11 +164,18 @@ Decision 3, never as a replacement for the Ed25519 root: and OKP keys cannot merely sit alongside an added RSA one. Every algorithm Decision 1 and the two profiles above name is excluded by that constraint. This profile is adopted for one purpose only, signing assertions presented - to a third-party IdP, and it never becomes an agent's root anchor; the - "only RSA keys" requirement also forces a *separate* published key set - rather than an extra key in the AAuth well-known documents, which is why - the surface itself is decided in - [ADR#0053](./0053-external-oidc-federation-surface.md) rather than here. + to a third-party IdP, and it never becomes an agent's root anchor. It is + also the one profile here that is *not* bound to an agent identity through + Decision 3: the key is signing material belonging to the separate federation + issuer of [ADR#0053](./0053-external-oidc-federation-surface.md), whose + custody and rotation schedule are the issuer's and are governed apart from + agent key management. The agent identity an assertion speaks for travels in + that assertion's claims, not in the key that signs it, which is what keeps + one issuer key able to serve many agents without becoming any of their + anchors. The "only RSA keys" requirement additionally forces a *separate* + published key set rather than an extra key in the AAuth well-known + documents, which is why the surface itself is decided in ADR#0053 rather + than here. ### 5. Where the implementation currently stands, and where it diverges From 93f15ab8611a6aa7fc48d29df3e6da8f933970ae Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 19:12:57 -0400 Subject: [PATCH 15/32] chore(adr): stop offboarding from reading as complete at the platform edge Naming TTL and unbinding as the only bounds left the residual window ambiguous, and an operator sizing an offboarding procedure from it would have underestimated how long an already-issued external token keeps working. Signed-off-by: Yordis Prieto --- docs/adr/0053-external-oidc-federation-surface.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/adr/0053-external-oidc-federation-surface.md b/docs/adr/0053-external-oidc-federation-surface.md index a2c1be3b2..5bbe719c2 100644 --- a/docs/adr/0053-external-oidc-federation-surface.md +++ b/docs/adr/0053-external-oidc-federation-surface.md @@ -145,9 +145,15 @@ Standing across the boundary is therefore bounded by two things only: the assertion TTL, which is kept short and is the primary lever, and deleting the binding of Decision 3, which propagates asynchronously through the IdP's own caches and is not immediate. Both are measured and neither is presented as -equivalent to in-platform revocation. Offboarding an agent must revoke on both -planes; revoking only in-platform leaves a window in which an already-issued -external resource token still works. +equivalent to in-platform revocation. + +Both levers bound *future* exchanges: they stop the agent obtaining a new +external resource token. Neither reaches a token the external provider has +already issued, which stays valid until its own expiry or until that provider +revokes it, and no action on this platform shortens that. Offboarding an agent +must therefore revoke on both planes, and the residual window after the +platform-side revocation is the lifetime of the already-issued external token, +not the assertion TTL. ### 6. Assertion validity is scoped, and delegated authority is not implied From 192caa366547e82aabb5a9c6ef3a9c6cc9524f4c Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 19:36:19 -0400 Subject: [PATCH 16/32] fix(trogon-nats): stop stream provisioning from overwriting what the operator tuned Signed-off-by: Yordis Prieto --- .../src/source/gitlab/server.rs | 24 ++++++++++----- .../src/source/gitlab/server/tests.rs | 13 ++++++++- .../trogon-nats/src/jetstream/client.rs | 24 ++++++++++++--- .../trogon-nats/src/jetstream/mocks.rs | 20 ++++++++----- .../trogon-nats/src/jetstream/traits.rs | 29 ++++++++++++++----- 5 files changed, 81 insertions(+), 29 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs index 5774567c3..7a911a1f8 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs @@ -87,14 +87,22 @@ pub async fn provision(js: &C, config: &GitlabConfig) -> Re // Reconciled rather than created-if-absent: `duplicate_window` is the // replay bound paired with the signature timestamp tolerance below, so a // stream provisioned before that pairing existed would otherwise keep - // JetStream's default window and leave replays live past it. - js.create_or_update_stream(async_nats::jetstream::stream::Config { - name: config.stream_name.as_str().to_owned(), - subjects: vec![format!("{}.>", config.subject_prefix)], - duplicate_window: config.timestamp_tolerance.into(), - max_age: config.stream_max_age.into(), - ..Default::default() - }) + // JetStream's default window and leave replays live past it. The merge + // lists what this source owns; placement and limits stay the operator's. + js.create_or_reconcile_stream( + async_nats::jetstream::stream::Config { + name: config.stream_name.as_str().to_owned(), + subjects: vec![format!("{}.>", config.subject_prefix)], + duplicate_window: config.timestamp_tolerance.into(), + max_age: config.stream_max_age.into(), + ..Default::default() + }, + |current, desired| { + current.subjects = desired.subjects.clone(); + current.duplicate_window = desired.duplicate_window; + current.max_age = desired.max_age; + }, + ) .await?; let stream = config.stream_name.as_str(); diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs index c63b8c7da..23dde659c 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server/tests.rs @@ -176,10 +176,14 @@ async fn provision_widens_the_dedup_window_on_an_already_provisioned_stream() { let _guard = tracing_guard(); let js = MockJetStreamContext::new(); // A deployment that provisioned GITLAB before the dedup window was paired - // with the signature tolerance: JetStream's own default, not ours. + // with the signature tolerance: JetStream's own default, not ours. The + // replica count and storage tier are an operator's, set out of band. js.get_or_create_stream(async_nats::jetstream::stream::Config { name: "GITLAB".to_owned(), duplicate_window: Duration::from_secs(120), + num_replicas: 3, + storage: async_nats::jetstream::stream::StorageType::Memory, + max_bytes: 1_024, ..Default::default() }) .await @@ -191,6 +195,13 @@ async fn provision_widens_the_dedup_window_on_an_already_provisioned_stream() { assert_eq!(streams.len(), 1, "reconciled in place rather than duplicated"); assert_eq!(streams[0].name, "GITLAB"); assert_eq!(streams[0].duplicate_window, Duration::from_secs(300)); + assert_eq!(streams[0].num_replicas, 3, "operator-set replica count survives"); + assert_eq!( + streams[0].storage, + async_nats::jetstream::stream::StorageType::Memory, + "operator-set storage tier survives" + ); + assert_eq!(streams[0].max_bytes, 1_024, "operator-set limit survives"); } #[tokio::test] diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs index 14d4b997c..1f28dd004 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs @@ -39,11 +39,27 @@ impl JetStreamContext for NatsJetStreamClient { self.context.get_or_create_stream(config).await } - async fn create_or_update_stream + Send>( + async fn create_or_reconcile_stream( &self, - config: S, - ) -> Result<(), async_nats::jetstream::context::CreateStreamError> { - self.context.create_or_update_stream(config.into()).await?; + desired: S, + merge: F, + ) -> Result<(), async_nats::jetstream::context::CreateStreamError> + where + S: Into + Send, + F: FnOnce(&mut stream::Config, &stream::Config) + Send, + { + let desired = desired.into(); + // A lookup that fails for any reason falls through to create, which + // errors on a name already in use. Failing provisioning is the right + // outcome for a stream we could not read: the alternative is updating + // it from a config we never reconciled against. + let Ok(stream) = self.context.get_stream(&desired.name).await else { + self.context.create_stream(desired).await?; + return Ok(()); + }; + let mut current = stream.cached_info().config.clone(); + merge(&mut current, &desired); + self.context.update_stream(¤t).await?; Ok(()) } } diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs index 86242184b..a409df166 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs @@ -262,18 +262,22 @@ impl JetStreamContext for MockJetStreamContext { Ok(()) } - /// Upserts by stream name, mirroring the server-side reconcile the real - /// context performs, so a test can tell it apart from - /// [`Self::get_or_create_stream`] leaving an existing stream alone. - async fn create_or_update_stream + Send>(&self, config: S) -> Result<(), MockError> { - let config = config.into(); + /// Merges into the stored config for a matching name rather than replacing + /// it, so a test can observe that a field the caller does not own survives + /// provisioning. + async fn create_or_reconcile_stream(&self, desired: S, merge: F) -> Result<(), MockError> + where + S: Into + Send, + F: FnOnce(&mut stream::Config, &stream::Config) + Send, + { + let desired = desired.into(); if self.take_failure() { return Err(MockError("simulated stream creation failure".to_string())); } let mut streams = self.created_streams.lock().unwrap(); - match streams.iter_mut().find(|existing| existing.name == config.name) { - Some(existing) => *existing = config, - None => streams.push(config), + match streams.iter_mut().find(|existing| existing.name == desired.name) { + Some(existing) => merge(existing, &desired), + None => streams.push(desired), } Ok(()) } diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs index 700433fa0..4e229f176 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs @@ -21,18 +21,31 @@ pub trait JetStreamContext: Send + Sync + Clone + 'static { config: S, ) -> impl Future> + Send; - /// Reconcile a stream to `config`, creating it when it does not exist. + /// Create `desired` when the stream is absent; otherwise read the live + /// config, let `merge` copy over the fields this service is authoritative + /// for, and write the result back. /// - /// [`Self::get_or_create_stream`] returns an already-existing stream + /// Two behaviours are wrong here and this method is the narrow path + /// between them. [`Self::get_or_create_stream`] returns an existing stream /// untouched, so a setting that carries a security property -- a /// `duplicate_window` sized to a signature timestamp tolerance, say -- - /// would keep whatever value the stream was first created with and the - /// declared config would never take effect. Use this where the config has - /// to hold on an existing deployment rather than only on a fresh one. - fn create_or_update_stream + Send>( + /// keeps whatever value it was first created with and the declared config + /// never takes effect. Sending `desired` wholesale to `STREAM.UPDATE` + /// instead reconciles the *entire* config, so every field the caller left + /// at `Default::default()` overwrites what the operator set: replica + /// count, storage tier, and retention limits all roll back on the next + /// boot. + /// + /// `merge` is what separates the two. It names the fields the service + /// owns, and everything it does not touch stays as the server reports it. + fn create_or_reconcile_stream( &self, - config: S, - ) -> impl Future> + Send; + desired: S, + merge: F, + ) -> impl Future> + Send + where + S: Into + Send, + F: FnOnce(&mut stream::Config, &stream::Config) + Send; } pub trait JetStreamKeyValueStatus: Send + Sync + Clone + 'static { From 373a9dfeabc5b06721eda7625be9d5b3f39e03f8 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 19:36:19 -0400 Subject: [PATCH 17/32] fix(a2a-auth-callout): let the two key-policy refusals be counted apart Signed-off-by: Yordis Prieto --- .../a2a-auth-callout/src/credentials/oidc.rs | 14 +++--- .../src/credentials/oidc/tests.rs | 45 ++++++++++--------- .../a2a-auth-callout/src/denial_category.rs | 8 +++- .../crates/a2a/a2a-auth-callout/src/error.rs | 17 +++++++ 4 files changed, 53 insertions(+), 31 deletions(-) diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs index a63106d69..e5d95ca50 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc.rs @@ -288,17 +288,13 @@ impl JwksOidcVerifier { .find(kid) .ok_or_else(|| CredentialError::InvalidCredentials(format!("no JWK for kid {kid}")))?; if !OIDC_ALLOWED_ALGORITHMS.contains(&header.alg) { - return Err(CredentialError::InvalidCredentials(format!( - "unsupported OIDC token algorithm {:?}", - header.alg - )) - .into()); + return Err(CredentialError::UnsupportedTokenAlgorithm { algorithm: header.alg }.into()); } if !jwk_permits_verification_with(jwk, header.alg) { - return Err(CredentialError::InvalidCredentials(format!( - "JWK for kid {kid} is not published for verifying {:?} signatures", - header.alg - )) + return Err(CredentialError::JwkNotPublishedForVerification { + kid: kid.clone(), + algorithm: header.alg, + } .into()); } let auds: Vec<&str> = self.expected_id_token_audiences.iter().map(String::as_str).collect(); diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs index ce15ccf9b..f74346035 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs @@ -355,10 +355,10 @@ async fn verify_fails_with_non_rsa_jwk() { let jwks = JwkSet { keys: vec![ec_jwk] }; let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["aud".into()], jwks); - // Craft a fake JWT whose kid matches the EC JWK; decode_header will succeed - // but decoding_key_for_jwk must reject the non-RSA key. - // We can't sign with the EC key easily, but we can make a header-only token - // that references the EC kid. decode_header just parses the header. + // The kid resolves to the EC JWK, but ES256 is outside the deployment's + // allow-list, so the token is refused on its algorithm before the key it + // points at is ever considered. A header-only token is enough: nothing + // past decode_header runs. let header_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(br#"{"alg":"ES256","kid":"ec-kid","typ":"JWT"}"#); let payload_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(b"{}"); @@ -368,10 +368,10 @@ async fn verify_fails_with_non_rsa_jwk() { .verify_internal(&BearerToken::new(fake_token), &AudienceAccount::new("acct")) .await .unwrap_err(); - assert!(matches!( - err, - AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(_)) - )); + let AuthCalloutError::CredentialVerification(CredentialError::UnsupportedTokenAlgorithm { algorithm }) = err else { + panic!("expected UnsupportedTokenAlgorithm, got {err:?}"); + }; + assert_eq!(algorithm, jsonwebtoken::Algorithm::ES256); } #[tokio::test] @@ -473,13 +473,10 @@ async fn verify_rejects_algorithm_outside_the_allowlist() { .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) .await .unwrap_err(); - let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(msg)) = err else { - panic!("expected InvalidCredentials, got {err:?}"); + let AuthCalloutError::CredentialVerification(CredentialError::UnsupportedTokenAlgorithm { algorithm }) = err else { + panic!("expected UnsupportedTokenAlgorithm, got {err:?}"); }; - assert!( - msg.contains("unsupported OIDC token algorithm"), - "unexpected message: {msg}" - ); + assert_eq!(algorithm, jsonwebtoken::Algorithm::HS256); } #[tokio::test] @@ -498,10 +495,13 @@ async fn verify_rejects_a_jwk_published_for_encryption() { .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) .await .unwrap_err(); - let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(msg)) = err else { - panic!("expected InvalidCredentials, got {err:?}"); + let AuthCalloutError::CredentialVerification(CredentialError::JwkNotPublishedForVerification { kid, algorithm }) = + err + else { + panic!("expected JwkNotPublishedForVerification, got {err:?}"); }; - assert!(msg.contains("not published for verifying"), "unexpected message: {msg}"); + assert_eq!(kid, "test-kid"); + assert_eq!(algorithm, jsonwebtoken::Algorithm::RS256); } #[tokio::test] @@ -521,7 +521,7 @@ async fn verify_rejects_a_jwk_whose_key_ops_omit_verify() { .unwrap_err(); assert!(matches!( err, - AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(_)) + AuthCalloutError::CredentialVerification(CredentialError::JwkNotPublishedForVerification { .. }) )); } @@ -540,10 +540,13 @@ async fn verify_rejects_a_jwk_pinned_to_a_different_rsa_algorithm() { .verify_internal(&BearerToken::new(token), &AudienceAccount::new("acct")) .await .unwrap_err(); - let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(msg)) = err else { - panic!("expected InvalidCredentials, got {err:?}"); + let AuthCalloutError::CredentialVerification(CredentialError::JwkNotPublishedForVerification { kid, algorithm }) = + err + else { + panic!("expected JwkNotPublishedForVerification, got {err:?}"); }; - assert!(msg.contains("not published for verifying"), "unexpected message: {msg}"); + assert_eq!(kid, "test-kid"); + assert_eq!(algorithm, jsonwebtoken::Algorithm::RS256); } #[tokio::test] diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/denial_category.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/denial_category.rs index 13de8d7d2..bbfd42daf 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/denial_category.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/denial_category.rs @@ -47,7 +47,13 @@ impl DenialCategory { CredentialError::UnknownAccount(_) => Self::UnknownAccount, CredentialError::VerifierUnavailable { .. } => Self::VerifierUnavailable, CredentialError::InvalidRequest(_) => Self::InvalidRequest, - CredentialError::InvalidCredentials(_) => Self::InvalidCredentials, + // The algorithm and JWK-policy rejections stay behind the same + // wire category as any other bad credential. The typed variants + // exist so this side can count them apart, not so a caller can + // learn which check refused it. + CredentialError::InvalidCredentials(_) + | CredentialError::UnsupportedTokenAlgorithm { .. } + | CredentialError::JwkNotPublishedForVerification { .. } => Self::InvalidCredentials, } } } diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/error.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/error.rs index 8d28c3eb1..a81ce25d1 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/error.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/error.rs @@ -22,6 +22,23 @@ pub enum CredentialError { /// The verifier ran and refused the credential material itself. #[error("credential verification failed: {0}")] InvalidCredentials(String), + /// The token nominated an `alg` outside the verifier's allowlist. Kept + /// apart from [`Self::InvalidCredentials`] because an algorithm-confusion + /// attempt is a different signal from a merely bad signature, and a + /// rejection counter should be able to tell them apart without matching + /// on message text. + #[error("credential verification failed: unsupported token algorithm {algorithm:?}")] + UnsupportedTokenAlgorithm { algorithm: jsonwebtoken::Algorithm }, + /// The JWK named by `kid` verified as well-formed but its own `alg`, + /// `use`, or `key_ops` do not permit verifying signatures with the + /// token's algorithm. + #[error( + "credential verification failed: JWK for kid {kid:?} is not published for verifying {algorithm:?} signatures" + )] + JwkNotPublishedForVerification { + kid: String, + algorithm: jsonwebtoken::Algorithm, + }, } impl From for AuthCalloutError { From 9df73332b2eb2fefe157b3ec6aa83086dc7b4678 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 19:37:13 -0400 Subject: [PATCH 18/32] fix(trogon-identity-types): keep an unusable confirmation key out of an issued token Signed-off-by: Yordis Prieto --- .../a2a/a2a-gateway/tests/aauth_roundtrip.rs | 2 +- .../trogon-aauth-as/src/pending/tests.rs | 5 +- .../aauth/trogon-aauth-as/src/policy/tests.rs | 5 +- .../trogon-aauth-as/src/request/tests.rs | 5 +- .../aauth/trogon-aauth-as/src/server.rs | 2 +- .../aauth/trogon-aauth-as/src/test_support.rs | 2 +- .../trogon-aauth-person/src/agent/tests.rs | 4 +- .../trogon-aauth-person/src/http/tests.rs | 8 +- .../trogon-aauth-person/src/pending/tests.rs | 5 +- .../aauth/trogon-aauth-person/src/server.rs | 2 +- .../trogon-aauth-person/src/server/tests.rs | 9 +-- .../trogon-aauth-person/src/store/tests.rs | 5 +- .../tests/person_server_e2e.rs | 2 +- .../aauth/trogon-aauth-sdk/src/tests.rs | 2 +- .../trogon-aauth-sdk/src/verify_response.rs | 2 +- .../src/verify_response/tests.rs | 2 +- .../aauth/trogon-aauth-verify/src/http_pop.rs | 6 +- .../aauth/trogon-aauth-verify/src/nats_pop.rs | 2 +- .../aauth/trogon-aauth-verify/src/token.rs | 6 +- .../tests/nats_pop_roundtrip.rs | 2 +- .../trogon-identity-types/src/aauth/mod.rs | 66 ++++++++++++----- .../trogon-identity-types/src/aauth/tests.rs | 73 +++++++++++++++++-- .../trogon-identity-types/src/constants.rs | 13 ++++ .../src/provider/tests.rs | 2 +- 24 files changed, 161 insertions(+), 71 deletions(-) diff --git a/rsworkspace/crates/a2a/a2a-gateway/tests/aauth_roundtrip.rs b/rsworkspace/crates/a2a/a2a-gateway/tests/aauth_roundtrip.rs index 3e030baaf..b53f8b2d8 100644 --- a/rsworkspace/crates/a2a/a2a-gateway/tests/aauth_roundtrip.rs +++ b/rsworkspace/crates/a2a/a2a-gateway/tests/aauth_roundtrip.rs @@ -80,7 +80,7 @@ fn mint_agent_jwt(ap_signing: &SigningKey, ap_kid: &str, ap_iss: &str, agent: &A "iat": now - 5, "exp": now + 600, "dwk": DWK_AGENT, - "cnf": Cnf { jwk: agent.jwk_val.clone() }, + "cnf": Cnf::public(agent.jwk_val.clone()).expect("test fixture is a public jwk"), }); encode(&header, &claims, &enc).expect("encode agent jwt") } diff --git a/rsworkspace/crates/aauth/trogon-aauth-as/src/pending/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-as/src/pending/tests.rs index c3916ef4e..1798a28ff 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-as/src/pending/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-as/src/pending/tests.rs @@ -13,9 +13,8 @@ fn verified_request() -> VerifiedRequest { iat: 0, exp: 1000, dwk: "aauth-agent.json".into(), - cnf: Cnf { - jwk: serde_json::json!({"kty": "EC"}), - }, + cnf: Cnf::public(serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"})) + .expect("test fixture is a public jwk"), ps: None, }; let resource = ResourceClaims { diff --git a/rsworkspace/crates/aauth/trogon-aauth-as/src/policy/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-as/src/policy/tests.rs index 4ae453941..dab31b45f 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-as/src/policy/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-as/src/policy/tests.rs @@ -13,9 +13,8 @@ fn agent_claims() -> AgentClaims { iat: 0, exp: 1000, dwk: "aauth-agent.json".into(), - cnf: Cnf { - jwk: serde_json::json!({"kty": "EC"}), - }, + cnf: Cnf::public(serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"})) + .expect("test fixture is a public jwk"), ps: None, } } diff --git a/rsworkspace/crates/aauth/trogon-aauth-as/src/request/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-as/src/request/tests.rs index 1255a147e..cac9901db 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-as/src/request/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-as/src/request/tests.rs @@ -11,9 +11,8 @@ fn as_token_context_carries_all_verified_inputs() { iat: 0, exp: 1000, dwk: "aauth-agent.json".into(), - cnf: Cnf { - jwk: serde_json::json!({"kty": "EC"}), - }, + cnf: Cnf::public(serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"})) + .expect("test fixture is a public jwk"), ps: None, }; let resource = ResourceClaims { diff --git a/rsworkspace/crates/aauth/trogon-aauth-as/src/server.rs b/rsworkspace/crates/aauth/trogon-aauth-as/src/server.rs index 0056ab64d..3e8dd4656 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-as/src/server.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-as/src/server.rs @@ -273,7 +273,7 @@ fn cnf_jwk_for(verified: &VerifiedRequest) -> serde_json::Value { .as_ref() .unwrap_or(&verified.agent_claims) .cnf - .jwk + .jwk() .clone() } diff --git a/rsworkspace/crates/aauth/trogon-aauth-as/src/test_support.rs b/rsworkspace/crates/aauth/trogon-aauth-as/src/test_support.rs index b5cd8a0c8..d4ddcf3d6 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-as/src/test_support.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-as/src/test_support.rs @@ -80,7 +80,7 @@ pub fn mint_agent_jwt( "iat": now - 5, "exp": now + 600, "dwk": DWK_AGENT, - "cnf": Cnf { jwk: agent_jwk.clone() }, + "cnf": Cnf::public(agent_jwk.clone()).expect("test fixture is a public jwk"), }); if let Some(parent) = parent_agent { claims["parent_agent"] = serde_json::Value::String(parent.to_string()); diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/agent/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/agent/tests.rs index 5a8f4f0de..3204a6671 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/agent/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/agent/tests.rs @@ -46,9 +46,7 @@ fn mint_agent_jwt(fixture: &KeyFixture, iss: &str, sub: &str, kid: &str) -> Stri iat: now - 5, exp: now + 600, dwk: "aauth-agent.json".to_string(), - cnf: Cnf { - jwk: fixture.jwk.clone(), - }, + cnf: Cnf::public(fixture.jwk.clone()).expect("test fixture is a public jwk"), ps: None, }; let mut header = Header::new(Algorithm::ES256); diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/http/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/http/tests.rs index ff8e18f91..916090df6 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/http/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/http/tests.rs @@ -55,9 +55,7 @@ fn mint_agent_jwt(fixture: &KeyFixture, iss: &str, sub: &str, kid: &str) -> Stri iat: now - 5, exp: now + 600, dwk: "aauth-agent.json".to_string(), - cnf: Cnf { - jwk: fixture.jwk.clone(), - }, + cnf: Cnf::public(fixture.jwk.clone()).expect("test fixture is a public jwk"), ps: None, }; let mut header = Header::new(Algorithm::ES256); @@ -531,9 +529,7 @@ async fn seed_pending(store: &InMemoryStore, phase: PendingPhase) -> String { iat: now_unix() - 5, exp: now_unix() + 600, dwk: "aauth-agent.json".to_string(), - cnf: Cnf { - jwk: agent_fixture.jwk.clone(), - }, + cnf: Cnf::public(agent_fixture.jwk.clone()).expect("test fixture is a public jwk"), ps: None, }; let resource = ResourceClaims { diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/pending/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/pending/tests.rs index f42df4d3b..0f0f331d7 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/pending/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/pending/tests.rs @@ -11,9 +11,8 @@ fn agent_claims() -> AgentClaims { iat: 0, exp: 1000, dwk: "aauth-agent.json".to_string(), - cnf: Cnf { - jwk: serde_json::json!({"kty": "EC"}), - }, + cnf: Cnf::public(serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"})) + .expect("test fixture is a public jwk"), ps: None, } } diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/server.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/server.rs index c3d20fff4..ebc0390f9 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/server.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/server.rs @@ -206,7 +206,7 @@ where cnf_jwk: verified .subagent_claims .as_ref() - .map_or(&verified.agent_claims.cnf.jwk, |s| &s.cnf.jwk) + .map_or(verified.agent_claims.cnf.jwk(), |s| s.cnf.jwk()) .clone(), scope: &scope, act, diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/server/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/server/tests.rs index b77386ca5..e34bd2af3 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/server/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/server/tests.rs @@ -50,9 +50,7 @@ fn mint_agent_jwt(fixture: &KeyFixture, iss: &str, sub: &str, kid: &str) -> Stri iat: now - 5, exp: now + 600, dwk: "aauth-agent.json".to_string(), - cnf: Cnf { - jwk: fixture.jwk.clone(), - }, + cnf: Cnf::public(fixture.jwk.clone()).expect("test fixture is a public jwk"), ps: None, }; let mut header = Header::new(Algorithm::ES256); @@ -891,9 +889,8 @@ async fn respond_to_clarification_on_terminal_pending_returns_gone() { iat: 0, exp: 1000, dwk: "aauth-agent.json".to_string(), - cnf: Cnf { - jwk: serde_json::json!({"kty": "EC"}), - }, + cnf: Cnf::public(serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"})) + .expect("test fixture is a public jwk"), ps: None, }, trogon_identity_types::aauth::ResourceClaims { diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/src/store/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-person/src/store/tests.rs index 54af8ca28..520396618 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/src/store/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/src/store/tests.rs @@ -12,9 +12,8 @@ fn agent_claims() -> AgentClaims { iat: 0, exp: 1000, dwk: "aauth-agent.json".to_string(), - cnf: Cnf { - jwk: serde_json::json!({"kty": "EC"}), - }, + cnf: Cnf::public(serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"})) + .expect("test fixture is a public jwk"), ps: None, } } diff --git a/rsworkspace/crates/aauth/trogon-aauth-person/tests/person_server_e2e.rs b/rsworkspace/crates/aauth/trogon-aauth-person/tests/person_server_e2e.rs index d302fc739..58405e86b 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-person/tests/person_server_e2e.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-person/tests/person_server_e2e.rs @@ -64,7 +64,7 @@ fn mint_agent_jwt(ap_signing: &SigningKey, ap_kid: &str, ap_iss: &str, agent: &A "iat": now - 5, "exp": now + 600, "dwk": DWK_AGENT, - "cnf": Cnf { jwk: agent.jwk_val.clone() }, + "cnf": Cnf::public(agent.jwk_val.clone()).expect("test fixture is a public jwk"), }); encode(&header, &claims, &enc).expect("encode agent jwt") } diff --git a/rsworkspace/crates/aauth/trogon-aauth-sdk/src/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-sdk/src/tests.rs index cd2004ba6..526c1dfc7 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-sdk/src/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-sdk/src/tests.rs @@ -41,7 +41,7 @@ fn mint_agent_jwt(ap_signing: &SigningKey, agent_jwk_val: &serde_json::Value, no "iat": now - 5, "exp": now + 600, "dwk": DWK_AGENT, - "cnf": Cnf { jwk: agent_jwk_val.clone() }, + "cnf": Cnf::public(agent_jwk_val.clone()).expect("test fixture is a public jwk"), }); encode(&header, &claims, &enc).expect("encode agent jwt") } diff --git a/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response.rs b/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response.rs index f625c6982..fcee083a3 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response.rs @@ -111,7 +111,7 @@ pub fn verify_auth_claims( .cnf .as_ref() .ok_or(VerifyResponseError::ConfirmationClaimMissing)?; - let token_jkt = jwk_thumbprint(&cnf.jwk).map_err(VerifyResponseError::ConfirmationKeyThumbprint)?; + let token_jkt = jwk_thumbprint(cnf.jwk()).map_err(VerifyResponseError::ConfirmationKeyThumbprint)?; let own_jkt = jwk_thumbprint(own_agent_jwk).map_err(VerifyResponseError::ConfirmationKeyThumbprint)?; if token_jkt != own_jkt { return Err(VerifyResponseError::ConfirmationKeyMismatch); diff --git a/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response/tests.rs index 921feac56..090c68eb2 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-sdk/src/verify_response/tests.rs @@ -85,7 +85,7 @@ fn valid_claims(agent_jwk: serde_json::Value) -> AuthClaims { consent_id: None, resource: None, act: None, - cnf: Some(Cnf { jwk: agent_jwk }), + cnf: Some(Cnf::public(agent_jwk).expect("test fixture is a public jwk")), } } diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs index 12e9b6df5..c5403ad1e 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs @@ -324,7 +324,7 @@ impl HttpPopVerifier { .verify_agent(&jwt) .await .map_err(HttpPopError::Agent)?; - let cnf_jwk = verified.claims.cnf.jwk.clone(); + let cnf_jwk = verified.claims.cnf.jwk().clone(); let jkt = verified.jkt.clone(); (cnf_jwk, jkt, VerifiedPresenter::Agent(verified)) } @@ -337,14 +337,14 @@ impl HttpPopVerifier { let cnf = verified.claims.cnf.clone().ok_or(HttpPopError::InvalidConfirmationKey( InvalidConfirmationKeyError::MissingConfirmationClaim, ))?; - let jkt = crate::jkt::jwk_thumbprint(&cnf.jwk).map_err(|e| { + let jkt = crate::jkt::jwk_thumbprint(cnf.jwk()).map_err(|e| { HttpPopError::InvalidConfirmationKey(InvalidConfirmationKeyError::StructurallyIncomplete(e)) })?; let presenter = VerifiedPresenter::Auth(VerifiedAuthPresenter { auth: verified, jkt: jkt.clone(), }); - (cnf.jwk, jkt, presenter) + (cnf.jwk().clone(), jkt, presenter) } }; diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/nats_pop.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/nats_pop.rs index 678adad15..d80d85318 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/nats_pop.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/nats_pop.rs @@ -264,7 +264,7 @@ impl NatsPopVerifier { // and a later valid retry from the same agent is wrongly rejected as // a replay. let canonical = envelope.canonical_base(req.subject, req.reply, &verified_agent.jkt); - verify_signature_with_jwk(&verified_agent.claims.cnf.jwk, canonical.as_bytes(), sig)?; + verify_signature_with_jwk(verified_agent.claims.cnf.jwk(), canonical.as_bytes(), sig)?; // Replay protection only fires once the signature has authenticated // the request. Derive the TTL using saturating arithmetic and floor diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs index 2a7b742a6..98bb9aeb7 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/token.rs @@ -175,7 +175,7 @@ impl TokenVerifier { .await?; let claims: AgentClaims = serde_json::from_value(claims_raw.clone()).map_err(|_| TokenError::MissingClaim("agent claims"))?; - let jkt = crate::jkt::jwk_thumbprint(&claims.cnf.jwk).map_err(|e| { + let jkt = crate::jkt::jwk_thumbprint(claims.cnf.jwk()).map_err(|e| { TokenError::InvalidClaim(match e { crate::jkt::JktError::MissingKty => "cnf.jwk.kty", crate::jkt::JktError::MissingField(f) => f, @@ -273,13 +273,13 @@ impl TokenVerifier { // None of its error variants indicate key material that parses // structurally but is cryptographically invalid -- that case is // caught below by `DecodingKey::from_jwk`. - let jkt = crate::jkt::jwk_thumbprint(&cnf.jwk).map_err(RequestContextError::StructurallyIncompleteKey)?; + let jkt = crate::jkt::jwk_thumbprint(cnf.jwk()).map_err(RequestContextError::StructurallyIncompleteKey)?; // jwk_thumbprint already validates presence of the type-specific // required members (crv/x/y for EC, crv/x for OKP, n/e for RSA); a // JWK that reaches this point but still cannot be parsed into a // `jsonwebtoken` decoding key is invalid key material, not merely // structurally incomplete. - let parsed_jwk: Jwk = serde_json::from_value(cnf.jwk.clone()) + let parsed_jwk: Jwk = serde_json::from_value(cnf.jwk().clone()) .map_err(|e| RequestContextError::InvalidKeyMaterial(InvalidKeyMaterialSourceError::Deserialize(e)))?; DecodingKey::from_jwk(&parsed_jwk) .map_err(|e| RequestContextError::InvalidKeyMaterial(InvalidKeyMaterialSourceError::DecodingKey(e)))?; diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/tests/nats_pop_roundtrip.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/tests/nats_pop_roundtrip.rs index 88c4ac93a..191f615f8 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/tests/nats_pop_roundtrip.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/tests/nats_pop_roundtrip.rs @@ -87,7 +87,7 @@ async fn end_to_end_nats_pop_verifies() { "iat": now - 5, "exp": now + 600, "dwk": DWK_AGENT, - "cnf": Cnf { jwk: agent_jwk_val.clone() }, + "cnf": Cnf::public(agent_jwk_val.clone()).expect("test fixture is a public jwk"), }); let agent_jwt = encode(&header, &claims, &enc_key).expect("encode agent jwt"); diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs index 87e5d9daa..93a3d15b0 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs @@ -28,38 +28,41 @@ pub mod person_server; pub use delegation::Act; /// Public-key confirmation claim (`cnf`) as carried in `aa-agent+jwt`. +/// +/// Issuer-side construction goes through [`Cnf::public`], which is the only +/// constructor; the field is private so no caller can assemble one around it. +/// Deserialization is deliberately exempt: a peer's inbound `cnf` is parsed as +/// sent, because what a peer put in its own confirmation claim is not this +/// type's call to reject, and verification reads only the public parameters. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Cnf { /// Embedded JWK. Stored as serde_json::Value so this crate avoids depending on /// `jsonwebtoken`. Verifier-side parses into `jsonwebtoken::jwk::Jwk`. - pub jwk: Value, + jwk: Value, } impl Cnf { /// Build a confirmation claim, refusing any JWK that carries private or - /// symmetric key material. + /// symmetric key material, or that is missing the public members its key + /// type needs to be usable. /// - /// Issuers must go through this rather than constructing [`Cnf`] - /// literally. A `cnf` claim is embedded in a signed token that is handed - /// to resource servers by design, so a caller that passes a full keypair - /// instead of its public half publishes the private key to every party - /// that sees the token, with a valid signature over it. That mistake is - /// easy to make (JWK serializers include `d` unless asked not to) and - /// impossible to walk back once a token is issued, which is why it is - /// checked at the one place every issuer passes through. - /// - /// The field stays public so the verifier side can still deserialize an - /// inbound token: what a peer chose to put in its own `cnf` is not ours - /// to reject here, and verification reads only the public parameters. + /// Two failures are guarded here and they fail in opposite directions. A + /// caller that passes a full keypair instead of its public half publishes + /// the private key to every party that sees the token, with a valid + /// signature over it; that mistake is easy to make (JWK serializers + /// include `d` unless asked not to) and impossible to walk back once a + /// token is issued. A caller that passes an incomplete JWK instead mints a + /// token whose confirmation key can never satisfy a proof of possession, + /// so every request bound to it fails at the resource with no indication + /// that the fault is in the token rather than the request. pub fn public(jwk: Value) -> Result { let Some(members) = jwk.as_object() else { return Err(CnfError::NotAnObject); }; - if members - .get("kty") - .and_then(Value::as_str) - .is_some_and(|kty| kty.eq_ignore_ascii_case(crate::constants::KTY_OCT)) - { + let Some(kty) = members.get("kty").and_then(Value::as_str) else { + return Err(CnfError::MissingKeyType); + }; + if kty.eq_ignore_ascii_case(crate::constants::KTY_OCT) { return Err(CnfError::SymmetricKey); } for member in crate::constants::JWK_PRIVATE_MEMBERS { @@ -67,8 +70,27 @@ impl Cnf { return Err(CnfError::PrivateKeyMaterial { member }); } } + let (kty, required): (&'static str, &[&'static str]) = match kty { + crate::constants::KTY_EC => (crate::constants::KTY_EC, &crate::constants::JWK_REQUIRED_EC_MEMBERS), + crate::constants::KTY_RSA => (crate::constants::KTY_RSA, &crate::constants::JWK_REQUIRED_RSA_MEMBERS), + crate::constants::KTY_OKP => (crate::constants::KTY_OKP, &crate::constants::JWK_REQUIRED_OKP_MEMBERS), + other => { + return Err(CnfError::UnsupportedKeyType { kty: other.to_owned() }); + } + }; + for member in required { + if members.get(*member).and_then(Value::as_str).is_none_or(str::is_empty) { + return Err(CnfError::UnusablePublicMember { kty, member }); + } + } Ok(Self { jwk }) } + + /// The embedded JWK, as it will appear in the issued token. + #[must_use] + pub fn jwk(&self) -> &Value { + &self.jwk + } } /// Rejections from [`Cnf::public`]. @@ -76,10 +98,16 @@ impl Cnf { pub enum CnfError { #[error("cnf.jwk must be a JSON object")] NotAnObject, + #[error("cnf.jwk must name a kty")] + MissingKeyType, #[error("cnf.jwk must not be a symmetric key")] SymmetricKey, #[error("cnf.jwk carries private key material in member {member:?}")] PrivateKeyMaterial { member: &'static str }, + #[error("cnf.jwk names key type {kty:?}, which cannot carry a confirmation key")] + UnsupportedKeyType { kty: String }, + #[error("cnf.jwk of type {kty} needs a non-empty string member {member:?}")] + UnusablePublicMember { kty: &'static str, member: &'static str }, } /// Claims for an `aa-agent+jwt`. Issued by an Agent Provider at bootstrap. diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs index 3aab5c994..a251d239c 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs @@ -44,9 +44,8 @@ fn agent_claims_serde() { iat: 100, exp: 200, dwk: DWK_AGENT.into(), - cnf: Cnf { - jwk: serde_json::json!({"kty": "EC", "crv": "P-256", "x": "X", "y": "Y"}), - }, + cnf: Cnf::public(serde_json::json!({"kty": "EC", "crv": "P-256", "x": "X", "y": "Y"})) + .expect("test fixture is a public jwk"), ps: Some("https://ps.example".into()), }; let j = serde_json::to_value(&c).unwrap(); @@ -243,7 +242,7 @@ fn split_header_skips_empty_segments_from_stray_semicolons() { fn cnf_public_accepts_an_ec_public_jwk() { let jwk = serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA", "y": "BBB"}); let cnf = Cnf::public(jwk.clone()).expect("public jwk accepted"); - assert_eq!(cnf.jwk, jwk); + assert_eq!(cnf.jwk(), &jwk); } #[test] @@ -300,11 +299,75 @@ fn cnf_public_rejects_non_object_jwk() { } } +#[test] +fn cnf_public_accepts_the_other_supported_key_types() { + for jwk in [ + serde_json::json!({"kty": "RSA", "n": "AAA", "e": "AQAB"}), + serde_json::json!({"kty": "OKP", "crv": "Ed25519", "x": "AAA"}), + ] { + Cnf::public(jwk.clone()).unwrap_or_else(|e| panic!("{jwk} must be accepted, got {e}")); + } +} + +#[test] +fn cnf_public_rejects_a_jwk_with_no_kty() { + let jwk = serde_json::json!({"crv": "P-256", "x": "AAA", "y": "BBB"}); + assert_eq!(Cnf::public(jwk).unwrap_err(), CnfError::MissingKeyType); + let non_string = serde_json::json!({"kty": 256, "x": "AAA"}); + assert_eq!(Cnf::public(non_string).unwrap_err(), CnfError::MissingKeyType); +} + +#[test] +fn cnf_public_rejects_a_key_type_it_cannot_check() { + // Anything outside the three known types would sail past the member scan + // with nothing verified, so it is refused rather than waved through. + let jwk = serde_json::json!({"kty": "ec", "crv": "P-256", "x": "AAA", "y": "BBB"}); + assert_eq!( + Cnf::public(jwk).unwrap_err(), + CnfError::UnsupportedKeyType { kty: "ec".to_owned() } + ); +} + +#[test] +fn cnf_public_rejects_a_jwk_missing_a_member_its_key_type_needs() { + // The failure this guards is silent at issuance: the token verifies, and + // every proof of possession against it fails at the resource instead. + let cases = [ + (serde_json::json!({"kty": "EC", "x": "AAA", "y": "BBB"}), "EC", "crv"), + (serde_json::json!({"kty": "EC", "crv": "P-256", "y": "BBB"}), "EC", "x"), + (serde_json::json!({"kty": "EC", "crv": "P-256", "x": "AAA"}), "EC", "y"), + (serde_json::json!({"kty": "RSA", "e": "AQAB"}), "RSA", "n"), + (serde_json::json!({"kty": "RSA", "n": "AAA"}), "RSA", "e"), + (serde_json::json!({"kty": "OKP", "x": "AAA"}), "OKP", "crv"), + (serde_json::json!({"kty": "OKP", "crv": "Ed25519"}), "OKP", "x"), + ]; + for (jwk, kty, member) in cases { + assert_eq!( + Cnf::public(jwk).unwrap_err(), + CnfError::UnusablePublicMember { kty, member }, + "{kty} without {member} must be refused" + ); + } +} + +#[test] +fn cnf_public_rejects_a_required_member_that_is_present_but_unusable() { + // Present-but-empty and present-but-not-a-string are the same defect as + // absent: nothing a verifier can build a key from. + for x in [serde_json::json!(""), serde_json::json!(0), serde_json::json!(null)] { + let jwk = serde_json::json!({"kty": "EC", "crv": "P-256", "x": x, "y": "BBB"}); + assert_eq!( + Cnf::public(jwk).unwrap_err(), + CnfError::UnusablePublicMember { kty: "EC", member: "x" } + ); + } +} + #[test] fn cnf_still_deserializes_a_peer_supplied_confirmation_claim() { // The verifier read path must stay lenient: rejecting a peer's own `cnf` // at parse time is not this type's call, and the guard is issuer-side. let raw = r#"{"jwk":{"kty":"EC","crv":"P-256","x":"AAA","y":"BBB","d":"THEIRS"}}"#; let cnf: Cnf = serde_json::from_str(raw).expect("inbound cnf parses"); - assert!(cnf.jwk.get("d").is_some()); + assert!(cnf.jwk().get("d").is_some()); } diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs b/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs index c0b0c4c7a..5cbf774d6 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs @@ -15,6 +15,19 @@ pub const JWK_PRIVATE_MEMBERS: [&str; 8] = ["d", "p", "q", "dp", "dq", "qi", "ot /// possession, and publishing it in a token discloses it. pub const KTY_OCT: &str = "oct"; +/// `kty` values an issuer may put in a confirmation claim. +pub const KTY_EC: &str = "EC"; +pub const KTY_RSA: &str = "RSA"; +pub const KTY_OKP: &str = "OKP"; + +/// Public members each key type requires before the JWK can verify anything. +/// EC per RFC 7518 Section 6.2.1, RSA per Section 6.3.1, OKP per RFC 8037 +/// Section 2. A confirmation key missing any of these is syntactically a JWK +/// but cannot be used to check a proof of possession. +pub const JWK_REQUIRED_EC_MEMBERS: [&str; 3] = ["crv", "x", "y"]; +pub const JWK_REQUIRED_RSA_MEMBERS: [&str; 2] = ["n", "e"]; +pub const JWK_REQUIRED_OKP_MEMBERS: [&str; 2] = ["crv", "x"]; + /// `typ` header value identifying an agent identity token. pub const TYP_AGENT: &str = "aa-agent+jwt"; /// `typ` header value identifying a resource challenge token. diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs index 07fc0f7a6..f53a8c5f0 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/provider/tests.rs @@ -182,7 +182,7 @@ fn mint_produces_header_and_claims_per_spec() { assert_eq!(claims.iss, "https://ap.example"); assert_eq!(claims.sub, sub.as_str()); assert_eq!(claims.dwk, DWK_AGENT); - assert_eq!(claims.cnf.jwk, test_agent_jwk()); + assert_eq!(claims.cnf.jwk(), &test_agent_jwk()); assert_eq!(claims.ps.as_deref(), Some("https://ps.example")); assert!(claims.exp > claims.iat); assert_eq!(claims.exp - claims.iat, 3600); From 98f63a35a5fd3104396aaabc539d8a98ddf3dd11 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 19:37:13 -0400 Subject: [PATCH 19/32] fix(trogon-aauth-verify): accept the Content-Digest shapes RFC 9530 allows Signed-off-by: Yordis Prieto --- .../aauth/trogon-aauth-verify/src/http_pop.rs | 24 ++++++-- .../trogon-aauth-verify/src/http_pop/tests.rs | 55 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs index c5403ad1e..d9c6849b3 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs @@ -552,18 +552,34 @@ fn verify_content_digest(req: &HttpRequest) -> Result<(), HttpPopError> { /// value, a Structured Fields Dictionary of algorithm keys to Byte Sequences /// (`sha-256=::`). Other algorithm entries are skipped rather than /// rejected, since a peer is free to send additional ones. +/// +/// Two RFC 8941 Dictionary rules matter for interop and are honoured here. +/// A member's Item may carry parameters (`sha-256=:...:;q=1`), which are not +/// part of the Byte Sequence and are ignored. A repeated key resolves to its +/// *last* occurrence, so the scan cannot return early: taking the first would +/// make this verifier disagree with any RFC-compliant peer about which digest +/// a duplicated `sha-256` member names. +/// +/// This is a targeted reader for one Byte Sequence member, not a general +/// Structured Fields parser. It does not model Inner Lists, and a parameter +/// whose value is a String containing `,` or `;` would split wrongly; no +/// parameter defined for `Content-Digest` takes such a value. fn parse_sha256_content_digest(raw: &str) -> Option> { + let mut last = None; for member in raw.split(',') { - let Some((algorithm, encoded)) = member.split_once('=') else { + let Some((algorithm, value)) = member.split_once('=') else { continue; }; if !algorithm.trim().eq_ignore_ascii_case("sha-256") { continue; } - let inner = encoded.trim().strip_prefix(':')?.strip_suffix(':')?; - return decode_base64_any_alphabet(inner); + // The Byte Sequence ends at its closing colon; anything after that is + // the parameter list. + let value = value.trim_start(); + let inner = value.strip_prefix(':')?.split_once(':')?.0; + last = Some(decode_base64_any_alphabet(inner)?); } - None + last } /// Decodes a digest that may arrive in any of the base64 alphabets seen in diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs index 73f3db31a..958f8cbf6 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs @@ -257,6 +257,61 @@ async fn verify_rejects_stripped_body_against_covered_content_digest() { assert!(matches!(err, HttpPopError::ContentDigestMismatch)); } +#[tokio::test(flavor = "current_thread")] +async fn verify_accepts_a_parameterized_sha256_content_digest_item() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // RFC 8941 lets any Dictionary member's Item carry parameters. They are + // not part of the Byte Sequence, so a peer that sends them must still + // interoperate rather than be read as an unsupported digest. + let body = br#"{"scope":"data.read"}"#; + let parameterized = format!("sha-256=:{}:;q=1", STANDARD.encode(Sha256::digest(body))); + let req = signed_body_request(&fixture, &jwt, body, parameterized); + + verifier.verify(&req).await.expect("parameterized item verifies"); +} + +#[tokio::test(flavor = "current_thread")] +async fn verify_resolves_a_duplicated_sha256_member_to_the_last_one() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // RFC 8941 Dictionary parsing keeps the last member for a repeated key. + // Reading the first instead would let a peer show one digest to this + // verifier and a different one to every other RFC-compliant component. + let body = br#"{"scope":"data.read"}"#; + let stale = STANDARD.encode(Sha256::digest(br#"{"scope":"data.write"}"#)); + let live = STANDARD.encode(Sha256::digest(body)); + let duplicated = format!("sha-256=:{stale}:, sha-256=:{live}:"); + let req = signed_body_request(&fixture, &jwt, body, duplicated); + + verifier.verify(&req).await.expect("last duplicate member wins"); +} + +#[tokio::test(flavor = "current_thread")] +async fn verify_rejects_when_the_last_duplicated_sha256_member_mismatches() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // The mirror of the case above: a correct leading member must not rescue + // a trailing one that does not match the body. + let body = br#"{"scope":"data.read"}"#; + let live = STANDARD.encode(Sha256::digest(body)); + let stale = STANDARD.encode(Sha256::digest(br#"{"scope":"data.write"}"#)); + let duplicated = format!("sha-256=:{live}:, sha-256=:{stale}:"); + let req = signed_body_request(&fixture, &jwt, body, duplicated); + + let err = verifier.verify(&req).await.unwrap_err(); + assert!(matches!(err, HttpPopError::ContentDigestMismatch)); +} + #[tokio::test(flavor = "current_thread")] async fn verify_rejects_content_digest_without_sha256_entry() { let fixture = p256_fixture("k1"); From 1a275c48152da11f0455386ee87753070d5595e9 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 21:28:49 -0400 Subject: [PATCH 20/32] chore(a2a-auth-callout): check every segment of the token the redaction hides Signed-off-by: Yordis Prieto --- .../a2a/a2a-auth-callout/src/credentials/oidc/tests.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs index f74346035..06fcfd1fd 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs @@ -591,7 +591,12 @@ async fn verify_accepts_a_jwk_that_declares_no_purpose_at_all() { fn bearer_token_debug_does_not_leak_the_assertion() { let token = BearerToken::new("hhh.ppp.sss"); let dbg = format!("{token:?}"); - assert!(!dbg.contains("ppp"), "{dbg}"); + assert!(!dbg.contains(token.as_str()), "{dbg}"); + // Each segment separately: a Debug that printed only the header, or only + // the signature, would still disclose the assertion piecewise. + for segment in ["hhh", "ppp", "sss"] { + assert!(!dbg.contains(segment), "{dbg}"); + } assert!(dbg.contains(""), "{dbg}"); assert_eq!(token.as_str(), "hhh.ppp.sss"); } From 69189383bcc6683d67ed5b8df56b632aa2875b85 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 21:28:49 -0400 Subject: [PATCH 21/32] fix(trogon-identity-types): stop a peer's private key from reaching the logs Signed-off-by: Yordis Prieto --- .../trogon-identity-types/src/aauth/mod.rs | 22 +++++++++++++- .../trogon-identity-types/src/aauth/tests.rs | 30 +++++++++++++++++++ .../trogon-identity-types/src/constants.rs | 7 +++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs index 93a3d15b0..2facd4d57 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/mod.rs @@ -34,13 +34,33 @@ pub use delegation::Act; /// Deserialization is deliberately exempt: a peer's inbound `cnf` is parsed as /// sent, because what a peer put in its own confirmation claim is not this /// type's call to reject, and verification reads only the public parameters. -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Cnf { /// Embedded JWK. Stored as serde_json::Value so this crate avoids depending on /// `jsonwebtoken`. Verifier-side parses into `jsonwebtoken::jwk::Jwk`. jwk: Value, } +/// Prints only the members that say *which* key this is, never the key. +/// +/// [`Cnf::public`] refuses private key material, but the deserialization path +/// above accepts whatever a peer sent, so a `Cnf` reached by that path may hold +/// a private scalar. Anything that logs a claim set at debug level would then +/// write it out, and a derived `Debug` gives no warning that this is what it +/// does. The peer's own key is theirs to mishandle; writing it into this +/// platform's logs is not. +impl std::fmt::Debug for Cnf { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut out = f.debug_struct("Cnf"); + for member in crate::constants::JWK_DESCRIPTIVE_MEMBERS { + if let Some(value) = self.jwk.get(member) { + out.field(member, value); + } + } + out.finish_non_exhaustive() + } +} + impl Cnf { /// Build a confirmation claim, refusing any JWK that carries private or /// symmetric key material, or that is missing the public members its key diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs index a251d239c..a5f3a26bf 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/aauth/tests.rs @@ -371,3 +371,33 @@ fn cnf_still_deserializes_a_peer_supplied_confirmation_claim() { let cnf: Cnf = serde_json::from_str(raw).expect("inbound cnf parses"); assert!(cnf.jwk().get("d").is_some()); } + +#[test] +fn cnf_debug_does_not_print_the_key_it_holds() { + // Reached through the lenient inbound path, so `d` is present: this is + // exactly the shape whose Debug output must stay clean. + let raw = r#"{"jwk":{"kty":"EC","crv":"P-256","kid":"peer-1","x":"PUBX","y":"PUBY","d":"THEIRS"}}"#; + let cnf: Cnf = serde_json::from_str(raw).expect("inbound cnf parses"); + + let printed = format!("{cnf:?}"); + for secret in ["THEIRS", "PUBX", "PUBY"] { + assert!(!printed.contains(secret), "{printed}"); + } + // Still says which key it is, or the redaction costs every log line its + // diagnostic value. + assert!(printed.contains("peer-1"), "{printed}"); + assert!(printed.contains("P-256"), "{printed}"); +} + +#[test] +fn cnf_debug_omits_members_it_does_not_recognise() { + // The allow-list is the point: a member added to a future key type must be + // withheld until someone decides it is safe to print. + let raw = r#"{"jwk":{"kty":"OKP","crv":"Ed25519","x":"AAA","some_future_member":"UNVETTED"}}"#; + let cnf: Cnf = serde_json::from_str(raw).expect("inbound cnf parses"); + + let printed = format!("{cnf:?}"); + assert!(!printed.contains("UNVETTED"), "{printed}"); + assert!(!printed.contains("some_future_member"), "{printed}"); + assert!(printed.contains(".."), "{printed}"); +} diff --git a/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs b/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs index 5cbf774d6..e9fe4ce8d 100644 --- a/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs +++ b/rsworkspace/crates/platform/trogon-identity-types/src/constants.rs @@ -10,6 +10,13 @@ pub const MAX_ACT_CHAIN_DEPTH: usize = 8; /// which *is* the secret for a symmetric `oct` key (Section 6.4.1). pub const JWK_PRIVATE_MEMBERS: [&str; 8] = ["d", "p", "q", "dp", "dq", "qi", "oth", "k"]; +/// JWK members that describe a key without being any part of one: they name +/// which key is meant and what it is for, and none of them is key material of +/// either half. This is an allow-list rather than the inverse of +/// [`JWK_PRIVATE_MEMBERS`] because a member this crate has never heard of must +/// stay unprinted by default. +pub const JWK_DESCRIPTIVE_MEMBERS: [&str; 5] = ["kty", "crv", "kid", "alg", "use"]; + /// `kty` value for a symmetric key. Never valid in a confirmation claim: a /// proof-of-possession key that both parties must hold is not a proof of /// possession, and publishing it in a token discloses it. From 21eda5a52310592515bbc6a3a17aecd14efab412 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 21:29:24 -0400 Subject: [PATCH 22/32] chore(trogon-jwks-publisher): make an unselectable published key set unrepresentable Signed-off-by: Yordis Prieto --- .../trogon-jwks-publisher/src/publisher.rs | 103 ++++++++++++------ .../src/publisher/tests.rs | 41 ++++++- 2 files changed, 104 insertions(+), 40 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs index faf5a188d..22b0f38bd 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs @@ -54,10 +54,12 @@ pub enum PublisherError { UnknownDwk(String), #[error("dwk filename {0:?} was registered more than once")] DuplicateDwk(String), - #[error("dwk {dwk:?} publishes key id {kid:?} more than once")] - DuplicateKeyId { dwk: String, kid: String }, - #[error("dwk {dwk:?} publishes {keys} keys and at least one omits `kid`; only a single-key set may omit it")] - MissingKeyId { dwk: String, keys: usize }, + #[error("dwk {dwk:?} cannot be published: {source}")] + Unpublishable { + dwk: String, + #[source] + source: UnpublishableJwkSet, + }, #[error("invalid EC PKCS8 PEM for kid {kid:?}: {source}")] InvalidPem { kid: String, @@ -75,6 +77,19 @@ fn is_known_dwk(dwk: &str) -> bool { known_dwk_filenames().contains(&dwk) } +/// Why a [`JwkSet`] cannot be published as it stands. Names no dwk: which +/// document a set was registered under is the registrar's context, not the +/// set's own. +#[derive(Debug, thiserror::Error)] +pub enum UnpublishableJwkSet { + #[error("key id {kid:?} is published more than once")] + DuplicateKeyId { kid: String }, + #[error("{keys} keys are published and at least one omits `kid`; only a single-key set may omit it")] + MissingKeyId { keys: usize }, +} + +/// A [`JwkSet`] every consumer can actually select a key from. +/// /// A published set is selected from by `kid`: [`JwkSet::find`] matches the JWT /// header's `kid` against `common.key_id`, and `trogon-aauth-verify`'s /// `pick_jwk` does the same, falling back to the sole compatible key only when @@ -86,31 +101,48 @@ fn is_known_dwk(dwk: &str) -> bool { /// cannot be selected at all once the set holds more than one, which is exactly /// the state a rotation overlap creates. Both fail at the consumer, remotely, /// with nothing to see on this side, so they are refused at startup instead -- -/// the same reason the dwk filename is validated here rather than left to -/// surface as a 404. -fn validate_selectable_by_kid(dwk: &str, set: &JwkSet) -> Result<(), PublisherError> { - let multi_key = set.keys.len() > 1; - let mut seen: HashSet<&str> = HashSet::new(); - for jwk in &set.keys { - match jwk.common.key_id.as_deref() { - Some(kid) => { - if !seen.insert(kid) { - return Err(PublisherError::DuplicateKeyId { - dwk: dwk.to_owned(), - kid: kid.to_owned(), - }); +/// the same reason the dwk filename is checked at registration rather than left +/// to surface as a 404. +/// +/// Holding that as a type rather than a check run on the way past means a set +/// reaching [`JwksPublisherConfig`] has already been through it, and an +/// unselectable one has no way to be represented there at all. An empty set +/// (`{"keys":[]}`) is publishable: it is a legitimate discovery state, not a +/// misconfiguration. +#[derive(Debug, Clone)] +pub struct PublishableJwkSet(JwkSet); + +impl TryFrom for PublishableJwkSet { + type Error = UnpublishableJwkSet; + + fn try_from(set: JwkSet) -> Result { + let multi_key = set.keys.len() > 1; + let mut seen: HashSet<&str> = HashSet::new(); + for jwk in &set.keys { + match jwk.common.key_id.as_deref() { + Some(kid) => { + if !seen.insert(kid) { + return Err(UnpublishableJwkSet::DuplicateKeyId { kid: kid.to_owned() }); + } } + None if multi_key => { + return Err(UnpublishableJwkSet::MissingKeyId { keys: set.keys.len() }); + } + None => {} } - None if multi_key => { - return Err(PublisherError::MissingKeyId { - dwk: dwk.to_owned(), - keys: set.keys.len(), - }); - } - None => {} } + Ok(Self(set)) + } +} + +impl PublishableJwkSet { + /// The underlying set, for serialization. Borrowed rather than owned so + /// the validated value cannot be taken apart and put back together with + /// the invariant broken. + #[must_use] + pub fn as_jwk_set(&self) -> &JwkSet { + &self.0 } - Ok(()) } /// Build a public EC P-256 JWK from a PKCS8 PEM private key, mirroring @@ -155,7 +187,7 @@ fn base64_url(bytes: impl AsRef<[u8]>) -> String { #[derive(Debug, Default)] pub struct JwksPublisherConfigBuilder { max_age: CacheMaxAge, - entries: HashMap, + entries: HashMap, } impl JwksPublisherConfigBuilder { @@ -167,9 +199,9 @@ impl JwksPublisherConfigBuilder { } } - /// Register a pre-built `JwkSet` under a dwk filename. An empty `JwkSet` - /// (`{"keys":[]}`) is accepted -- it is a legitimate discovery state, not - /// an error. + /// Register a pre-built `JwkSet` under a dwk filename. This is the one + /// boundary where a raw set is converted into a [`PublishableJwkSet`]; + /// past here the configuration holds nothing else. pub fn with_jwk_set(mut self, dwk: impl Into, set: JwkSet) -> Result { let dwk = dwk.into(); if !is_known_dwk(&dwk) { @@ -178,7 +210,10 @@ impl JwksPublisherConfigBuilder { if self.entries.contains_key(&dwk) { return Err(PublisherError::DuplicateDwk(dwk)); } - validate_selectable_by_kid(&dwk, &set)?; + let set = PublishableJwkSet::try_from(set).map_err(|source| PublisherError::Unpublishable { + dwk: dwk.clone(), + source, + })?; self.entries.insert(dwk, set); Ok(self) } @@ -199,12 +234,12 @@ impl JwksPublisherConfigBuilder { } } -/// Validated publisher configuration: dwk filename -> `JwkSet`, plus the -/// `Cache-Control` max-age applied to every discovery response. +/// Validated publisher configuration: dwk filename -> [`PublishableJwkSet`], +/// plus the `Cache-Control` max-age applied to every discovery response. #[derive(Clone)] pub struct JwksPublisherConfig { max_age: CacheMaxAge, - entries: HashMap, + entries: HashMap, } /// Build the `GET /.well-known/{dwk}` discovery router. Mount this into a @@ -241,7 +276,7 @@ async fn serve_dwk(State(config): State, Path(dwk): Path Date: Fri, 7 Aug 2026 21:47:29 -0400 Subject: [PATCH 23/32] chore(trogon-nats): let a test name the provisioning refusal it asked for Signed-off-by: Yordis Prieto --- .../trogon-nats/src/jetstream/mocks.rs | 26 +++++++++++++++---- .../trogon-nats/src/jetstream/mocks/tests.rs | 15 +++++++++-- .../platform/trogon-nats/src/jetstream/mod.rs | 2 +- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs index a409df166..a41a07fb9 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs @@ -210,6 +210,19 @@ impl JsDoubleAckWith for MockJsMessage { } } +/// Why a [`MockJetStreamContext`] refused to provision. +/// +/// Its own type rather than the crate's shared [`MockError`]: a test that +/// armed `fail_next` should be able to name the refusal it asked for instead +/// of matching on the prose of a message. +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +pub enum MockStreamProvisionError { + #[error("simulated stream creation failure")] + Creation, + #[error("simulated stream reconciliation failure")] + Reconciliation, +} + #[derive(Clone, Debug)] pub struct MockJetStreamContext { created_streams: Arc>>, @@ -250,13 +263,16 @@ impl Default for MockJetStreamContext { } impl JetStreamContext for MockJetStreamContext { - type Error = MockError; + type Error = MockStreamProvisionError; type Stream = (); - async fn get_or_create_stream + Send>(&self, config: S) -> Result<(), MockError> { + async fn get_or_create_stream + Send>( + &self, + config: S, + ) -> Result<(), MockStreamProvisionError> { let config = config.into(); if self.take_failure() { - return Err(MockError("simulated stream creation failure".to_string())); + return Err(MockStreamProvisionError::Creation); } self.created_streams.lock().unwrap().push(config); Ok(()) @@ -265,14 +281,14 @@ impl JetStreamContext for MockJetStreamContext { /// Merges into the stored config for a matching name rather than replacing /// it, so a test can observe that a field the caller does not own survives /// provisioning. - async fn create_or_reconcile_stream(&self, desired: S, merge: F) -> Result<(), MockError> + async fn create_or_reconcile_stream(&self, desired: S, merge: F) -> Result<(), MockStreamProvisionError> where S: Into + Send, F: FnOnce(&mut stream::Config, &stream::Config) + Send, { let desired = desired.into(); if self.take_failure() { - return Err(MockError("simulated stream creation failure".to_string())); + return Err(MockStreamProvisionError::Reconciliation); } let mut streams = self.created_streams.lock().unwrap(); match streams.iter_mut().find(|existing| existing.name == desired.name) { diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs index 8b05b0206..6e86794b2 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs @@ -74,8 +74,19 @@ async fn mock_context_records_stream_creation() { async fn mock_context_fails_when_configured() { let ctx = MockJetStreamContext::new(); ctx.fail_next(); - let result = ctx.get_or_create_stream(stream::Config::default()).await; - assert!(result.is_err()); + let err = ctx.get_or_create_stream(stream::Config::default()).await.unwrap_err(); + assert_eq!(err, MockStreamProvisionError::Creation); +} + +#[tokio::test] +async fn mock_context_names_a_reconcile_refusal_apart_from_a_creation_one() { + let ctx = MockJetStreamContext::new(); + ctx.fail_next(); + let err = ctx + .create_or_reconcile_stream(stream::Config::default(), |_, _| {}) + .await + .unwrap_err(); + assert_eq!(err, MockStreamProvisionError::Reconciliation); } #[tokio::test] diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs index 00cfd49d7..a308a324a 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs @@ -46,5 +46,5 @@ pub use mocks::{ AckKindSnapshot, AckKindValue, MockJetStreamConsumer, MockJetStreamConsumerFactory, MockJetStreamContext, MockJetStreamKvClient, MockJetStreamKvStore, MockJetStreamPublishMessage, MockJetStreamPublisher, MockJetStreamPurger, MockJetStreamStream, MockJsMessage, MockKvEntryOutcome, MockKvGetOutcome, MockObjectStore, - MockPublishedOutboundMessage, + MockPublishedOutboundMessage, MockStreamProvisionError, }; From 07239d82ebd91d7fdf119aed882612408311b8dd Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 21:48:59 -0400 Subject: [PATCH 24/32] fix(trogon-nats): stop provisioning from being able to widen what it owns Signed-off-by: Yordis Prieto --- .../src/source/gitlab/server.rs | 17 +++--- .../trogon-nats/src/jetstream/client.rs | 17 ++---- .../trogon-nats/src/jetstream/mocks.rs | 20 +++---- .../trogon-nats/src/jetstream/mocks/tests.rs | 2 +- .../platform/trogon-nats/src/jetstream/mod.rs | 3 +- .../trogon-nats/src/jetstream/traits.rs | 56 +++++++++++++++---- .../trogon-nats/src/jetstream/traits/tests.rs | 47 +++++++++++++++- 7 files changed, 120 insertions(+), 42 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs index 7a911a1f8..856960dfb 100644 --- a/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs +++ b/rsworkspace/crates/platform/trogon-gateway/src/source/gitlab/server.rs @@ -17,7 +17,7 @@ use std::pin::Pin; use tracing::{info, instrument, warn}; use trogon_nats::NatsToken; use trogon_nats::jetstream::{ - ClaimCheckPublisher, JetStreamContext, JetStreamPublisher, ObjectStorePut, PublishOutcome, + ClaimCheckPublisher, JetStreamContext, JetStreamPublisher, ObjectStorePut, ProvisionedStreamField, PublishOutcome, }; use trogon_semconv::span::GITLAB_WEBHOOK; use trogon_std::NonZeroDuration; @@ -87,8 +87,9 @@ pub async fn provision(js: &C, config: &GitlabConfig) -> Re // Reconciled rather than created-if-absent: `duplicate_window` is the // replay bound paired with the signature timestamp tolerance below, so a // stream provisioned before that pairing existed would otherwise keep - // JetStream's default window and leave replays live past it. The merge - // lists what this source owns; placement and limits stay the operator's. + // JetStream's default window and leave replays live past it. The listed + // fields are what this source owns; placement and limits stay the + // operator's. js.create_or_reconcile_stream( async_nats::jetstream::stream::Config { name: config.stream_name.as_str().to_owned(), @@ -97,11 +98,11 @@ pub async fn provision(js: &C, config: &GitlabConfig) -> Re max_age: config.stream_max_age.into(), ..Default::default() }, - |current, desired| { - current.subjects = desired.subjects.clone(); - current.duplicate_window = desired.duplicate_window; - current.max_age = desired.max_age; - }, + &[ + ProvisionedStreamField::Subjects, + ProvisionedStreamField::DuplicateWindow, + ProvisionedStreamField::MaxAge, + ], ) .await?; diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs index 1f28dd004..f26fd60e6 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs @@ -10,7 +10,7 @@ use bytes::Bytes; use super::message::{JsAck, JsAckWith, JsDoubleAck, JsDoubleAckWith, JsMessageRef}; use super::traits::{ JetStreamContext, JetStreamCreateKeyValue, JetStreamGetKeyValue, JetStreamGetStream, JetStreamPublishMessage, - JetStreamPublisher, + JetStreamPublisher, ProvisionedStreamField, reconciled_stream_config, }; #[derive(Clone)] @@ -39,15 +39,11 @@ impl JetStreamContext for NatsJetStreamClient { self.context.get_or_create_stream(config).await } - async fn create_or_reconcile_stream( + async fn create_or_reconcile_stream + Send>( &self, desired: S, - merge: F, - ) -> Result<(), async_nats::jetstream::context::CreateStreamError> - where - S: Into + Send, - F: FnOnce(&mut stream::Config, &stream::Config) + Send, - { + owned: &[ProvisionedStreamField], + ) -> Result<(), async_nats::jetstream::context::CreateStreamError> { let desired = desired.into(); // A lookup that fails for any reason falls through to create, which // errors on a name already in use. Failing provisioning is the right @@ -57,9 +53,8 @@ impl JetStreamContext for NatsJetStreamClient { self.context.create_stream(desired).await?; return Ok(()); }; - let mut current = stream.cached_info().config.clone(); - merge(&mut current, &desired); - self.context.update_stream(¤t).await?; + let reconciled = reconciled_stream_config(&stream.cached_info().config, &desired, owned); + self.context.update_stream(&reconciled).await?; Ok(()) } } diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs index a41a07fb9..227f497e3 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs @@ -32,7 +32,7 @@ use super::traits::{ JetStreamGetRawMessage, JetStreamGetStream, JetStreamGetStreamInfo, JetStreamKeyValueCreateWithTtl, JetStreamKeyValueDeleteExpectRevision, JetStreamKeyValueStatus, JetStreamKeyValueUpdate, JetStreamKvCreate, JetStreamKvEntry, JetStreamKvGet, JetStreamKvKeys, JetStreamLastRawMessageBySubject, JetStreamPublishMessage, - JetStreamPublisher, JetStreamSubjectPurger, + JetStreamPublisher, JetStreamSubjectPurger, ProvisionedStreamField, reconciled_stream_config, }; use crate::mocks::MockError; @@ -278,21 +278,21 @@ impl JetStreamContext for MockJetStreamContext { Ok(()) } - /// Merges into the stored config for a matching name rather than replacing - /// it, so a test can observe that a field the caller does not own survives - /// provisioning. - async fn create_or_reconcile_stream(&self, desired: S, merge: F) -> Result<(), MockStreamProvisionError> - where - S: Into + Send, - F: FnOnce(&mut stream::Config, &stream::Config) + Send, - { + /// Reconciles into the stored config for a matching name rather than + /// replacing it, so a test can observe that a field the caller does not + /// own survives provisioning. + async fn create_or_reconcile_stream + Send>( + &self, + desired: S, + owned: &[ProvisionedStreamField], + ) -> Result<(), MockStreamProvisionError> { let desired = desired.into(); if self.take_failure() { return Err(MockStreamProvisionError::Reconciliation); } let mut streams = self.created_streams.lock().unwrap(); match streams.iter_mut().find(|existing| existing.name == desired.name) { - Some(existing) => merge(existing, &desired), + Some(existing) => *existing = reconciled_stream_config(existing, &desired, owned), None => streams.push(desired), } Ok(()) diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs index 6e86794b2..807093259 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs @@ -83,7 +83,7 @@ async fn mock_context_names_a_reconcile_refusal_apart_from_a_creation_one() { let ctx = MockJetStreamContext::new(); ctx.fail_next(); let err = ctx - .create_or_reconcile_stream(stream::Config::default(), |_, _| {}) + .create_or_reconcile_stream(stream::Config::default(), &[ProvisionedStreamField::Subjects]) .await .unwrap_err(); assert_eq!(err, MockStreamProvisionError::Reconciliation); diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs index a308a324a..96de58973 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mod.rs @@ -38,7 +38,8 @@ pub use traits::{ JetStreamGetRawMessage, JetStreamGetStream, JetStreamGetStreamInfo, JetStreamKeyValueCreateWithTtl, JetStreamKeyValueDeleteExpectRevision, JetStreamKeyValueStatus, JetStreamKeyValueUpdate, JetStreamKvCreate, JetStreamKvEntry, JetStreamKvGet, JetStreamKvKeys, JetStreamLastRawMessageBySubject, JetStreamPublishMessage, - JetStreamPublisher, JetStreamSubjectPurger, JsMessageOf, PurgeOutcome, + JetStreamPublisher, JetStreamSubjectPurger, JsMessageOf, ProvisionedStreamField, PurgeOutcome, + reconciled_stream_config, }; #[cfg(any(test, feature = "test-support"))] diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs index 4e229f176..601c6e15d 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs @@ -12,6 +12,46 @@ use futures::Stream; use std::error::Error; use std::future::{Future, IntoFuture}; +/// A [`stream::Config`] field a service that provisions a stream is +/// authoritative for. +/// +/// Everything absent from this enum belongs to whoever runs the cluster: +/// placement, storage tier, retention limits. Naming the owned fields as a +/// closed set, rather than handing reconciliation a merge closure, is what +/// makes the split hold -- there is no variant for `num_replicas`, so no +/// provisioning code can write one back. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProvisionedStreamField { + Subjects, + DuplicateWindow, + MaxAge, +} + +impl ProvisionedStreamField { + fn reconcile(self, live: &mut stream::Config, declared: &stream::Config) { + match self { + Self::Subjects => live.subjects.clone_from(&declared.subjects), + Self::DuplicateWindow => live.duplicate_window = declared.duplicate_window, + Self::MaxAge => live.max_age = declared.max_age, + } + } +} + +/// The config a stream should hold once the `owned` fields of `declared` are +/// applied to `live`, leaving every other field as the server reports it. +#[must_use] +pub fn reconciled_stream_config( + live: &stream::Config, + declared: &stream::Config, + owned: &[ProvisionedStreamField], +) -> stream::Config { + let mut reconciled = live.clone(); + for field in owned { + field.reconcile(&mut reconciled, declared); + } + reconciled +} + pub trait JetStreamContext: Send + Sync + Clone + 'static { type Error: Error + Send + Sync; type Stream: Send; @@ -22,8 +62,7 @@ pub trait JetStreamContext: Send + Sync + Clone + 'static { ) -> impl Future> + Send; /// Create `desired` when the stream is absent; otherwise read the live - /// config, let `merge` copy over the fields this service is authoritative - /// for, and write the result back. + /// config, apply the fields named by `owned`, and write the result back. /// /// Two behaviours are wrong here and this method is the narrow path /// between them. [`Self::get_or_create_stream`] returns an existing stream @@ -36,16 +75,13 @@ pub trait JetStreamContext: Send + Sync + Clone + 'static { /// count, storage tier, and retention limits all roll back on the next /// boot. /// - /// `merge` is what separates the two. It names the fields the service - /// owns, and everything it does not touch stays as the server reports it. - fn create_or_reconcile_stream( + /// [`ProvisionedStreamField`] is what separates the two, and it is a + /// closed set precisely so a caller cannot widen its own authority. + fn create_or_reconcile_stream + Send>( &self, desired: S, - merge: F, - ) -> impl Future> + Send - where - S: Into + Send, - F: FnOnce(&mut stream::Config, &stream::Config) + Send; + owned: &[ProvisionedStreamField], + ) -> impl Future> + Send; } pub trait JetStreamKeyValueStatus: Send + Sync + Clone + 'static { diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits/tests.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits/tests.rs index b0806b3a8..fb964d942 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits/tests.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits/tests.rs @@ -1,6 +1,7 @@ -use super::PurgeOutcome; +use super::{ProvisionedStreamField, PurgeOutcome, reconciled_stream_config}; use async_nats::jetstream::stream; use serde_json::json; +use std::time::Duration; fn purge_response(success: bool) -> stream::PurgeResponse { serde_json::from_value(json!({ "success": success, "purged": 0_u64 })).unwrap() @@ -16,3 +17,47 @@ fn purge_response_outcome_reflects_success_field() { assert!(purge_response(true).is_success()); assert!(!purge_response(false).is_success()); } + +fn operator_managed() -> stream::Config { + stream::Config { + name: "OPERATED".to_owned(), + subjects: vec!["stale.>".to_owned()], + duplicate_window: Duration::from_secs(120), + num_replicas: 3, + storage: stream::StorageType::Memory, + max_bytes: 1_024, + ..Default::default() + } +} + +#[test] +fn reconciling_applies_only_the_named_fields() { + let declared = stream::Config { + name: "OPERATED".to_owned(), + subjects: vec!["fresh.>".to_owned()], + duplicate_window: Duration::from_secs(300), + max_age: Duration::from_secs(3_600), + ..Default::default() + }; + + let reconciled = reconciled_stream_config( + &operator_managed(), + &declared, + &[ProvisionedStreamField::Subjects, ProvisionedStreamField::MaxAge], + ); + + assert_eq!(reconciled.subjects, vec!["fresh.>"]); + assert_eq!(reconciled.max_age, Duration::from_secs(3_600)); + // Unnamed, so the declared value never reaches the server, defaults least + // of all: this is the roll-back a whole-config update would have caused. + assert_eq!(reconciled.duplicate_window, Duration::from_secs(120)); + assert_eq!(reconciled.num_replicas, 3); + assert_eq!(reconciled.storage, stream::StorageType::Memory); + assert_eq!(reconciled.max_bytes, 1_024); +} + +#[test] +fn reconciling_nothing_leaves_the_live_config_alone() { + let live = operator_managed(); + assert_eq!(reconciled_stream_config(&live, &stream::Config::default(), &[]), live); +} From fb1f50d3fce3efb7e0a7980cd635f156f09da4a5 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 21:50:17 -0400 Subject: [PATCH 25/32] fix(trogon-nats): stop reconciliation from rewriting a stream that already agrees Signed-off-by: Yordis Prieto --- .../trogon-nats/src/jetstream/client.rs | 10 ++++++- .../trogon-nats/src/jetstream/mocks.rs | 30 +++++++++++++++---- .../trogon-nats/src/jetstream/mocks/tests.rs | 24 +++++++++++++++ .../trogon-nats/src/jetstream/traits.rs | 8 +++++ 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs index f26fd60e6..86ff7512c 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/client.rs @@ -53,7 +53,15 @@ impl JetStreamContext for NatsJetStreamClient { self.context.create_stream(desired).await?; return Ok(()); }; - let reconciled = reconciled_stream_config(&stream.cached_info().config, &desired, owned); + let live = &stream.cached_info().config; + let reconciled = reconciled_stream_config(live, &desired, owned); + // An update would send the whole config back, so a stream already + // holding what we declare is one we leave alone rather than one we + // rewrite identically: no write, no window for an operator's + // concurrent edit to fall into. + if reconciled == *live { + return Ok(()); + } self.context.update_stream(&reconciled).await?; Ok(()) } diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs index 227f497e3..bbe9c5f68 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks.rs @@ -227,6 +227,7 @@ pub enum MockStreamProvisionError { pub struct MockJetStreamContext { created_streams: Arc>>, should_fail: Arc>, + stream_writes: Arc>, } impl MockJetStreamContext { @@ -234,6 +235,7 @@ impl MockJetStreamContext { Self { created_streams: Arc::new(Mutex::new(Vec::new())), should_fail: Arc::new(Mutex::new(false)), + stream_writes: Arc::new(Mutex::new(0)), } } @@ -241,6 +243,13 @@ impl MockJetStreamContext { self.created_streams.lock().unwrap().clone() } + /// How many times a stream config was written. A reconcile that finds the + /// stream already holding what the caller declares writes nothing, so it + /// does not count here. + pub fn stream_writes(&self) -> usize { + *self.stream_writes.lock().unwrap() + } + pub fn fail_next(&self) { *self.should_fail.lock().unwrap() = true; } @@ -275,12 +284,14 @@ impl JetStreamContext for MockJetStreamContext { return Err(MockStreamProvisionError::Creation); } self.created_streams.lock().unwrap().push(config); + *self.stream_writes.lock().unwrap() += 1; Ok(()) } /// Reconciles into the stored config for a matching name rather than /// replacing it, so a test can observe that a field the caller does not - /// own survives provisioning. + /// own survives provisioning, and records no write when the reconcile + /// changes nothing. async fn create_or_reconcile_stream + Send>( &self, desired: S, @@ -290,11 +301,20 @@ impl JetStreamContext for MockJetStreamContext { if self.take_failure() { return Err(MockStreamProvisionError::Reconciliation); } - let mut streams = self.created_streams.lock().unwrap(); - match streams.iter_mut().find(|existing| existing.name == desired.name) { - Some(existing) => *existing = reconciled_stream_config(existing, &desired, owned), - None => streams.push(desired), + { + let mut streams = self.created_streams.lock().unwrap(); + match streams.iter_mut().find(|existing| existing.name == desired.name) { + Some(existing) => { + let reconciled = reconciled_stream_config(existing, &desired, owned); + if reconciled == *existing { + return Ok(()); + } + *existing = reconciled; + } + None => streams.push(desired), + } } + *self.stream_writes.lock().unwrap() += 1; Ok(()) } } diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs index 807093259..2a1bdf57d 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/mocks/tests.rs @@ -89,6 +89,30 @@ async fn mock_context_names_a_reconcile_refusal_apart_from_a_creation_one() { assert_eq!(err, MockStreamProvisionError::Reconciliation); } +#[tokio::test] +async fn reconciling_a_stream_that_already_matches_writes_nothing() { + let ctx = MockJetStreamContext::new(); + let declared = stream::Config { + name: "RECONCILED".to_owned(), + subjects: vec!["reconciled.>".to_owned()], + duplicate_window: Duration::from_secs(300), + ..Default::default() + }; + let owned = [ + ProvisionedStreamField::Subjects, + ProvisionedStreamField::DuplicateWindow, + ]; + + ctx.create_or_reconcile_stream(declared.clone(), &owned).await.unwrap(); + assert_eq!(ctx.stream_writes(), 1); + + // The second boot finds the stream already holding what it declares. An + // update would resend the whole config for nothing, and every resend is a + // chance to land on top of an operator's concurrent edit. + ctx.create_or_reconcile_stream(declared, &owned).await.unwrap(); + assert_eq!(ctx.stream_writes(), 1); +} + #[tokio::test] async fn mock_publisher_records_publishes() { let pub_mock = MockJetStreamPublisher::new(); diff --git a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs index 601c6e15d..cecce3d74 100644 --- a/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs +++ b/rsworkspace/crates/platform/trogon-nats/src/jetstream/traits.rs @@ -77,6 +77,14 @@ pub trait JetStreamContext: Send + Sync + Clone + 'static { /// /// [`ProvisionedStreamField`] is what separates the two, and it is a /// closed set precisely so a caller cannot widen its own authority. + /// + /// Implementations must write nothing when every named field already + /// matches what the server reports. `STREAM.UPDATE` carries the whole + /// config and accepts no expected-revision, so any write races an + /// operator editing the same stream and one of the two changes is lost. + /// JetStream offers no conditional form that would close that window, so + /// not writing is what keeps it shut, and a boot against an + /// already-reconciled stream is the case that actually happens. fn create_or_reconcile_stream + Send>( &self, desired: S, From 3a5bc0f9bc05be77de057763ac312a11c79221c5 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 22:06:02 -0400 Subject: [PATCH 26/32] chore(adr): keep the crypto suite's federation reference navigable Signed-off-by: Yordis Prieto --- docs/adr/0038-agent-identity-crypto-suite.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/adr/0038-agent-identity-crypto-suite.md b/docs/adr/0038-agent-identity-crypto-suite.md index 0d52fa42e..d15eea148 100644 --- a/docs/adr/0038-agent-identity-crypto-suite.md +++ b/docs/adr/0038-agent-identity-crypto-suite.md @@ -174,8 +174,8 @@ at all, and its entry records where it lives instead: one issuer key able to serve many agents without becoming any of their anchors. The "only RSA keys" requirement additionally forces a *separate* published key set rather than an extra key in the AAuth well-known - documents, which is why the surface itself is decided in ADR#0053 rather - than here. + documents, which is why the surface itself is decided in + [ADR#0053](./0053-external-oidc-federation-surface.md) rather than here. ### 5. Where the implementation currently stands, and where it diverges From 24da38c96c46680b3afa83ff3b48292984f98640 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 7 Aug 2026 22:06:06 -0400 Subject: [PATCH 27/32] fix(trogon-channel): stop bucket provisioning tests from resting on who wins a race Whichever concurrent create the scheduler and the server ordered first decided whether the arm under test ran at all, so the suite failed on CI for reasons the code under test had nothing to do with. Signed-off-by: Yordis Prieto --- .../channel/trogon-channel/src/store.rs | 12 +++- .../channel/trogon-channel/src/store/tests.rs | 64 ++++++++++--------- 2 files changed, 44 insertions(+), 32 deletions(-) diff --git a/rsworkspace/crates/channel/trogon-channel/src/store.rs b/rsworkspace/crates/channel/trogon-channel/src/store.rs index a739f2f3b..4ef8b465f 100644 --- a/rsworkspace/crates/channel/trogon-channel/src/store.rs +++ b/rsworkspace/crates/channel/trogon-channel/src/store.rs @@ -172,11 +172,17 @@ pub struct ChannelStore { /// through would let a momentary read failure reconfigure live storage. async fn ensure_bucket(js: &jetstream::Context, bucket: String) -> Result { match js.get_key_value(&bucket).await { - Ok(store) => return Ok(store), - Err(source) if is_get_key_value_not_found(&source) => {} - Err(source) => return Err(ChannelStoreError::OpenBucket { bucket, source }), + Ok(store) => Ok(store), + Err(source) if is_get_key_value_not_found(&source) => create_bucket(js, bucket).await, + Err(source) => Err(ChannelStoreError::OpenBucket { bucket, source }), } +} +/// The half of [`ensure_bucket`] that runs once the bucket has been found +/// missing, split out so the interleaving it exists to survive can be staged +/// rather than raced for: another replica creating the same bucket, with a +/// config of its own, in the window this call opens. +async fn create_bucket(js: &jetstream::Context, bucket: String) -> Result { info!(bucket = %bucket, "Creating channel KV bucket"); match js .create_key_value(jetstream::kv::Config { diff --git a/rsworkspace/crates/channel/trogon-channel/src/store/tests.rs b/rsworkspace/crates/channel/trogon-channel/src/store/tests.rs index 3388158ad..80b767cfa 100644 --- a/rsworkspace/crates/channel/trogon-channel/src/store/tests.rs +++ b/rsworkspace/crates/channel/trogon-channel/src/store/tests.rs @@ -573,39 +573,47 @@ async fn a_claim_that_cannot_be_read_back_takes_the_conversation_record_with_it( /// `ensure_is_idempotent_under_concurrent_creation` races two *identical* /// configs, and neither side ever takes this arm: `STREAM.CREATE` only /// errors when the stream that beat it has a different config, and an -/// identical race succeeds silently on both sides. Racing a bare create -/// against `ensure_bucket`'s own get-then-create for the same bucket name -/// reliably loses that race instead: the bare create skips the get's extra -/// round trip, so its differently-configured bucket already exists by the -/// time this store's own create is rejected. +/// identical race succeeds silently on both sides. Reaching the arm needs a +/// differently-configured bucket to exist by the time the create runs, and +/// which of two concurrent calls the scheduler and the server put first is +/// not something a test gets to decide, so the halves of `ensure_bucket` are +/// called in the order the losing interleaving would have produced. #[tokio::test] async fn a_bucket_created_with_a_different_config_between_the_get_and_the_create_is_still_opened() { let server = JetStreamTestServer::start().await; let js = server.jetstream().await; let bucket = "conflict".to_string(); - // `ensure_bucket` is private and reachable only through `ChannelStore::ensure`, - // so it is called directly here to race a single bucket instead of all four. - let racing_create = js.create_key_value(jetstream::kv::Config { + // Matched rather than `expect_err`ed because a `Store` is not `Debug`. + let Err(missing) = js.get_key_value(&bucket).await else { + panic!("the bucket must be absent for the create half to be what runs next"); + }; + assert!( + is_get_key_value_not_found(&missing), + "expected the bucket to read as absent, got {missing:?}" + ); + + js.create_key_value(jetstream::kv::Config { bucket: bucket.clone(), history: 1, storage: jetstream::stream::StorageType::File, ..Default::default() - }); - - let (ours, theirs) = tokio::join!(ensure_bucket(&js, bucket.clone()), racing_create); + }) + .await + .expect("the other replica's create must land for there to be anything to recover"); - ours.expect("ensure_bucket must recover the bucket the race left behind"); - theirs.expect("the racing create must succeed for there to be anything to recover"); + create_bucket(&js, bucket.clone()) + .await + .expect("create_bucket must open the bucket the replica that beat it left behind"); let mut stream = js .get_stream(format!("KV_{bucket}")) .await - .expect("the bucket the race created must still be there"); + .expect("the bucket the other replica created must still be there"); let info = stream.info().await.expect("stream info"); assert_eq!( info.config.max_messages_per_subject, 1, - "the surviving config must be the racing create's, not ensure_bucket's own attempt" + "the surviving config must be the winner's, not the rejected create's" ); } @@ -641,12 +649,12 @@ async fn a_bucket_whose_subject_space_is_already_claimed_fails_to_create() { ); } -/// Line 105's arm: the already-exists recovery read can itself fail. Racing -/// a stream into existence with `max_messages_per_subject` below the minimum -/// a real KV config ever produces (`kv_to_stream_config` floors it at 1) -/// forces exactly that: the name conflict sends `ensure_bucket` to recover by -/// reading the bucket back, and that read rejects what it finds as not a -/// valid KV store rather than returning it. +/// `create_bucket`'s already-exists arm recovers by reading the bucket back, +/// and that read can itself fail. A stream claiming the bucket's name with +/// `max_messages_per_subject` below the minimum a real KV config ever +/// produces (`kv_to_stream_config` floors it at 1) forces exactly that: the +/// name conflict sends the create off to recover, and the recovery read +/// rejects what it finds as not a valid KV store rather than returning it. #[tokio::test] async fn a_recovery_read_that_also_fails_surfaces_as_a_bucket_read_failure() { let server = JetStreamTestServer::start().await; @@ -655,19 +663,17 @@ async fn a_recovery_read_that_also_fails_surfaces_as_a_bucket_read_failure() { // Plain `create_stream`, not `create_key_value`, because the KV wrapper // floors `max_messages_per_subject` at 1 and could never produce this. - let racing_create = js.create_stream(jetstream::stream::Config { + js.create_stream(jetstream::stream::Config { name: format!("KV_{bucket}"), subjects: vec![format!("$KV.{bucket}.>")], max_messages_per_subject: 0, ..Default::default() - }); - - let (ours, theirs) = tokio::join!(ensure_bucket(&js, bucket.clone()), racing_create); - - theirs.expect("the racing create must win for there to be a conflicting bucket to recover"); + }) + .await + .expect("the conflicting bucket must exist for there to be a recovery read to fail"); - let Err(error) = ours else { - panic!("ensure_bucket must fail when its own recovery read also fails"); + let Err(error) = create_bucket(&js, bucket.clone()).await else { + panic!("create_bucket must fail when its own recovery read also fails"); }; assert!( matches!(error, ChannelStoreError::OpenBucket { .. }), From 99167e92d12f546ebe0f3cbe91d413d3d9c8b9da Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 8 Aug 2026 00:56:33 -0400 Subject: [PATCH 28/32] chore(trogon-aauth-verify): pin the digest member this reader refuses to skip The last-wins note read as a promise to recover from a member that does not parse, and nothing held the parser to refusing one, so a later reader could have relaxed it into burying an unreadable digest under a trailing good one. Signed-off-by: Yordis Prieto --- .../aauth/trogon-aauth-verify/src/http_pop.rs | 7 +++++++ .../trogon-aauth-verify/src/http_pop/tests.rs | 21 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs index d9c6849b3..e7affe7c3 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs @@ -560,6 +560,13 @@ fn verify_content_digest(req: &HttpRequest) -> Result<(), HttpPopError> { /// make this verifier disagree with any RFC-compliant peer about which digest /// a duplicated `sha-256` member names. /// +/// Last-wins ranges over the members this reader can read. A `sha-256` member +/// it cannot fails the whole value rather than yielding to a later one, which +/// is what an RFC-compliant peer does too: RFC 8941 discards a field that does +/// not parse, and a discarded `Content-Digest` leaves nothing to check the body +/// against. Skipping ahead would instead let a sender bury a member this +/// verifier cannot read under a trailing one it can. +/// /// This is a targeted reader for one Byte Sequence member, not a general /// Structured Fields parser. It does not model Inner Lists, and a parameter /// whose value is a String containing `,` or `;` would split wrongly; no diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs index 958f8cbf6..82dc84184 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs @@ -312,6 +312,27 @@ async fn verify_rejects_when_the_last_duplicated_sha256_member_mismatches() { assert!(matches!(err, HttpPopError::ContentDigestMismatch)); } +#[tokio::test(flavor = "current_thread")] +async fn verify_rejects_a_readable_sha256_member_trailing_an_unreadable_one() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // Last-wins ranges over the members this verifier can read. RFC 8941 + // discards a field carrying a member that does not parse, so a `sha-256` + // whose Byte Sequence cannot be decoded refuses the request rather than + // yielding to whatever follows it. Recovering would let a sender bury an + // unreadable member under a trailing readable one. + let body = br#"{"scope":"data.read"}"#; + let live = STANDARD.encode(Sha256::digest(body)); + let unreadable = format!("sha-256=:not base64!:, sha-256=:{live}:"); + let req = signed_body_request(&fixture, &jwt, body, unreadable); + + let err = verifier.verify(&req).await.unwrap_err(); + assert!(matches!(err, HttpPopError::UnsupportedContentDigest)); +} + #[tokio::test(flavor = "current_thread")] async fn verify_rejects_content_digest_without_sha256_entry() { let fixture = p256_fixture("k1"); From 43f75331a089cc71bf3136a462ddd5140600e154 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 8 Aug 2026 01:40:09 -0400 Subject: [PATCH 29/32] fix(trogon-jwks-publisher): keep a set's rejection reason inside the error-naming convention Signed-off-by: Yordis Prieto --- .../platform/trogon-jwks-publisher/src/publisher.rs | 10 +++++----- .../trogon-jwks-publisher/src/publisher/tests.rs | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs index 22b0f38bd..b5a5082e4 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher.rs @@ -58,7 +58,7 @@ pub enum PublisherError { Unpublishable { dwk: String, #[source] - source: UnpublishableJwkSet, + source: PublishableJwkSetError, }, #[error("invalid EC PKCS8 PEM for kid {kid:?}: {source}")] InvalidPem { @@ -81,7 +81,7 @@ fn is_known_dwk(dwk: &str) -> bool { /// document a set was registered under is the registrar's context, not the /// set's own. #[derive(Debug, thiserror::Error)] -pub enum UnpublishableJwkSet { +pub enum PublishableJwkSetError { #[error("key id {kid:?} is published more than once")] DuplicateKeyId { kid: String }, #[error("{keys} keys are published and at least one omits `kid`; only a single-key set may omit it")] @@ -113,7 +113,7 @@ pub enum UnpublishableJwkSet { pub struct PublishableJwkSet(JwkSet); impl TryFrom for PublishableJwkSet { - type Error = UnpublishableJwkSet; + type Error = PublishableJwkSetError; fn try_from(set: JwkSet) -> Result { let multi_key = set.keys.len() > 1; @@ -122,11 +122,11 @@ impl TryFrom for PublishableJwkSet { match jwk.common.key_id.as_deref() { Some(kid) => { if !seen.insert(kid) { - return Err(UnpublishableJwkSet::DuplicateKeyId { kid: kid.to_owned() }); + return Err(PublishableJwkSetError::DuplicateKeyId { kid: kid.to_owned() }); } } None if multi_key => { - return Err(UnpublishableJwkSet::MissingKeyId { keys: set.keys.len() }); + return Err(PublishableJwkSetError::MissingKeyId { keys: set.keys.len() }); } None => {} } diff --git a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs index 5e873dfca..12a21e5f7 100644 --- a/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs +++ b/rsworkspace/crates/platform/trogon-jwks-publisher/src/publisher/tests.rs @@ -137,7 +137,7 @@ fn builder_rejects_a_set_that_repeats_a_key_id() { &err, PublisherError::Unpublishable { dwk, - source: UnpublishableJwkSet::DuplicateKeyId { kid }, + source: PublishableJwkSetError::DuplicateKeyId { kid }, } if dwk == DWK_AGENT && kid.as_str() == "same" ), "{err}" @@ -153,7 +153,7 @@ fn a_set_that_repeats_a_key_id_cannot_be_built_at_all() { }) .unwrap_err(); assert!( - matches!(&err, UnpublishableJwkSet::DuplicateKeyId { kid } if kid.as_str() == "same"), + matches!(&err, PublishableJwkSetError::DuplicateKeyId { kid } if kid.as_str() == "same"), "{err}" ); } @@ -173,7 +173,7 @@ fn builder_rejects_a_multi_key_set_holding_an_unidentified_key() { &err, PublisherError::Unpublishable { dwk, - source: UnpublishableJwkSet::MissingKeyId { keys: 2 }, + source: PublishableJwkSetError::MissingKeyId { keys: 2 }, } if dwk == DWK_AGENT ), "{err}" From 49cbd7aa833eb64ea9b3365f0a87f5cde3c1e535 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 8 Aug 2026 01:40:09 -0400 Subject: [PATCH 30/32] chore(a2a-auth-callout): pin the RSA-only refusal the new key guards can hide Signed-off-by: Yordis Prieto --- .../src/credentials/oidc/tests.rs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs index 06fcfd1fd..69413fe6d 100644 --- a/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs +++ b/rsworkspace/crates/a2a/a2a-auth-callout/src/credentials/oidc/tests.rs @@ -374,6 +374,73 @@ async fn verify_fails_with_non_rsa_jwk() { assert_eq!(algorithm, jsonwebtoken::Algorithm::ES256); } +#[tokio::test] +async fn verify_fails_with_a_non_rsa_jwk_reached_under_an_allowed_algorithm() { + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let ec_jwk = Jwk { + common: CommonParameters { + key_id: Some("ec-kid".into()), + ..Default::default() + }, + algorithm: AlgorithmParameters::EllipticCurve(EllipticCurveKeyParameters { + key_type: EllipticCurveKeyType::EC, + curve: jsonwebtoken::jwk::EllipticCurve::P256, + x: "dummyx".into(), + y: "dummyy".into(), + }), + }; + let jwks = JwkSet { keys: vec![ec_jwk] }; + let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["aud".into()], jwks); + + // RS256 is allow-listed and the EC JWK declares no purpose of its own, so + // neither guard ahead of the key material refuses this token. What refuses + // it is the verifier's own RSA-only support, which the guards must not be + // allowed to mask. + let header_b64 = URL_SAFE_NO_PAD.encode(br#"{"alg":"RS256","kid":"ec-kid","typ":"JWT"}"#); + let payload_b64 = URL_SAFE_NO_PAD.encode(b"{}"); + let fake_token = format!("{header_b64}.{payload_b64}.sig"); + + let err = verifier + .verify_internal(&BearerToken::new(fake_token), &AudienceAccount::new("acct")) + .await + .unwrap_err(); + let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(message)) = err else { + panic!("expected InvalidCredentials, got {err:?}"); + }; + assert!(message.contains("must be RSA"), "{message}"); +} + +#[tokio::test] +async fn verify_fails_with_an_rsa_jwk_whose_components_do_not_decode() { + let issuer = OidcIssuerUrl::parse("https://issuer.example").unwrap(); + let broken_jwk = Jwk { + common: CommonParameters { + key_id: Some("broken-kid".into()), + ..Default::default() + }, + algorithm: AlgorithmParameters::RSA(RSAKeyParameters { + key_type: RSAKeyType::RSA, + n: "not base64url".into(), + e: "AQAB".into(), + }), + }; + let jwks = JwkSet { keys: vec![broken_jwk] }; + let verifier = JwksOidcVerifier::with_static_jwks(issuer, vec!["aud".into()], jwks); + + let header_b64 = URL_SAFE_NO_PAD.encode(br#"{"alg":"RS256","kid":"broken-kid","typ":"JWT"}"#); + let payload_b64 = URL_SAFE_NO_PAD.encode(b"{}"); + let fake_token = format!("{header_b64}.{payload_b64}.sig"); + + let err = verifier + .verify_internal(&BearerToken::new(fake_token), &AudienceAccount::new("acct")) + .await + .unwrap_err(); + let AuthCalloutError::CredentialVerification(CredentialError::InvalidCredentials(message)) = err else { + panic!("expected InvalidCredentials, got {err:?}"); + }; + assert!(message.contains("invalid RSA JWK components"), "{message}"); +} + #[tokio::test] async fn oidc_verifier_trait_delegates_to_verify_internal() { // Exercise the OidcVerifier::verify blanket impl on JwksOidcVerifier. From 19fe392cda22530d33f2da5a353e88372cb896d7 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 8 Aug 2026 01:40:17 -0400 Subject: [PATCH 31/32] chore(trogon-aauth-verify): pin the bare-key member this reader has to step over Signed-off-by: Yordis Prieto --- .../trogon-aauth-verify/src/http_pop/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs index 82dc84184..dd00e3b82 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs @@ -274,6 +274,23 @@ async fn verify_accepts_a_parameterized_sha256_content_digest_item() { verifier.verify(&req).await.expect("parameterized item verifies"); } +#[tokio::test(flavor = "current_thread")] +async fn verify_accepts_a_content_digest_carrying_a_bare_key_member() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // RFC 8941 spells a Dictionary member with no `=` as the Boolean true, so + // one carries no Byte Sequence to compare a body against. Reading past it + // keeps an unknown member from displacing the `sha-256` beside it. + let body = br#"{"scope":"data.read"}"#; + let with_bare_key = format!("unixsum, sha-256=:{}:", STANDARD.encode(Sha256::digest(body))); + let req = signed_body_request(&fixture, &jwt, body, with_bare_key); + + verifier.verify(&req).await.expect("a bare-key member is stepped over"); +} + #[tokio::test(flavor = "current_thread")] async fn verify_resolves_a_duplicated_sha256_member_to_the_last_one() { let fixture = p256_fixture("k1"); From b771fb57718edb26701d80c1ac98cbfa7308ae24 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 8 Aug 2026 02:26:49 -0400 Subject: [PATCH 32/32] fix(trogon-aauth-verify): stop a bare sha-256 member from leaving last-wins behind Signed-off-by: Yordis Prieto --- .../aauth/trogon-aauth-verify/src/http_pop.rs | 16 ++++++-- .../trogon-aauth-verify/src/http_pop/tests.rs | 39 +++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs index e7affe7c3..5ef4d6f5f 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop.rs @@ -567,6 +567,11 @@ fn verify_content_digest(req: &HttpRequest) -> Result<(), HttpPopError> { /// against. Skipping ahead would instead let a sender bury a member this /// verifier cannot read under a trailing one it can. /// +/// So what a member is keyed on, not whether it carries a Byte Sequence, +/// decides between skipping it and failing. RFC 8941 reads a member with no +/// `=` as the Boolean true, which is a `sha-256` naming no digest once it wins +/// last, and a peer is still free to key its own algorithms however it likes. +/// /// This is a targeted reader for one Byte Sequence member, not a general /// Structured Fields parser. It does not model Inner Lists, and a parameter /// whose value is a String containing `,` or `;` would split wrongly; no @@ -574,15 +579,18 @@ fn verify_content_digest(req: &HttpRequest) -> Result<(), HttpPopError> { fn parse_sha256_content_digest(raw: &str) -> Option> { let mut last = None; for member in raw.split(',') { - let Some((algorithm, value)) = member.split_once('=') else { - continue; + let (key, value) = match member.split_once('=') { + Some((key, value)) => (key, Some(value)), + None => (member, None), }; - if !algorithm.trim().eq_ignore_ascii_case("sha-256") { + // A Boolean member's parameters sit on the key, an Item's on its value. + let key = key.split(';').next().unwrap_or(key); + if !key.trim().eq_ignore_ascii_case("sha-256") { continue; } // The Byte Sequence ends at its closing colon; anything after that is // the parameter list. - let value = value.trim_start(); + let value = value?.trim_start(); let inner = value.strip_prefix(':')?.split_once(':')?.0; last = Some(decode_base64_any_alphabet(inner)?); } diff --git a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs index dd00e3b82..a7c1f2905 100644 --- a/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs +++ b/rsworkspace/crates/aauth/trogon-aauth-verify/src/http_pop/tests.rs @@ -291,6 +291,45 @@ async fn verify_accepts_a_content_digest_carrying_a_bare_key_member() { verifier.verify(&req).await.expect("a bare-key member is stepped over"); } +#[tokio::test(flavor = "current_thread")] +async fn verify_rejects_a_bare_sha256_member_trailing_a_readable_one() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // A trailing bare `sha-256` is the Boolean true, and last-wins hands the + // key to it: an RFC-compliant peer is left with no digest to check. Keeping + // the earlier Byte Sequence would let a sender show this verifier a body it + // has agreed to and every other component nothing at all. + let body = br#"{"scope":"data.read"}"#; + let live = STANDARD.encode(Sha256::digest(body)); + let displaced = format!("sha-256=:{live}:, sha-256"); + let req = signed_body_request(&fixture, &jwt, body, displaced); + + let err = verifier.verify(&req).await.unwrap_err(); + assert!(matches!(err, HttpPopError::UnsupportedContentDigest)); +} + +#[tokio::test(flavor = "current_thread")] +async fn verify_rejects_a_parameterized_bare_sha256_member() { + let fixture = p256_fixture("k1"); + let jwt = agent_jwt(&fixture, "k1", "agent-provider.example"); + let jwks = jwks_with_key("agent-provider.example", fixture.jwk.clone()); + let verifier = verifier_at(jwks, 1000, "resource.example"); + + // The same displacement dressed as a parameterized member: `sha-256;q=1` + // splits at the parameter's `=`, so the algorithm has to be read off the + // key ahead of the `;` for the member to be recognised as the Boolean it is. + let body = br#"{"scope":"data.read"}"#; + let live = STANDARD.encode(Sha256::digest(body)); + let displaced = format!("sha-256=:{live}:, sha-256;q=1"); + let req = signed_body_request(&fixture, &jwt, body, displaced); + + let err = verifier.verify(&req).await.unwrap_err(); + assert!(matches!(err, HttpPopError::UnsupportedContentDigest)); +} + #[tokio::test(flavor = "current_thread")] async fn verify_resolves_a_duplicated_sha256_member_to_the_last_one() { let fixture = p256_fixture("k1");