Do not automatically trust the alg in the JWK

Author: anakinj

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

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'

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

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

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
 

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

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: :algorithm')
       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

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

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

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: :algorithm')
       end
     end
   end