diff --git a/CHANGELOG.md b/CHANGELOG.md index b1cb9840..c3eadd01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ **Fixes and enhancements:** +- Enforce optional JWK `alg`, `use`, and `key_ops` metadata during JWKS selection and verification. `KeyFinder#call` now returns the JWK when `alg` is present to bind verification to that algorithm; `key_for` keeps returning raw keys. Direct JWK verification also enforces `use` and `key_ops` [#769](https://github.com/jwt/ruby-jwt/pull/769) - [@sj26](https://github.com/sj26). - Test that `JWT::Claims::Required` raises `JWT::MissingRequiredClaim` for a non-Hash payload [#768](https://github.com/jwt/ruby-jwt/pull/768) ([@renanmpimentel](https://github.com/renanmpimentel)) - Your contribution here diff --git a/README.md b/README.md index fc8cd18c..7ee6aa7c 100644 --- a/README.md +++ b/README.md @@ -667,11 +667,26 @@ To decode a JWT using a trusted entity's JSON Web Key Set (JWKS): ```ruby jwks = JWT::JWK::Set.new(jwks_hash) -jwks.filter! {|key| key[:use] == 'sig' } # Signing keys only! algorithms = jwks.map { |key| key[:alg] }.compact.uniq JWT.decode(token, nil, true, algorithms: algorithms, jwks: jwks) ``` +The `jwks` option and `JWT::JWK::KeyFinder` select a key with compatible optional metadata before verification: + +- `alg`, when present, must match the token's `alg` header exactly (case-sensitive). +- `use`, when present, must be `sig`. +- `key_ops`, when present, must contain `verify`. + +Omitted metadata does not restrict the key. Incompatible candidates are skipped; if no eligible key is found, +the existing loader invalidation is attempted before raising `JWT::SignatureError`. +Direct verification with a JWK also enforces `use` and `key_ops`, raising `JWT::VerificationKeyError` if incompatible. +The caller must still supply the allowed algorithms; JWK metadata does not replace that allowlist. +`KeyFinder#call` returns the JWK itself when `alg` is present, binding verification to that algorithm even +when the caller allows several algorithms. Without `alg`, it returns the raw verification key as before. +`KeyFinder#key_for(kid, key_field = :kid)` still returns a raw key and has no token algorithm context. +`call` resolves JWKs directly; subclasses that customize token lookup should override `call`, not `key_for`. +Applications extracting a raw key with `key_for` or `jwk.verify_key` must enforce algorithm restrictions themselves. + 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. @@ -695,9 +710,7 @@ jwks_loader = ->(options) do @cached_keys ||= begin @cache_last_update = Time.now.to_i # Replace with your own JWKS fetching routine - jwks = JWT::JWK::Set.new(jwks_hash) - jwks.select! { |key| key[:use] == 'sig' } # Signing Keys only - jwks + JWT::JWK::Set.new(jwks_hash) end end diff --git a/lib/jwt/jwa.rb b/lib/jwt/jwa.rb index 5b3654eb..b5938df9 100644 --- a/lib/jwt/jwa.rb +++ b/lib/jwt/jwa.rb @@ -64,7 +64,7 @@ def validate_jwk_algorithms!(jwks, algorithms, error_class) return if Array(jwks).all? do |jwk| algorithms.any? do |alg| - jwk.jwa.valid_alg?(alg) + jwk.jwa.valid_alg?(resolve(alg).alg) end end diff --git a/lib/jwt/jwk/key_base.rb b/lib/jwt/jwk/key_base.rb index ac9d9b91..a22258b9 100644 --- a/lib/jwt/jwk/key_base.rb +++ b/lib/jwt/jwk/key_base.rb @@ -42,7 +42,17 @@ def ==(other) other.is_a?(::JWT::JWK::KeyBase) && self[:kid] == other[:kid] end + def usable_for_verification?(algorithm = nil) + return false if algorithm && parameters.fetch(:alg, algorithm) != algorithm + return false if parameters.fetch(:use, 'sig') != 'sig' + + key_ops = parameters.fetch(:key_ops, ['verify']) + key_ops.is_a?(Array) && key_ops.include?('verify') + end + def verify(**kwargs) + raise JWT::VerificationKeyError, 'JWK metadata does not permit signature verification' unless usable_for_verification? + jwa.verify(**kwargs, verification_key: verify_key) end diff --git a/lib/jwt/jwk/key_finder.rb b/lib/jwt/jwk/key_finder.rb index 6fd18bb8..d1b489b3 100644 --- a/lib/jwt/jwk/key_finder.rb +++ b/lib/jwt/jwk/key_finder.rb @@ -27,46 +27,53 @@ def initialize(options) # Returns the verification key for the given kid # @param [String] kid the key id + # @param [Symbol] key_field the JWK field to match def key_for(kid, key_field = :kid) - raise ::JWT::MalformedTokenError, "Invalid type for #{key_field} header parameter" unless kid.nil? || kid.is_a?(String) - - jwk = resolve_key(kid, key_field) - - raise ::JWT::SignatureError, 'No keys found in jwks' unless @jwks.any? - raise ::JWT::SignatureError, "Could not find public key for kid #{kid}" unless jwk - - jwk.verify_key + find_jwk(kid, key_field).verify_key end # Returns the key for the given token # @param [JWT::EncodedToken] token the token + # @return [JWT::JWK::KeyBase, Object] the JWK when alg is present, otherwise its raw verification key def call(token) - @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 + key_field = @key_fields.find { |field| token.header[field.to_s] } - raise ::JWT::SignatureError, 'No key id (kid) or x5t found from token headers' unless @allow_nil_kid + raise ::JWT::SignatureError, 'No key id (kid) or x5t found from token headers' unless key_field || @allow_nil_kid - kid = token.header['kid'] - key_for(kid) + key_field ||= :kid + jwk = find_jwk(token.header[key_field.to_s], key_field, token.header['alg']) + jwk.parameters.key?(:alg) ? jwk : jwk.verify_key end private - def resolve_key(kid, key_field) - key_matcher = ->(key) { (kid.nil? && @allow_nil_kid) || key[key_field] == kid } + def find_jwk(kid, key_field, algorithm = nil) + raise ::JWT::MalformedTokenError, "Invalid type for #{key_field} header parameter" unless kid.nil? || kid.is_a?(String) + + jwk = resolve_key(kid, key_field, algorithm) + raise ::JWT::SignatureError, 'No keys found in jwks' unless @jwks.any? + raise ::JWT::SignatureError, "Could not find public key for kid #{kid}" unless jwk + + jwk + end + + def resolve_key(kid, key_field, algorithm) # First try without invalidation to facilitate application caching @jwks ||= JWT::JWK::Set.new(@jwks_loader.call(key_field => kid)) - jwk = @jwks.find { |key| key_matcher.call(key) } + jwk = matching_key(kid, key_field, algorithm) return jwk if jwk # Second try, invalidate for backwards compatibility @jwks = JWT::JWK::Set.new(@jwks_loader.call(invalidate: true, kid_not_found: true, key_field => kid)) - @jwks.find { |key| key_matcher.call(key) } + matching_key(kid, key_field, algorithm) + end + + def matching_key(kid, key_field, algorithm) + @jwks.find do |key| + ((kid.nil? && @allow_nil_kid) || key[key_field] == kid) && key.usable_for_verification?(algorithm) + end end end end diff --git a/spec/jwt/jwk/key_finder_spec.rb b/spec/jwt/jwk/key_finder_spec.rb new file mode 100644 index 00000000..efb208e1 --- /dev/null +++ b/spec/jwt/jwk/key_finder_spec.rb @@ -0,0 +1,168 @@ +# frozen_string_literal: true + +RSpec.describe JWT::JWK::KeyFinder do + let(:jwk) { JWT::JWK.new('a-secret-for-HMAC') } + let(:metadata) { {} } + let(:parameters) { jwk.export(include_private: true).merge(metadata) } + let(:finder) { described_class.new(jwks: { keys: [parameters] }) } + + describe '#key_for' do + it 'preserves lookup without an algorithm' do + expect(finder.key_for(jwk.kid)).to eq(jwk.verify_key) + end + + it 'preserves lookup by an explicit key field' do + parameters[:x5t] = 'thumbprint' + expect(finder.key_for('thumbprint', :x5t)).to eq(jwk.verify_key) + end + + it 'preserves raw-key lookup with algorithm metadata' do + parameters[:alg] = 'HS256' + expect(finder.key_for(jwk.kid)).to eq(jwk.verify_key) + end + + it 'checks key usage even without an algorithm' do + parameters[:use] = 'enc' + expect { finder.key_for(jwk.kid) }.to raise_error(JWT::SignatureError) + end + end + + [ + ['HS256', 'HS512', nil], + ['RS512', 'RS256', 'rsa-2048-private.pem'], + ['PS256', 'PS512', 'rsa-2048-private.pem'], + ['ES256', 'ES384', 'ec256-private.pem'] + ].each do |algorithm, other_algorithm, key_file| + context "when verifying #{algorithm}" do + before do + skip 'OpenSSL gem missing RSA-PSS support' if algorithm.start_with?('PS') && !OpenSSL::PKey::RSA.method_defined?(:sign_pss) + end + + let(:jwk) { JWT::JWK.new(key_file ? test_pkey(key_file) : 'a-secret-for-HMAC') } + let(:parameters) { jwk.export(include_private: key_file.nil?).merge(metadata) } + let(:payload) { { 'data' => 'signed payload' } } + let(:headers) { { kid: jwk.kid } } + let(:jwt) { JWT.encode(payload, jwk.signing_key, algorithm, headers) } + let(:token) { JWT::EncodedToken.new(jwt) } + let(:algorithms) { [algorithm, other_algorithm] } + + [{}, { alg: algorithm }, { use: 'sig' }, { key_ops: ['verify'] }, + { alg: algorithm, use: 'sig', key_ops: %w[sign verify] }].each do |metadata| + context "with compatible metadata #{metadata.inspect}" do + let(:metadata) { metadata } + + it 'decodes through the jwks option' do + expect(JWT.decode(jwt, nil, true, algorithms: algorithms, jwks: { keys: [parameters] }).first).to eq(payload) + end + + it 'verifies through the token key finder' do + expect(token.verify_signature!(algorithm: algorithms, key_finder: finder)).to be_nil + end + end + end + + [{ alg: other_algorithm }, { alg: algorithm.downcase }, { use: 'enc' }, { key_ops: ['encrypt'] }, + { key_ops: ['sign'] }, { key_ops: [] }, + { use: 'enc', key_ops: ['verify'] }, { use: 'sig', key_ops: ['encrypt'] }].each do |metadata| + context "with incompatible metadata #{metadata.inspect}" do + let(:metadata) { metadata } + + it 'rejects decoding through the jwks option' do + expect { JWT.decode(jwt, nil, true, algorithms: algorithms, jwks: { keys: [parameters] }) }.to raise_error(JWT::SignatureError) + end + + it 'rejects verification through the token key finder' do + expect { token.verify_signature!(algorithm: algorithms, key_finder: finder) }.to raise_error(JWT::SignatureError) + end + end + end + end + end + + context 'when looking up a token without a kid' do + let(:metadata) { { alg: 'HS512' } } + let(:finder) { described_class.new(jwks: { keys: [parameters] }, allow_nil_kid: true) } + let(:token) { JWT::EncodedToken.new(JWT.encode({}, jwk.signing_key, 'HS256')) } + + it 'still enforces the JWK algorithm' do + expect { finder.call(token) }.to raise_error(JWT::SignatureError) + end + end + + context 'when looking up JSON keys by x5t' do + let(:metadata) { { alg: 'HS512', x5t: 'thumbprint' } } + let(:finder) { described_class.new(jwks: JSON.parse(JSON.generate(keys: [parameters])), key_fields: [:x5t]) } + let(:token) { JWT::EncodedToken.new(JWT.encode({}, jwk.signing_key, 'HS256', x5t: 'thumbprint')) } + + it 'still enforces the JWK algorithm' do + expect { finder.call(token) }.to raise_error(JWT::SignatureError) + end + end + + context 'when the signature uses a different allowed algorithm from the metadata' do + let(:metadata) { { alg: 'HS512' } } + let(:jwt) do + header = JWT::Base64.url_encode(JSON.generate(alg: 'HS512', kid: jwk.kid)) + payload = JWT::Base64.url_encode(JSON.generate(data: 'example')) + input = [header, payload].join('.') + signature = OpenSSL::HMAC.digest('SHA256', jwk.signing_key, input) + [input, JWT::Base64.url_encode(signature)].join('.') + end + + it 'rejects the signature despite matching JWK and token alg headers' do + token = JWT::EncodedToken.new(jwt) + expect { token.verify_signature!(algorithm: %w[HS256 HS512], key_finder: finder) }.to raise_error(JWT::VerificationError) + end + + it 'also rejects decoding through the jwks option' do + expect { JWT.decode(jwt, nil, true, algorithms: %w[HS256 HS512], jwks: { keys: [parameters] }) }.to raise_error(JWT::VerificationError) + end + end + + context 'when several keys match the identifier' do + let(:metadata) { { alg: 'HS256', use: 'sig', key_ops: ['verify'] } } + let(:jwt) { JWT.encode({ 'data' => 'example' }, jwk.signing_key, 'HS256', kid: jwk.kid) } + let(:finder) { described_class.new(jwks: { keys: [incompatible_key, parameters] }) } + + [{ use: 'enc' }, { key_ops: ['sign'] }, { alg: 'HS512' }].each do |restrictions| + context "with an incompatible first key #{restrictions.inspect}" do + let(:incompatible_key) { JWT::JWK.new('a-different-secret', kid: jwk.kid).export(include_private: true).merge(restrictions) } + + it 'verifies with the compatible candidate' do + expect(JWT::EncodedToken.new(jwt).verify_signature!(algorithm: %w[HS256 HS512], key_finder: finder)).to be_nil + expect(JWT.decode(jwt, nil, true, algorithms: %w[HS256 HS512], jwks: { keys: [incompatible_key, parameters] }).first).to eq('data' => 'example') + end + end + end + + context 'without a kid' do + let(:incompatible_key) { JWT::JWK.new('a-different-secret', use: 'enc').export(include_private: true) } + let(:jwt) { JWT.encode({ 'data' => 'example' }, jwk.signing_key, 'HS256') } + + it 'skips incompatible candidates with allow_nil_kid' do + payload, = JWT.decode(jwt, nil, true, algorithms: ['HS256'], jwks: { keys: [incompatible_key, parameters] }, allow_nil_kid: true) + expect(payload).to eq('data' => 'example') + end + end + end + + context 'when cached metadata does not permit verification' do + let(:metadata) { { use: 'sig' } } + + it 'uses the existing loader invalidation path to find an eligible key' do + loader = ->(options) { { keys: [options[:invalidate] ? parameters : parameters.merge(use: 'enc')] } } + jwt = JWT.encode({ 'data' => 'example' }, jwk.signing_key, 'HS256', kid: jwk.kid) + expect(JWT.decode(jwt, nil, true, algorithms: ['HS256'], jwks: loader).first).to eq('data' => 'example') + end + end + + context 'when a restricted JWK is used directly' do + [{ use: 'enc' }, { key_ops: ['sign'] }].each do |restrictions| + it "rejects #{restrictions.inspect} without a key finder" do + key = JWT::JWK.new(jwk.signing_key, restrictions.merge(alg: 'HS256')) + token = JWT::EncodedToken.new(JWT.encode({}, jwk.signing_key, 'HS256')) + expect { token.verify_signature!(algorithm: ['HS256'], key: key) }.to raise_error(JWT::VerificationKeyError) + end + end + end +end