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

Ignore keys of unknown alg when verifying JWTs with JWKS #4725

Conversation

anderseknert
Copy link
Member

Additionally, improve the JWT verification process when a JWKS
is provided:

  • If kid is present in JWT header, and exists in JWKS — verify using
    that key only.
  • If kid not present in JWT header, try verification only using keys
    matching the alg provided in the JWT header (mandatory claim).

Fixes #4699

Signed-off-by: Anders Eknert anders@eknert.com

@anderseknert anderseknert force-pushed the jwt-verification-enhancements branch 4 times, most recently from e4e7377 to 9daa6b3 Compare June 1, 2022 14:00
johanfylling
johanfylling previously approved these changes Jun 1, 2022
Copy link
Contributor

@johanfylling johanfylling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

topdown/tokens.go Outdated Show resolved Hide resolved
internal/jwx/jwa/signature.go Show resolved Hide resolved
topdown/tokens.go Show resolved Hide resolved
topdown/tokens_test.go Show resolved Hide resolved
@johanfylling
Copy link
Contributor

The ticket also has a request for improved documentation. At the very least, we should document this new behavior.

Additionally, improve the JWT verification process when a JWKS
is provided:

* If `kid` is present in JWT header, and exists in JWKS — verify using
  that key only.
* If `kid` not present in JWT header, try verification only using keys
  matching the `alg` provided in the JWT header (mandatory claim).

Fixes open-policy-agent#4699

Signed-off-by: Anders Eknert <anders@eknert.com>
@anderseknert
Copy link
Member Author

anderseknert commented Jun 1, 2022

The ticket also has a request for improved documentation. At the very least, we should document this new behavior.

If I understand the conversation between @lenalebt and @srenatus correctly, the suggested update to the docs would be to change the example not to use the raw body (i.e. string) representation of the JWKS, but to rather hack around the issue (fixed by this PR) by unmarshalling the JWKS, filter out the non-parseable key(s), and then marshall it back into a new JWKS to use for validation. The documented solution is AFAICS the correct one.

@anderseknert anderseknert merged commit cb6a4c0 into open-policy-agent:main Jun 2, 2022
@anderseknert anderseknert deleted the jwt-verification-enhancements branch June 2, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JWKS Uri containing an encryption key (besides signature keys) will fail io.jwt.decode_verify
2 participants