ory / hydra

The most scalable and customizable OpenID Certified™ OpenID Connect and OAuth Provider on the market. Become an OpenID Connect and OAuth2 Provider over night. Broad support for related RFCs. Written in Go, cloud native, headless, API-first. Available as a service on Ory Network and for self-hosters.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=hydra
Apache License 2.0
15.65k stars 1.5k forks source link

Calling end_session_endpoint with id_token_hint errors when JWK is rotated #3719

Open dlpetrie opened 9 months ago

dlpetrie commented 9 months ago

Preflight checklist

Ory Network Project

No response

Describe the bug

After creating/rotating a new JWK, when existing sessions call the end_session_endpoint with the id_token_hint Hydra is redirecting to the error page with the following error:

error: invalid_request
error_description: The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. go-jose/go-jose: error in cryptographic primitive

Reproducing the bug

  1. Successfully login and retrieve an access_token and id_token from Hydra.
  2. Create a new JWK by making a POST call to /admin/keys/hydra.openid.id-token
  3. Logout from the application being redirected to the end_session_endpoint with id_token_hint populated.
  4. Hydra redirects to the error page with error:
    The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. go-jose/go-jose: error in cryptographic primitive

Relevant log output

time=2024-02-15T23:03:15Z level=error msg=An error occurred audience=application error=map[debug: message:invalid_request reason:go-jose/go-jose: error in cryptographic primitive status:Bad Request status_code:400]

Relevant configuration

No response

Version

2.2.0-pre.1

On which operating system are you observing this issue?

None

In which environment are you deploying?

None

Additional Context

Digging into the code this is the trail I found:

  1. Logout Handler calls: h.r.ConsentStrategy().HandleOpenIDConnectLogout(...): https://github.com/ory/hydra/blob/0421fdad0b30b24a1aa5939e69c20e9d0d8b649f/oauth2/handler.go#L129
  2. calls s.issueLogoutVerifier(...) because no logout_verifier was passed: https://github.com/ory/hydra/blob/0421fdad0b30b24a1aa5939e69c20e9d0d8b649f/consent/strategy_default.go#L1091
  3. calls s.getIDTokenHintClaims(...) with id token: https://github.com/ory/hydra/blob/0421fdad0b30b24a1aa5939e69c20e9d0d8b649f/consent/strategy_default.go#L878
  4. calls s.r.OpenIDJWTStrategy().Decode(ctx, idTokenHint) using jwk.NewDefaultJWTSigner(): https://github.com/ory/hydra/blob/0421fdad0b30b24a1aa5939e69c20e9d0d8b649f/consent/strategy_default.go#L173
  5. NewDefaultJWTSigner sets a GetPrivateKey func to essentially: https://github.com/ory/hydra/blob/0421fdad0b30b24a1aa5939e69c20e9d0d8b649f/jwk/jwt_strategy.go#L42-L57
  6. GetOrGenerateKeys(...) called by the GetPrivateKey func and calls FindPrivateKey(...): https://github.com/ory/hydra/blob/0421fdad0b30b24a1aa5939e69c20e9d0d8b649f/jwk/helper.go#L46-L78
  7. FindPrivateKeys(...) removes public keys and returns the First key in the set: https://github.com/ory/hydra/blob/0421fdad0b30b24a1aa5939e69c20e9d0d8b649f/jwk/helper.go#L96-L103

This is where the issue lies, because the active id_token is no longer the first token anymore since the JWK was rotated. It seems the id_tokens KID should be pulled out along the way at some point and passed in so the proper key is found instead of the first one found.

dlpetrie commented 9 months ago

Hello. Is there any confirmation that this is indeed a bug? I can potentially find time to look further into this if confirmation is given.

Thank you

alnr commented 8 months ago

Yes, this does look like a bug. Instead of only using the first key, we should attempt verification with previous keys as well.

PRs are welcome.