Skip to content

Commit

Permalink
Allow the use of all appropriate crypto algorithms that match the JWK…
Browse files Browse the repository at this point in the history
… retrieved from the URL (#1613)

### What
This PR changes the behaviour of JWT validation when the user has
configured getting their JWK from a URL in the AuthConfig.

Previously we used the algorithm specified on the JWK and only validated
the JWT if it was signed with that algorithm. However, in HGE v2, we did
not look at this field, and instead validated the JWT with the algorithm
it was signed with (so long as that algorithm was compatible with the
key). This is better behaviour, IMHO, because the algorithm field on the
JWK is optional (and customers often don't set it), plus I suspect it is
more useful to specify which algorithm to use when _signing_ a JWT with
that JWK, rather than _validating_ an existing signature.

In this PR, we widen the acceptable algorithms to validate a JWT with to
be all algorithms compatible with the JWK. However, this behaviour only
applies to JWKs retrieved from a URL. For inline JWTs, we currently
provide an `algorithm` property in the AuthConfig that specifies a
single algorithm to use. I think this should be widened into a list to
allow for multiple algorithms (and JWKs from a URL should also be able
to optionally specify a list of acceptable algorithms), but that can be
left to another PR.

### How
There is a new function `get_acceptable_algorithms_for_key` that returns
all acceptable algorithms for the specific JWK key type. It is used to
set the acceptable algorithms to use during validation.

V3_GIT_ORIGIN_REV_ID: 3c37a45b37611593b41b31968726923f45336bde
  • Loading branch information
daniel-chambers authored and hasura-bot committed Feb 12, 2025
1 parent 0f3d615 commit 97a2e09
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 9 deletions.
5 changes: 5 additions & 0 deletions v3/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@

### Changed

- When JWKs are configured to be read from a URL in AuthConfig, we now no longer
require the JWT to specify the `alg` property. We will validate the signature
in a JWT so long as the algorithm used is supported by the JWK retrieved from
the URL.

## [v2025.02.07]

### Added
Expand Down
42 changes: 33 additions & 9 deletions v3/crates/auth/hasura-authn-jwt/src/jwt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ async fn get_decoding_key_from_jwk_url(
http_client: &reqwest::Client,
jwk_url: Url,
jwt_authorization_header: &str,
) -> Result<(jwt::Algorithm, jwt::DecodingKey), Error> {
) -> Result<(Vec<jwt::Algorithm>, jwt::DecodingKey), Error> {
let tracer = tracing_util::global_tracer();
tracer
.in_span_async("fetch_jwk", "Fetch JWK", SpanVisibility::Internal, || {
Expand Down Expand Up @@ -494,11 +494,8 @@ async fn get_decoding_key_from_jwk_url(
.ok_or(InternalError::NoMatchingJWKFound { kid })?;
let decoding_key = jwt::DecodingKey::from_jwk(jwk)
.map_err(InternalError::JWTDecodingKeyError)?;
let algorithm = jwk
.common
.algorithm
.ok_or(InternalError::AlgorithmNotFoundInJWK)?;
Ok((algorithm, decoding_key))
let acceptable_algorithms = get_acceptable_algorithms_for_key(jwk);
Ok((acceptable_algorithms, decoding_key))
} else {
Err(InternalError::UnsuccessfulJWKFetch(jwk_response.status()))?
}
Expand All @@ -507,6 +504,32 @@ async fn get_decoding_key_from_jwk_url(
.await
}

fn get_acceptable_algorithms_for_key(jwk: &jwt::jwk::Jwk) -> Vec<jwt::Algorithm> {
match &jwk.algorithm {
// Elliptic curve family algorithms
jsonwebtoken::jwk::AlgorithmParameters::EllipticCurve(_) => {
vec![jwt::Algorithm::ES256, jwt::Algorithm::ES384]
}
// RSA family algorithms
jsonwebtoken::jwk::AlgorithmParameters::RSA(_) => vec![
jwt::Algorithm::RS256,
jwt::Algorithm::RS384,
jwt::Algorithm::RS512,
jwt::Algorithm::PS256,
jwt::Algorithm::PS384,
jwt::Algorithm::PS512,
],
// HMAC family algorithms
jsonwebtoken::jwk::AlgorithmParameters::OctetKey(_) => vec![
jwt::Algorithm::HS256,
jwt::Algorithm::HS384,
jwt::Algorithm::HS512,
],
// Edwards-curve algorithms
jsonwebtoken::jwk::AlgorithmParameters::OctetKeyPair(_) => vec![jwt::Algorithm::EdDSA],
}
}

fn get_claims_mapping_entry_value<T: for<'de> serde::Deserialize<'de>>(
claim_name: String,
claims_mapping_entry: JWTClaimsMappingEntry<T>,
Expand All @@ -531,14 +554,15 @@ pub(crate) async fn decode_and_parse_hasura_claims(
jwt_config: JWTConfig,
jwt: String,
) -> Result<HasuraClaims, Error> {
let (alg, decoding_key) = match jwt_config.key {
JWTKey::Fixed(conf) => (conf.algorithm, get_decoding_key(&conf)?),
let (acceptable_algorithms, decoding_key) = match jwt_config.key {
JWTKey::Fixed(conf) => (vec![conf.algorithm], get_decoding_key(&conf)?),
JWTKey::JwkFromUrl(jwk_url) => {
get_decoding_key_from_jwk_url(http_client, jwk_url, &jwt).await?
}
};

let mut validation = Validation::new(alg);
let mut validation = Validation::default();
validation.algorithms = acceptable_algorithms;

// Additional validations according to the `jwt_config`.
if let Some(aud) = jwt_config.audience {
Expand Down

0 comments on commit 97a2e09

Please sign in to comment.