Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Using the same key id (kid) in both sig and enc is problematic, breaking change since 2.12.0 #4618

Open
jarkko-rantavuori-vincit opened this issue Aug 2, 2024 · 1 comment
Labels
bug Something isn't working triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@jarkko-rantavuori-vincit
Copy link

jarkko-rantavuori-vincit commented Aug 2, 2024

What is the bug?
Given following configuration:

{
  "keys": [
    {
      "kid": "key/a",
      "kty": "RSA",
      "alg": "RSA-OAEP",
      "use": "enc",
    },
    {
      "kid": "key/a",
      "kty": "RSA",
      "alg": "RS256",
      "use": "sig",
    }
  ]
}

Before 2.11.1, the code would use the latter key for verification based on kid. Since 2.12.0, it would use the first. This seems to be due to changing of libraries, which thus introduced a breaking change for us for using JWTs (#4617). They key with use "sig" should be used in this case for JWT verification, as that by spec is used for signature verification. We use Keycloak, so for the same key it would generate same kid, so we don't get to choose the kid values and as far as I know, this should be supported configuration.

A workaround is to change key order in JWKS. That is however brittle and might break with other libraries or future updates.

How can one reproduce the bug?
Implement configuration as explained before. JWT should be signed with key/a, using RS256. Observe that since 2.12.0 it will error out with
[WARN ][o.o.s.a.BackendRegistry ] Authentication finally failed for null from (...)
and on TRACE logging level we can see

com.amazon.dlic.auth.http.jwt.keybyoidc.BadCredentialsException: Algorithm of JWT does not match algorithm of JWK (RSA-OAEP != RS256)
at com.amazon.dlic.auth.http.jwt.keybyoidc.JwtVerifier.validateSignatureAlgorithm(JwtVerifier.java:91) ~[opensearch-security-2.13.0.0.jar:2.13.0.0]

What is the expected behavior?
Configuration should work. It should take the "use" parameter into account as well, or support multiple keys with same kid. I think it should also support "alg" and "kty" parameters, based on RFC, and not just choosing based on kid, like it does now.

https://www.rfc-editor.org/rfc/rfc7517#page-5:

4.5. "kid" (Key ID) Parameter

The "kid" (key ID) parameter is used to match a specific key. This
is used, for instance, to choose among a set of keys within a JWK Set
during key rollover. The structure of the "kid" value is
unspecified. When "kid" values are used within a JWK Set, different
keys within the JWK Set SHOULD use distinct "kid" values. (One
example in which different keys might use the same "kid" value is if
they have different "kty" (key type) values but are considered to be
equivalent alternatives by the application using them.) The "kid"
value is a case-sensitive string. Use of this member is OPTIONAL.
When used with JWS or JWE, the "kid" value is used to match a JWS or
JWE "kid" Header Parameter value.

So RFC also gives an example where multiple keys with same kid is used.

See also related bug report in keycloak about "kty" parameter: keycloak/keycloak#14794

What is your host/environment?

  • Keycloak in use
  • Versions tested: 2.13, 2.12, 2.11.1

Additional context

Relevant part in the code for apache cxf, choosing the latter key when given same ids multiple times: https://github.com/apache/cxf/blob/cxf-3.5.9/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwk/JsonWebKeys.java#L69-L82
Relevant part in the code for jose-jwt, choosing the first key when given same ids multiple times when using getKeyByKeyId like opensearch-security does: https://www.javadoc.io/doc/com.nimbusds/nimbus-jose-jwt/latest/src-html/com/nimbusds/jose/jwk/JWKSet.html#line-188
Related part of code in opensearch-security
2.11.1: https://github.com/opensearch-project/security/blob/2.11.1.0/src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/SelfRefreshingKeySet.java#L146
2.12.0: https://github.com/opensearch-project/security/blob/2.12.0.0/src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/SelfRefreshingKeySet.java#L147

@jarkko-rantavuori-vincit jarkko-rantavuori-vincit added bug Something isn't working untriaged Require the attention of the repository maintainers and may need to be prioritized labels Aug 2, 2024
@cwperks cwperks added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Aug 5, 2024
@cwperks
Copy link
Member

cwperks commented Aug 5, 2024

[Triage] @jarkko-rantavuori-vincit Thanks for submitting this issue with detailed steps on how to reproduce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

No branches or pull requests

2 participants