From ee07b6ae2069b066bc29081d22038a0cd3f9ca55 Mon Sep 17 00:00:00 2001 From: "Claude Opus 5.5" Date: Sat, 17 Oct 2026 18:30:13 +0000 Subject: [PATCH] fix(oidc): accept a token when the allowed algorithms mix key families OidcValidator built one jsonwebtoken Validation with every allowed algorithm. jsonwebtoken 9 refuses a key whose family differs from any algorithm in that list, so a config such as [RS256, ES256] rejected every token, RSA and EC alike. A token is now verified with its own header alg only. The allow-list check before key lookup is unchanged, so an alg outside the list is still refused as UnsupportedAlgorithm, and a key is used only for the family the token names. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 ++++++ cli-framework-oidc/src/server/mod.rs | 14 +++++++-- cli-framework-oidc/tests/multi_issuer.rs | 39 +++++++++++++++++++++++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 856948b..378c736 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -464,6 +464,16 @@ ### Fixed +- `cli-framework-oidc`: an `OidcValidationConfig` whose `algorithms` mix key + families (e.g. `[RS256, ES256]`, for a Keycloak realm that may sign with an + RSA or an EC key) rejected every token: all the algorithms were handed to + `jsonwebtoken`, which refuses a key whose family differs from any of them. + A token is now verified with its own header `alg` only (already checked to + be in `algorithms`), by a key of that family. A header naming another + allowed family than the key's is still rejected as + `TokenRejection::UnsupportedAlgorithm`. Configs with one algorithm, or one + family, behave as before. + - `cli-framework-oidc`: single-issuer validation (`OidcValidator::new`, `oidc_validation_layer`) and the bearer-token checks of the browser layers (`oidc_browser_session_layer`, `oidc_dual_mode_layer`) now require the `iss` diff --git a/cli-framework-oidc/src/server/mod.rs b/cli-framework-oidc/src/server/mod.rs index c9788cb..8243a7c 100644 --- a/cli-framework-oidc/src/server/mod.rs +++ b/cli-framework-oidc/src/server/mod.rs @@ -132,6 +132,9 @@ pub struct OidcValidationConfig { /// `{issuer_url}/.well-known/openid-configuration`. Must be `None` when /// `static_jwks` is set. pub jwks_uri: Option, + /// The header `alg` values accepted. They may mix key families (e.g. + /// `[RS256, ES256]`): a token is verified with its own `alg` only, by a + /// key of that family. pub algorithms: Vec, pub jwks_ttl: Duration, pub clock_skew: Duration, @@ -385,7 +388,7 @@ impl IssuerState { let mut last_rejection: Option = None; for key in &keys { - match try_validate_jwt(token, key, self) { + match try_validate_jwt(token, key, header.alg, self) { Ok(claims) => return Ok(claims), Err(r) => { last_rejection = Some(r); @@ -864,15 +867,20 @@ fn jwt_err_to_rejection(e: &jsonwebtoken::errors::Error) -> TokenRejection { // ── Per-key JWT verification ───────────────────────────────────────────────── +/// `alg` is the token header's algorithm, already checked to be one of the +/// issuer's `algorithms`. Only it is handed to `jsonwebtoken`, which refuses a +/// key whose family differs from *any* algorithm it is given: passing the +/// whole set would reject every token once the set mixes families (e.g. +/// `[RS256, ES256]`). A key of another family than `alg` is still refused. fn try_validate_jwt( token: &str, key: &DecodingKey, + alg: Algorithm, issuer: &IssuerState, ) -> Result { let cfg = &issuer.cfg; let issuer_url = issuer.issuer_url.as_str(); - let mut validation = Validation::new(cfg.algorithms[0]); - validation.algorithms = cfg.algorithms.clone(); + let mut validation = Validation::new(alg); validation.set_issuer(&[issuer_url]); validation.set_required_spec_claims(crate::jwks::REQUIRED_SPEC_CLAIMS); crate::jwks::apply_audience_policy(&mut validation, &cfg.audience); diff --git a/cli-framework-oidc/tests/multi_issuer.rs b/cli-framework-oidc/tests/multi_issuer.rs index 1ca9445..ec0c4e2 100644 --- a/cli-framework-oidc/tests/multi_issuer.rs +++ b/cli-framework-oidc/tests/multi_issuer.rs @@ -15,7 +15,7 @@ use cli_framework_oidc::server::{ oidc_validation_layer_multi, AudiencePolicy, JwkSet, OidcClaims, OidcValidationConfig, OidcValidationError, OidcValidator, TokenRejection, DEFAULT_ROLES_CLAIM_PATH, }; -use cli_framework_oidc::test_support::{make_cfg, mint_jwt, TestIssuer}; +use cli_framework_oidc::test_support::{make_cfg, mint_jwt, now_secs, TestIssuer}; use cli_framework_oidc::OidcConfigError; const A: &str = "https://issuer-a.test"; @@ -278,6 +278,43 @@ async fn algorithms_are_enforced_per_issuer() { assert_eq!(rejection(err), TokenRejection::UnsupportedAlgorithm); } +#[tokio::test] +async fn algorithms_of_different_families_can_be_allowed_together() { + use base64::engine::general_purpose::URL_SAFE_NO_PAD; + use base64::Engine; + + // An issuer that may sign with RSA or EC keys (Keycloak can do either). + for algorithms in [ + vec![Algorithm::RS256, Algorithm::ES256], + vec![Algorithm::ES256, Algorithm::RS256], + ] { + let a = TestIssuer::new(A); + let v = OidcValidator::new(OidcValidationConfig { + algorithms: algorithms.clone(), + ..a.config() + }) + .unwrap(); + let claims = v.validate(&a.mint(json!({"sub": "u"}))).await; + assert_eq!(claims.unwrap().sub, "u", "{algorithms:?}"); + + // A header naming the other allowed family does not make the EC key + // verify it. + let kid = format!("{A}#key-1"); + let header = URL_SAFE_NO_PAD.encode(json!({"alg": "RS256", "kid": kid}).to_string()); + let payload = URL_SAFE_NO_PAD + .encode(json!({"sub": "u", "iss": A, "exp": now_secs() + 300}).to_string()); + let forged = format!("{header}.{payload}.{}", URL_SAFE_NO_PAD.encode([7u8; 256])); + let err = v.validate(&forged).await.unwrap_err(); + assert_eq!(rejection(err), TokenRejection::UnsupportedAlgorithm); + + // An algorithm outside the set is still refused before any key. + let header = URL_SAFE_NO_PAD.encode(json!({"alg": "HS256", "kid": kid}).to_string()); + let hs = format!("{header}.{payload}.{}", URL_SAFE_NO_PAD.encode([7u8; 32])); + let err = v.validate(&hs).await.unwrap_err(); + assert_eq!(rejection(err), TokenRejection::UnsupportedAlgorithm); + } +} + #[tokio::test] async fn expired_token_from_a_trusted_issuer_is_expired() { let (a, _b, v) = two_issuers();