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

Authentication plugin doesn't support overlapping kid values #3017

Closed
lleadbet opened this issue Apr 28, 2023 · 0 comments · Fixed by #3031
Closed

Authentication plugin doesn't support overlapping kid values #3017

lleadbet opened this issue Apr 28, 2023 · 0 comments · Fixed by #3031

Comments

@lleadbet
Copy link
Contributor

Describe the bug

Given a configuration that looks like:

# testing jwks
- url: https://test/default
   issuer: https://test_aud
# staging jwks
- url: https://staging/default
   issuer: https://staging_aud

Whereby both the staging and testing JWKS endpoints include a shared kid value (e.g. key1).

With this configuration, the auth plugin will only validate against the first in the list (in this case, the testing jwks). It may be necessary to support multiple of the same kid in cases of staging/testing environments, where there's a fallback kid for both that shares the same identifier.

To Reproduce

Given this token:
eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImtleTEifQ.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjE1MTYyMzkwMjIyLCJhdWQiOiJ0ZXN0aW5nIn0.DVPtUcLYlgEFE2OjLEJ6LQp6-Sflm9rJKHUqXCUFV8g

And these JWKS files:

Testing JWKS

{
    "keys": [{
        "alg": "HS256",
        "k": "dGVzdA==",
        "use": "sig",
        "kty": "oct",
        "kid": "key1"
    }]
}

Staging JWKS

{
    "keys": [{
        "alg": "HS256",
        "k": "dGVzdGluZw==",
        "use": "sig",
        "kty": "oct",
        "kid": "key1"
    }]
}

And lastly, this config:

authentication:
  jwt:
    jwks:
      - url: file://${env.PWD}/jwks_test.json
        aud: testing
      - url: file://${env.PWD}/jwks_staging.json
        aud: staging

You'll first see the token failing- the last entry takes precedence. Swapping the two config options results in it working.

Expected behavior
The kid is not only checked, but the right JWKS is used based on the aud, or the token is checked against all possible matches vs. the first one.

Desktop (please complete the following information):

  • OS: macOS
  • Version [e.g. 22]: 1.15.1
Geal added a commit that referenced this issue May 23, 2023
Fix #3017

in some cases, multiple keys can match what a JWT asks (algorithm and
optional kid). Previously, we scored each possible match and only took
the one with the highest score. But even then, we can have multiple keys
with the same score (example: colliding kid between multiple JWKS in
tests).
This changes the behaviour to:
- return a list of matching key instead of the one with the highest
score
- try them one by one until the JWT is validated, or return an error
- if some keys were found with the highest possible score (matching alg,
kid is present and matching too), then we only test those ones

Co-authored-by: Coenen Benjamin <benjamin.coenen@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants