Skip to content

Add support for x5t and x5t#S256 header #669

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Take a look at the [upgrade guide](UPGRADING.md) for more details.
- JWT::EncodedToken#verify! method that bundles signature and claim validation [#647](https://github.com/jwt/ruby-jwt/pull/647) ([@anakinj](https://github.com/anakinj))
- Do not override the alg header if already given [#659](https://github.com/jwt/ruby-jwt/pull/659) ([@anakinj](https://github.com/anakinj))
- Make `JWK::KeyFinder` compatible with `JWT::EncodedToken` [#663](https://github.com/jwt/ruby-jwt/pull/663) ([@anakinj](https://github.com/anakinj))
- Add support for x5t header parameter for X.509 certificate thumbprint verification [#669](https://github.com/jwt/ruby-jwt/pull/669) ([@hieuk09](https://github.com/hieuk09))
- Your contribution here

**Fixes and enhancements:**
Expand Down
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -674,13 +674,14 @@ algorithms = jwks.map { |key| key[:alg] }.compact.uniq
JWT.decode(token, nil, true, algorithms: algorithms, jwks: jwks)
```

The `jwks` option can also be given as a lambda that evaluates every time a kid is resolved.
The `jwks` option can also be given as a lambda that evaluates every time a key identifier is resolved.
This can be used to implement caching of remotely fetched JWK Sets.

If the requested `kid` is not found from the given set the loader will be called a second time with the `kid_not_found` option set to `true`.
Key identifiers can be specified using `kid`, `x5t` or `x5c` header parameters.
If the requested identifier is not found from the given set the loader will be called a second time with the `kid_not_found` option set to `true`.
The application can choose to implement some kind of JWK cache invalidation or other mechanism to handle such cases.

Tokens without a specified `kid` are rejected by default.
Tokens without a specified key identifier (`kid`, `x5t` or `x5c`) are rejected by default.
This behaviour may be overwritten by setting the `allow_nil_kid` option for `decode` to `true`.

```ruby
Expand Down
6 changes: 6 additions & 0 deletions lib/jwt/base64.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ def url_encode(str)
::Base64.urlsafe_encode64(str, padding: false)
end

# Encode a string with Base64 complying with RFC 4648 (padded).
# @api private
def strict_encode(str)
::Base64.strict_encode64(str)
end

# Decode a string with URL-safe Base64 complying with RFC 4648.
# @api private
def url_decode(str)
Expand Down
9 changes: 8 additions & 1 deletion lib/jwt/decode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,14 @@ def verify_algo

def set_key
@key = find_key(&@keyfinder) if @keyfinder
@key = ::JWT::JWK::KeyFinder.new(jwks: @options[:jwks], allow_nil_kid: @options[:allow_nil_kid]).key_for(token.header['kid']) if @options[:jwks]
if @options[:jwks]
@key = ::JWT::JWK::KeyFinder.new(
jwks: @options[:jwks],
allow_nil_kid: @options[:allow_nil_kid],
key_fields: @options[:key_fields]
).call(token)
end

return unless (x5c_options = @options[:x5c])

@key = X5cKeyFinder.new(x5c_options[:root_certificates], x5c_options[:crls]).from(token.header['x5c'])
Expand Down
31 changes: 22 additions & 9 deletions lib/jwt/jwk/key_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ class KeyFinder
# @param [Hash] options the options to create a KeyFinder with
# @option options [Proc, JWT::JWK::Set] :jwks the jwks or a loader proc
# @option options [Boolean] :allow_nil_kid whether to allow nil kid
# @option options [Array] :key_fields the fields to use for key matching,
# the order of the fields are used to determine
# the priority of the keys.
def initialize(options)
@allow_nil_kid = options[:allow_nil_kid]
jwks_or_loader = options[:jwks]
Expand All @@ -18,15 +21,16 @@ def initialize(options)
else
->(_options) { jwks_or_loader }
end

@key_fields = options[:key_fields] || %i[kid]
end

# Returns the verification key for the given kid
# @param [String] kid the key id
def key_for(kid)
raise ::JWT::DecodeError, 'No key id (kid) found from token headers' unless kid || @allow_nil_kid
raise ::JWT::DecodeError, 'Invalid type for kid header parameter' unless kid.nil? || kid.is_a?(String)
def key_for(kid, key_field = :kid)
raise ::JWT::DecodeError, "Invalid type for #{key_field} header parameter" unless kid.nil? || kid.is_a?(String)

jwk = resolve_key(kid)
jwk = resolve_key(kid, key_field)

raise ::JWT::DecodeError, 'No keys found in jwks' unless @jwks.any?
raise ::JWT::DecodeError, "Could not find public key for kid #{kid}" unless jwk
Expand All @@ -37,22 +41,31 @@ def key_for(kid)
# Returns the key for the given token
# @param [JWT::EncodedToken] token the token
def call(token)
key_for(token.header['kid'])
@key_fields.each do |key_field|
field_value = token.header[key_field.to_s]

return key_for(field_value, key_field) if field_value
end

raise ::JWT::DecodeError, 'No key id (kid) or x5t found from token headers' unless @allow_nil_kid

kid = token.header['kid']
key_for(kid)
end

private

def resolve_key(kid)
key_matcher = ->(key) { (kid.nil? && @allow_nil_kid) || key[:kid] == kid }
def resolve_key(kid, key_field)
key_matcher = ->(key) { (kid.nil? && @allow_nil_kid) || key[key_field] == kid }

# First try without invalidation to facilitate application caching
@jwks ||= JWT::JWK::Set.new(@jwks_loader.call(kid: kid))
@jwks ||= JWT::JWK::Set.new(@jwks_loader.call(key_field => kid))
Copy link
Member

Choose a reason for hiding this comment

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

key_field should probably be set to :kid at before this is called if @allow_nil_kid is true

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it's too late to drop the @allow_nil_kid option for the 3.0 version... 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Would simplify this complicated piece of machinery....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the approach in #669 (comment) could address the above problem, too 🤔

jwk = @jwks.find { |key| key_matcher.call(key) }

return jwk if jwk

# Second try, invalidate for backwards compatibility
@jwks = JWT::JWK::Set.new(@jwks_loader.call(invalidate: true, kid_not_found: true, kid: kid))
@jwks = JWT::JWK::Set.new(@jwks_loader.call(invalidate: true, kid_not_found: true, key_field => kid))
Copy link
Member

Choose a reason for hiding this comment

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

This could cause some problems as the keyloader could always be expecting the kid. Also do we will start invalidating 3 times if nothing is found.

Copy link
Member

Choose a reason for hiding this comment

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

Also the line above where we do call with the key_field could cause backwards compatibility issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could cause some problems as the keyloader could always be expecting the kid

I think it's fine because we only call the JWKS loader with other key fields if token header does not contain kid. I think most of the JWKS endpoints support kid by default.

Also do we will start invalidating 3 times if nothing is found.

No, we only invalidating once. Basically, if kid field exists in token header, we only match using kid. Otherwise, if x5t exists and there is no kid, we'll match using x5t.

Also the line above where we do call with the key_field could cause backwards compatibility issues.

Could you clarify on what could be the compatibility issues? I change key_for signature but I added key_field as optional parameter so it should be backward-compatible. I may miss other scenarios, so please let me know.

Copy link
Member

Choose a reason for hiding this comment

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

You're right! It happens only once. Now the gem is dictating the lookup order, so if the key contains both kid and x5 headers we'll never try the x5 lookups.

I feel this key lookup is already so complicated, cant wrap my head around this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, the gem is dictating the lookup order. Currently I couldn't think of a better way to do this. Do you have any suggestions?

Copy link
Member

@anakinj anakinj Apr 30, 2025

Choose a reason for hiding this comment

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

For use via the JWT.decode method the predefined order is fine i think.

What about having the order as a option when initializing the keyfinder (with the current order as the default).

Then the new token object approach could be easily customized.

key_finder = JWT::JWK::KeyFinder.new(jwks: JWT::JWK::Set.new(jwk), key_fields: [:kid, :x5t])
encoded_token = JWT::EncodedToken.new(token.jwt)
encoded_token.verify!(signature: { algorithm: 'RS256', key_finder: key_finder})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's a good idea. Let me work on that.

@jwks.find { |key| key_matcher.call(key) }
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/jwt/jwk/rsa.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def verify_key
def export(options = {})
exported = parameters.clone
exported.reject! { |k, _| RSA_PRIVATE_KEY_ELEMENTS.include? k } unless private? && options[:include_private] == true

exported
end

Expand All @@ -67,7 +68,7 @@ def key_digest
def []=(key, value)
raise ArgumentError, 'cannot overwrite cryptographic key attributes' if RSA_KEY_ELEMENTS.include?(key.to_sym)

super(key, value)
super
end

private
Expand Down
25 changes: 23 additions & 2 deletions spec/jwt/jwk/decode_with_jwk_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
describe '.decode for JWK usecase' do
let(:keypair) { test_pkey('rsa-2048-private.pem') }
let(:jwk) { JWT::JWK.new(keypair) }
let(:public_jwks) { { keys: [jwk.export, { kid: 'not_the_correct_one', kty: 'oct', k: 'secret' }] } }
let(:valid_key) { jwk.export }
let(:public_jwks) { { keys: [valid_key, { kid: 'not_the_correct_one', kty: 'oct', k: 'secret' }] } }
let(:token_payload) { { 'data' => 'something' } }
let(:token_headers) { { kid: jwk.kid } }
let(:algorithm) { 'RS512' }
Expand Down Expand Up @@ -38,6 +39,26 @@
end
end

context 'and x5t is in the set' do
let(:x5t) { Base64.urlsafe_encode64(OpenSSL::Digest::SHA1.new(keypair.to_der).digest, padding: false) }
let(:valid_key) { jwk.export.merge({ x5t: x5t }) }
let(:token_headers) { { x5t: x5t } }
it 'is able to decode the token' do
payload, _header = described_class.decode(signed_token, nil, true, { algorithms: [algorithm], jwks: public_jwks, key_fields: [:x5t] })
expect(payload).to eq(token_payload)
end
end

context 'and both kid and x5t is in the set' do
let(:x5t) { Base64.urlsafe_encode64(OpenSSL::Digest::SHA1.new(keypair.to_der).digest, padding: false) }
let(:valid_key) { jwk.export.merge({ x5t: x5t }) }
let(:token_headers) { { x5t: x5t, kid: 'NOT_A_MATCH' } }
it 'is able to decode the token based on the priority of the key defined in key_fields' do
payload, _header = described_class.decode(signed_token, nil, true, { algorithms: [algorithm], jwks: public_jwks, key_fields: %i[x5t kid] })
expect(payload).to eq(token_payload)
end
end

context 'no keys are found in the set' do
let(:public_jwks) { { keys: [] } }
it 'raises an exception' do
Expand All @@ -51,7 +72,7 @@
let(:token_headers) { {} }
it 'raises an exception' do
expect { described_class.decode(signed_token, nil, true, { algorithms: [algorithm], jwks: public_jwks }) }.to raise_error(
JWT::DecodeError, 'No key id (kid) found from token headers'
JWT::DecodeError, 'No key id (kid) or x5t found from token headers'
)
end
end
Expand Down
Loading