Calling end_session_endpoint with id_token_hint errors when JWK is rotated

[dlpetrie]

Preflight checklist

• I could not find a solution in the existing issues, docs, nor discussions.
• I agree to follow this project's Code of Conduct.
• I have read and am following this repository's Contribution Guidelines.
• I have joined the Ory Community Slack.
• I am signed up to the Ory Security Patch Newsletter.

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(...):

   hydra/oauth2/handler.go

   Line 129 in 0421fda

   handled, err := h.r.ConsentStrategy().HandleOpenIDConnectLogout(ctx, w, r)

2. calls s.issueLogoutVerifier(...) because no logout_verifier was passed:

   hydra/consent/strategy_default.go

   Line 1091 in 0421fda

   return s.issueLogoutVerifier(ctx, w, r)

3. calls s.getIDTokenHintClaims(...) with id token:

   hydra/consent/strategy_default.go

   Line 878 in 0421fda

   claims, err := s.getIDTokenHintClaims(r.Context(), hint)

4. calls s.r.OpenIDJWTStrategy().Decode(ctx, idTokenHint) using jwk.NewDefaultJWTSigner():

   hydra/consent/strategy_default.go

   Line 173 in 0421fda

   token, err := s.r.OpenIDJWTStrategy().Decode(ctx, idTokenHint)

5. NewDefaultJWTSigner sets a GetPrivateKey func to essentially:

   hydra/jwk/jwt_strategy.go

   Lines 42 to 57 in 0421fda

   	func (j *DefaultJWTSigner) getKeys(ctx context.Context) (private *jose.JSONWebKey, err error) {
   	private, err = GetOrGenerateKeys(ctx, j.r, j.r.KeyManager(), j.setID, uuid.Must(uuid.NewV4()).String(), string(jose.RS256))
   	if err == nil {
   	return private, nil
   	}
   	var netError net.Error
   	if errors.As(err, &netError) {
   	return nil, errors.WithStack(fosite.ErrServerError.
   	WithHintf(`Could not ensure that signing keys for "%s" exists. A network error occurred, see error for specific details.`, j.setID))
   	}
   	return nil, errors.WithStack(fosite.ErrServerError.
   	WithWrap(err).
   	WithHintf(`Could not ensure that signing keys for "%s" exists. If you are running against a persistent SQL database this is most likely because your "secrets.system" ("SECRETS_SYSTEM" environment variable) is not set or changed. When running with an SQL database backend you need to make sure that the secret is set and stays the same, unless when doing key rotation. This may also happen when you forget to run "hydra migrate sql..`, j.setID))
   	}

6. GetOrGenerateKeys(...) called by the GetPrivateKey func and calls FindPrivateKey(...):

   hydra/jwk/helper.go

   Lines 46 to 78 in 0421fda

   	func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set, kid, alg string) (private *jose.JSONWebKey, err error) {
   	getLock(set).Lock()
   	defer getLock(set).Unlock()
   	keys, err := m.GetKeySet(ctx, set)
   	if errors.Is(err, x.ErrNotFound) || keys != nil && len(keys.Keys) == 0 {
   	r.Logger().Warnf("JSON Web Key Set \"%s\" does not exist yet, generating new key pair...", set)
   	keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig")
   	if err != nil {
   	return nil, err
   	}
   	} else if err != nil {
   	return nil, err
   	}
   	privKey, privKeyErr := FindPrivateKey(keys)
   	if privKeyErr == nil {
   	return privKey, nil
   	} else {
   	r.Logger().WithField("jwks", set).Warnf("JSON Web Key not found in JSON Web Key Set %s, generating new key pair...", set)
   	keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig")
   	if err != nil {
   	return nil, err
   	}
   	privKey, err := FindPrivateKey(keys)
   	if err != nil {
   	return nil, err
   	}
   	return privKey, nil
   	}
   	}

7. FindPrivateKeys(...) removes public keys and returns the First key in the set:

   hydra/jwk/helper.go

   Lines 96 to 103 in 0421fda

   	func FindPrivateKey(set *jose.JSONWebKeySet) (key *jose.JSONWebKey, err error) {
   	keys := ExcludePublicKeys(set)
   	if len(keys.Keys) == 0 {
   	return nil, errors.New("key not found")
   	}
   	return First(keys.Keys), nil
   	}

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]

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]

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.