kid value in headers in different format after upgrading from 1.8.3 to 1.8.5

[jessieay]

This may not be an issue for everyone but I wanted to post an issue warning to others who are upgrading or might be facing an issue with the upgrade as this upgrade (1.8.3 -> 1.8.5) caused an authentication incident for my team today.

Source of the problem

The root of the issue is this pull request: #177

This was called out as a potential issue in the PR itself by @nbulaj .

Specifically, this line of code.

The OpenID kid is an opaque value that is supposed to identify public keys. json-jwt and ruby-jwt implement the kid slightly differently:

kid format for json-jwt (used by 1.8.3)

kid format for ruby-jwt (used by 1.8.5)

Normally doorkeeper-openid_connect manages both the generation of an OpenID token and the endpoint that allows clients to discover the public keys.

In our application, however, we override Doorkeeper::OpenidConnect::DiscoveryController and for the JWK output in our controller we use call to_jwk, which uses json-jwt NOT ruby-jwt

Because our override of Doorkeeper::OpenidConnect::DiscoveryController assumes that the kid format will be the format used by json-jwt, an upgrade to doorkeeper-openid_connect1.8.5 resulted in an incident for us because any service that validates the signature of a JWT using the kid header value against the kid in our keys discovery endpoint will raise an error (because the kid s do not match).

What I actually see

Screenshot of a (test) decoded JWT generated with 1.8.5:

What I expect to see

The kid should match the kid value of my JWK#kid in my discovery keys endpoint ("OcGcrSBrUtT8mNNNkm1eofXGmOgM4SQtqFgFT8BCszU")
Screenshot of a (test) decoded JWT generated with 1.8.3:

[stanhu]

The issue here is that with the switch to #177, the jwt gem defaults to a key generator of JWT::JWK::KidAsKeyDigest instead of JWT::JWK::Thumbprint, which implements RFC 7638: https://www.rfc-editor.org/rfc/rfc7638.

It's not obvious that applications need to set the generator to make the kid values backwards compatible with json-jwt:

JWT.configuration.jwk.kid_generator_type = :rfc7638_thumbprint

This broke our application because we overrode Doorkeeper::OpenidConnect::DiscoveryController to export additional keys with RFC 7638 format via json-jwt, but Doorkeeper::OpenidConnect::IdToken uses a kid in the JWT::JWK::KidAsKeyDigest format. Clients would fail because the kid listed in the JWKS URL did not match the one provided in the token.

There is a json-jwt discussion to make RFC 7638 the default in a future version.

I think v1.8.x of this gem should default to the RFC7638 behavior:

diff --git a/lib/doorkeeper/openid_connect.rb b/lib/doorkeeper/openid_connect.rb
index b2bca97..d20642e 100644
--- a/lib/doorkeeper/openid_connect.rb
+++ b/lib/doorkeeper/openid_connect.rb
@@ -48,7 +48,7 @@ module Doorkeeper
         else
           OpenSSL::PKey.read(configuration.signing_key)
         end
-      ::JWT::JWK.new(key)
+      ::JWT::JWK.new(key, { kid_generator: JWT::JWK::Thumbprint })
     end
 
     def self.signing_key_normalized

I'm not sure if this needs to be customized. What do you think @nbulaj @kristof-mattei?

[stanhu]

#194 does this.