Check signing and verification key type for ECDSA algorithm (#688)

Author: anakinj

CHANGELOG.md

@@ -7,6 +7,7 @@
 **Features:**
 
 - 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))
+- Raise an error if the ECDSA signing or verification key is not an instance of `OpenSSL::PKey::EC` [#688](https://github.com/jwt/ruby-jwt/pull/688) ([@anakinj](https://github.com/anakinj))
 - Your contribution here
 
 **Fixes and enhancements:**

lib/jwt/jwa/ecdsa.rb

@@ -12,14 +12,19 @@ def initialize(alg, digest)
       end
 
       def sign(data:, signing_key:)
+        raise_sign_error!("The given key is a #{signing_key.class}. It has to be an OpenSSL::PKey::EC instance.") unless signing_key.is_a?(::OpenSSL::PKey::EC)
+
         curve_definition = curve_by_name(signing_key.group.curve_name)
         key_algorithm = curve_definition[:algorithm]
+
         raise IncorrectAlgorithm, "payload algorithm is #{alg} but #{key_algorithm} signing key was provided" if alg != key_algorithm
 
         asn1_to_raw(signing_key.dsa_sign_asn1(digest.digest(data)), signing_key)
       end
 
       def verify(data:, signature:, verification_key:)
+        raise_verify_error!("The given key is a #{verification_key.class}. It has to be an OpenSSL::PKey::EC instance.") unless verification_key.is_a?(::OpenSSL::PKey::EC)
+
         curve_definition = curve_by_name(verification_key.group.curve_name)
         key_algorithm = curve_definition[:algorithm]
         raise IncorrectAlgorithm, "payload algorithm is #{alg} but #{key_algorithm} verification key was provided" if alg != key_algorithm

spec/jwt/jwa/ecdsa_spec.rb

@@ -55,5 +55,40 @@
         end.to raise_error(JWT::VerificationError, 'Signature verification raised')
       end
     end
+
+    context 'when the verification key is not an OpenSSL::PKey::EC instance' do
+      it 'raises a JWT::DecodeError' do
+        expect do
+          instance.verify(data: data, signature: '', verification_key: 'not_a_key')
+        end.to raise_error(JWT::DecodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance.')
+      end
+    end
+  end
+
+  describe '#sign' do
+    context 'when the signing key is valid' do
+      it 'returns a valid signature' do
+        signature = instance.sign(data: data, signing_key: ecdsa_key)
+        expect(signature).to be_a(String)
+        expect(signature.length).to be > 0
+      end
+    end
+
+    context 'when the signing key is not an OpenSSL::PKey::EC instance' do
+      it 'raises a JWT::DecodeError' do
+        expect do
+          instance.sign(data: data, signing_key: 'not_a_key')
+        end.to raise_error(JWT::EncodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance.')
+      end
+    end
+
+    context 'when the signing key is invalid' do
+      it 'raises a JWT::DecodeError' do
+        invalid_key = OpenSSL::PKey::EC.generate('sect571r1')
+        expect do
+          instance.sign(data: data, signing_key: invalid_key)
+        end.to raise_error(JWT::DecodeError, "The ECDSA curve 'sect571r1' is not supported")
+      end
+    end
   end
 end

spec/jwt/jwk/decode_with_jwk_spec.rb

@@ -169,7 +169,7 @@
 
         it 'fails in some way' do
           expect { described_class.decode(signed_token, nil, true, algorithms: ['ES384'], jwks: jwks) }.to(
-            raise_error(NoMethodError, /undefined method .*group/)
+            raise_error(JWT::DecodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance.')
           )
         end
       end

spec/jwt/jwk/ec_spec.rb

@@ -110,6 +110,22 @@
     end
   end
 
+  describe '.to_openssl_curve' do
+    context 'when a valid curve name is given' do
+      it 'returns the corresponding OpenSSL curve name' do
+        expect(JWT::JWK::EC.to_openssl_curve('P-256')).to eq('prime256v1')
+        expect(JWT::JWK::EC.to_openssl_curve('P-384')).to eq('secp384r1')
+        expect(JWT::JWK::EC.to_openssl_curve('P-521')).to eq('secp521r1')
+        expect(JWT::JWK::EC.to_openssl_curve('P-256K')).to eq('secp256k1')
+      end
+    end
+    context 'when an invalid curve name is given' do
+      it 'raises an error' do
+        expect { JWT::JWK::EC.to_openssl_curve('invalid-curve') }.to raise_error(JWT::JWKError, 'Invalid curve provided')
+      end
+    end
+  end
+
   describe '.import' do
     subject { described_class.import(params) }
     let(:include_private) { false }

spec/jwt/jwt_spec.rb

@@ -39,10 +39,6 @@
     }
   end
 
-  after(:each) do
-    expect(OpenSSL.errors).to be_empty
-  end
-
   context 'alg: NONE' do
     let(:alg) { 'none' }
     let(:encoded_token) { data['NONE'] }