From 6b56868e6e6e6745fc2a888652dcb4ad22d40852 Mon Sep 17 00:00:00 2001 From: Joakim Antman Date: Tue, 24 Jun 2025 20:18:48 +0300 Subject: [PATCH] Do not automatically trust the alg in the JWK --- CHANGELOG.md | 1 + README.md | 2 +- lib/jwt/encoded_token.rb | 2 +- lib/jwt/jwa.rb | 31 ++++++++++++++++++++---- lib/jwt/jwk/key_base.rb | 11 +++------ lib/jwt/token.rb | 4 +-- spec/integration/readme_examples_spec.rb | 4 +-- spec/jwt/encoded_token_spec.rb | 26 +++++++++++++++++--- spec/jwt/jwk/ec_spec.rb | 21 ++++++++++++++++ spec/jwt/jwk/rsa_spec.rb | 16 +++++++++++- spec/jwt/token_spec.rb | 18 ++++++++++++-- 11 files changed, 111 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ed0aea07..002415a01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ **Fixes and enhancements:** +- Require the algorithm to be provided when signing and verifying tokens using JWKs [#695](https://github.com/jwt/ruby-jwt/pull/695) ([@anakinj](https://github.com/anakinj)) - Your contribution here ## [v3.1.0](https://github.com/jwt/ruby-jwt/tree/v3.1.0) (2025-06-23) diff --git a/README.md b/README.md index 8b41157d8..9a2673cf9 100644 --- a/README.md +++ b/README.md @@ -268,7 +268,7 @@ token = JWT::Token.new(payload: payload, header: header) token.sign!(key: jwk) encoded_token = JWT::EncodedToken.new(token.jwt) -encoded_token.verify!(signature: { key: jwk}) +encoded_token.verify!(signature: { algorithm: ["HS256", "HS512"], key: jwk}) ``` #### Using a keyfinder diff --git a/lib/jwt/encoded_token.rb b/lib/jwt/encoded_token.rb index 3e94edf12..cbaec1c8d 100644 --- a/lib/jwt/encoded_token.rb +++ b/lib/jwt/encoded_token.rb @@ -138,7 +138,7 @@ def valid?(signature:, claims: nil) # @return [nil] # @raise [JWT::VerificationError] if the signature verification fails. # @raise [ArgumentError] if neither key nor key_finder is provided, or if both are provided. - def verify_signature!(algorithm: nil, key: nil, key_finder: nil) + def verify_signature!(algorithm:, key: nil, key_finder: nil) return if valid_signature?(algorithm: algorithm, key: key, key_finder: key_finder) raise JWT::VerificationError, 'Signature verification failed' diff --git a/lib/jwt/jwa.rb b/lib/jwt/jwa.rb index d5b0de6ab..11f23c4b3 100644 --- a/lib/jwt/jwa.rb +++ b/lib/jwt/jwa.rb @@ -15,6 +15,8 @@ module JWT module JWA # @api private class VerifierContext + attr_reader :jwa + def initialize(jwa:, keys:) @jwa = jwa @keys = Array(keys) @@ -29,6 +31,8 @@ def verify(*args, **kwargs) # @api private class SignerContext + attr_reader :jwa + def initialize(jwa:, key:) @jwa = jwa @key = key @@ -37,10 +41,6 @@ def initialize(jwa:, key:) def sign(*args, **kwargs) @jwa.sign(*args, **kwargs, signing_key: @key) end - - def jwa_header - @jwa.header - end end class << self @@ -64,7 +64,11 @@ def resolve_and_sort(algorithms:, preferred_algorithm:) # @api private def create_signer(algorithm:, key:) - return key if key.is_a?(JWK::KeyBase) + if key.is_a?(JWK::KeyBase) + validate_jwk_algorithms!(key, algorithm, DecodeError) + + return key + end SignerContext.new(jwa: resolve(algorithm), key: key) end @@ -73,10 +77,27 @@ def create_signer(algorithm:, key:) def create_verifiers(algorithms:, keys:, preferred_algorithm:) jwks, other_keys = keys.partition { |key| key.is_a?(JWK::KeyBase) } + validate_jwk_algorithms!(jwks, algorithms, VerificationError) + jwks + resolve_and_sort(algorithms: algorithms, preferred_algorithm: preferred_algorithm) .map { |jwa| VerifierContext.new(jwa: jwa, keys: other_keys) } end + + # @api private + def validate_jwk_algorithms!(jwks, algorithms, error_class) + algorithms = Array(algorithms) + + return if algorithms.empty? + + return if Array(jwks).all? do |jwk| + algorithms.any? do |alg| + jwk.jwa.valid_alg?(alg) + end + end + + raise error_class, "Provided JWKs do not support one of the specified algorithms: #{algorithms.join(', ')}" + end end end end diff --git a/lib/jwt/jwk/key_base.rb b/lib/jwt/jwk/key_base.rb index 84c46f8fb..ac9d9b911 100644 --- a/lib/jwt/jwk/key_base.rb +++ b/lib/jwt/jwk/key_base.rb @@ -50,11 +50,6 @@ def sign(**kwargs) jwa.sign(**kwargs, signing_key: signing_key) end - # @api private - def jwa_header - jwa.header - end - alias eql? == def <=>(other) @@ -63,12 +58,12 @@ def <=>(other) self[:kid] <=> other[:kid] end - private - def jwa raise JWT::JWKError, 'Could not resolve the JWA, the "alg" parameter is missing' unless self[:alg] - JWA.resolve(self[:alg]) + JWA.resolve(self[:alg]).tap do |jwa| + raise JWT::JWKError, 'none algorithm usage not supported via JWK' if jwa.is_a?(JWA::None) + end end attr_reader :parameters diff --git a/lib/jwt/token.rb b/lib/jwt/token.rb index 8552e5317..0c643886f 100644 --- a/lib/jwt/token.rb +++ b/lib/jwt/token.rb @@ -91,11 +91,11 @@ def detach_payload! # @param algorithm [String, Object] the algorithm to use for signing. # @return [void] # @raise [JWT::EncodeError] if the token is already signed or other problems when signing - def sign!(key:, algorithm: nil) + def sign!(key:, algorithm:) raise ::JWT::EncodeError, 'Token already signed' if @signature JWA.create_signer(algorithm: algorithm, key: key).tap do |signer| - header.merge!(signer.jwa_header) { |_key, old, _new| old } + header.merge!(signer.jwa.header) { |_key, old, _new| old } @signature = signer.sign(data: signing_input) end diff --git a/spec/integration/readme_examples_spec.rb b/spec/integration/readme_examples_spec.rb index a1fd7c33d..ea9f779c5 100644 --- a/spec/integration/readme_examples_spec.rb +++ b/spec/integration/readme_examples_spec.rb @@ -472,10 +472,10 @@ def self.verify(data:, signature:, verification_key:) jwk = JWT::JWK.import(JSON.parse(jwk_json)) token = JWT::Token.new(payload: payload, header: header) - token.sign!(key: jwk) + token.sign!(key: jwk, algorithm: 'HS256') encoded_token = JWT::EncodedToken.new(token.jwt) - expect { encoded_token.verify!(signature: { key: jwk }) }.not_to raise_error + expect { encoded_token.verify!(signature: { algorithm: %w[HS256 HS512], key: jwk }) }.not_to raise_error end end end diff --git a/spec/jwt/encoded_token_spec.rb b/spec/jwt/encoded_token_spec.rb index e239c7857..2028a0feb 100644 --- a/spec/jwt/encoded_token_spec.rb +++ b/spec/jwt/encoded_token_spec.rb @@ -139,9 +139,15 @@ end end + context 'when algorithm is an empty array' do + it 'raises an error' do + expect { token.verify_signature!(key: 'secret', algorithm: []) }.to raise_error(JWT::VerificationError, 'No algorithm provided') + end + end + context 'when algorithm is not given' do it 'raises an error' do - expect { token.verify_signature!(key: 'secret') }.to raise_error(JWT::VerificationError, 'No algorithm provided') + expect { token.verify_signature!(key: 'secret') }.to raise_error(ArgumentError, /missing keyword/) end end @@ -226,8 +232,22 @@ .jwt end - it 'uses the JWK for verification' do - expect(token.verify_signature!(key: jwk)).to eq(nil) + context 'with empty algorithm array provided' do + it 'uses the JWK for verification' do + expect(token.verify_signature!(key: jwk, algorithm: [])).to eq(nil) + end + end + + context 'with algorithms supported by key provided' do + it 'uses the JWK for verification' do + expect(token.verify_signature!(algorithm: %w[RS256 RS512], key: jwk)).to eq(nil) + end + end + + context 'with algorithms not supported by key provided' do + it 'raises JWT::VerificationError' do + expect { token.verify_signature!(algorithm: %w[RS384 RS512], key: jwk) }.to raise_error(JWT::VerificationError, 'Provided JWKs do not support one of the specified algorithms: RS384, RS512') + end end end end diff --git a/spec/jwt/jwk/ec_spec.rb b/spec/jwt/jwk/ec_spec.rb index 88ef9b762..86349ef87 100644 --- a/spec/jwt/jwk/ec_spec.rb +++ b/spec/jwt/jwk/ec_spec.rb @@ -139,6 +139,27 @@ end end end + + context 'when the jwk has an invalid alg header' do + let(:rsa) { described_class.new(ec_key, alg: 'INVALID') } + it 'raises JWT::VerificationError' do + expect { rsa.verify(data: data, signature: 'signature') }.to raise_error(JWT::VerificationError, 'Algorithm not supported') + end + end + + context 'when the jwk has none as the alg parameter' do + let(:rsa) { described_class.new(ec_key, alg: 'none') } + it 'raises JWT::JWKError' do + expect { rsa.verify(data: data, signature: 'signature') }.to raise_error(JWT::JWKError, 'none algorithm usage not supported via JWK') + end + end + + context 'when the jwk has HS256 as the alg parameter' do + let(:rsa) { described_class.new(ec_key, alg: 'HS256') } + it 'raises JWT::DecodeError' do + expect { rsa.verify(data: data, signature: 'signature') }.to raise_error(JWT::DecodeError, 'HMAC key expected to be a String') + end + end end describe '.to_openssl_curve' do diff --git a/spec/jwt/jwk/rsa_spec.rb b/spec/jwt/jwk/rsa_spec.rb index 78457454e..1b283fb6a 100644 --- a/spec/jwt/jwk/rsa_spec.rb +++ b/spec/jwt/jwk/rsa_spec.rb @@ -119,10 +119,24 @@ context 'when the jwk has an invalid alg header' do let(:rsa) { described_class.new(rsa_key, alg: 'INVALID') } - it 'raises JWT::JWKError' do + it 'raises JWT::VerificationError' do expect { rsa.verify(data: data, signature: 'signature') }.to raise_error(JWT::VerificationError, 'Algorithm not supported') end end + + context 'when the jwk has none as the alg parameter' do + let(:rsa) { described_class.new(rsa_key, alg: 'none') } + it 'raises JWT::JWKError' do + expect { rsa.verify(data: data, signature: 'signature') }.to raise_error(JWT::JWKError, 'none algorithm usage not supported via JWK') + end + end + + context 'when the jwk has HS256 as the alg parameter' do + let(:rsa) { described_class.new(rsa_key, alg: 'HS256') } + it 'raises JWT::DecodeError' do + expect { rsa.verify(data: data, signature: 'signature') }.to raise_error(JWT::DecodeError, 'HMAC key expected to be a String') + end + end end describe '.common_parameters' do diff --git a/spec/jwt/token_spec.rb b/spec/jwt/token_spec.rb index e622dd7f1..26aa7a5a0 100644 --- a/spec/jwt/token_spec.rb +++ b/spec/jwt/token_spec.rb @@ -27,15 +27,29 @@ let(:jwk) { JWT::JWK::RSA.new(OpenSSL::PKey::RSA.new(2048), alg: 'RS256') } it 'signs the token' do - token.sign!(key: jwk) + token.sign!(key: jwk, algorithm: []) # any algorithm is fine here expect(JWT::EncodedToken.new(token.jwt).valid_signature?(algorithm: 'RS256', key: jwk.verify_key)).to be(true) end + + context 'with algorithm provided in sign call' do + it 'signs the token' do + token.sign!(algorithm: %w[RS256 RS512], key: jwk) + + expect(JWT::EncodedToken.new(token.jwt).valid_signature?(algorithm: 'RS256', key: jwk.verify_key)).to be(true) + end + end + + context 'with mismatching algorithm provided in sign call' do + it 'signs the token' do + expect { token.sign!(algorithm: %w[RS384 RS512], key: jwk) }.to raise_error(JWT::DecodeError, 'Provided JWKs do not support one of the specified algorithms: RS384, RS512') + end + end end context 'when string key is given but not algorithm' do it 'raises an error' do - expect { token.sign!(key: 'secret') }.to raise_error(ArgumentError, 'Algorithm must be provided') + expect { token.sign!(key: 'secret') }.to raise_error(ArgumentError, /missing keyword/) end end end