Allow point to be used as the verification key in ECDSA (#689)

* Convert points to ec pkeys

* Handle legacy Rubies

Author: anakinj

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 - 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))
+- Allow `OpenSSL::PKey::EC::Point` to be used as the verification key in ECDSA [#689](https://github.com/jwt/ruby-jwt/pull/689) ([@anakinj](https://github.com/anakinj))
 - Your contribution here
 
 **Fixes and enhancements:**

lib/jwt/jwa/ecdsa.rb

@@ -12,7 +12,8 @@ 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)
+        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)
+        raise_sign_error!('The given key is not a private key') unless signing_key.private?
 
         curve_definition = curve_by_name(signing_key.group.curve_name)
         key_algorithm = curve_definition[:algorithm]
@@ -23,7 +24,9 @@ def sign(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)
+        verification_key = self.class.create_public_key_from_point(verification_key) if verification_key.is_a?(::OpenSSL::PKey::EC::Point)
+
+        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]
@@ -67,6 +70,22 @@ def self.curve_by_name(name)
         end
       end
 
+      if ::JWT.openssl_3?
+        def self.create_public_key_from_point(point)
+          sequence = OpenSSL::ASN1::Sequence([
+                                               OpenSSL::ASN1::Sequence([OpenSSL::ASN1::ObjectId('id-ecPublicKey'), OpenSSL::ASN1::ObjectId(point.group.curve_name)]),
+                                               OpenSSL::ASN1::BitString(point.to_octet_string(:uncompressed))
+                                             ])
+          OpenSSL::PKey::EC.new(sequence.to_der)
+        end
+      else
+        def self.create_public_key_from_point(point)
+          OpenSSL::PKey::EC.new(point.group.curve_name).tap do |key|
+            key.public_key = point
+          end
+        end
+      end
+
       private
 
       attr_reader :digest

lib/jwt/jwk/ec.rb

@@ -136,67 +136,54 @@ def parse_ec_key(key)
         }.compact
       end
 
-      if ::JWT.openssl_3?
-        def create_ec_key(jwk_crv, jwk_x, jwk_y, jwk_d) # rubocop:disable Metrics/MethodLength
-          curve = EC.to_openssl_curve(jwk_crv)
-          x_octets = decode_octets(jwk_x)
-          y_octets = decode_octets(jwk_y)
-
-          point = OpenSSL::PKey::EC::Point.new(
-            OpenSSL::PKey::EC::Group.new(curve),
-            OpenSSL::BN.new([0x04, x_octets, y_octets].pack('Ca*a*'), 2)
-          )
-
-          sequence = if jwk_d
-                       # https://datatracker.ietf.org/doc/html/rfc5915.html
-                       # ECPrivateKey ::= SEQUENCE {
-                       #   version        INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
-                       #   privateKey     OCTET STRING,
-                       #   parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
-                       #   publicKey  [1] BIT STRING OPTIONAL
-                       # }
-
-                       OpenSSL::ASN1::Sequence([
-                                                 OpenSSL::ASN1::Integer(1),
-                                                 OpenSSL::ASN1::OctetString(OpenSSL::BN.new(decode_octets(jwk_d), 2).to_s(2)),
-                                                 OpenSSL::ASN1::ObjectId(curve, 0, :EXPLICIT),
-                                                 OpenSSL::ASN1::BitString(point.to_octet_string(:uncompressed), 1, :EXPLICIT)
-                                               ])
-                     else
-                       OpenSSL::ASN1::Sequence([
-                                                 OpenSSL::ASN1::Sequence([OpenSSL::ASN1::ObjectId('id-ecPublicKey'), OpenSSL::ASN1::ObjectId(curve)]),
-                                                 OpenSSL::ASN1::BitString(point.to_octet_string(:uncompressed))
-                                               ])
-                     end
+      def create_point(jwk_crv, jwk_x, jwk_y)
+        curve = EC.to_openssl_curve(jwk_crv)
+        x_octets = decode_octets(jwk_x)
+        y_octets = decode_octets(jwk_y)
+
+        # The details of the `Point` instantiation are covered in:
+        # - https://docs.ruby-lang.org/en/2.4.0/OpenSSL/PKey/EC.html
+        # - https://www.openssl.org/docs/manmaster/man3/EC_POINT_new.html
+        # - https://tools.ietf.org/html/rfc5480#section-2.2
+        # - https://www.secg.org/SEC1-Ver-1.0.pdf
+        # Section 2.3.3 of the last of these references specifies that the
+        # encoding of an uncompressed point consists of the byte `0x04` followed
+        # by the x value then the y value.
+        OpenSSL::PKey::EC::Point.new(
+          OpenSSL::PKey::EC::Group.new(curve),
+          OpenSSL::BN.new([0x04, x_octets, y_octets].pack('Ca*a*'), 2)
+        )
+      end
 
+      if ::JWT.openssl_3?
+        def create_ec_key(jwk_crv, jwk_x, jwk_y, jwk_d)
+          point = create_point(jwk_crv, jwk_x, jwk_y)
+
+          return ::JWT::JWA::Ecdsa.create_public_key_from_point(point) unless jwk_d
+
+          # https://datatracker.ietf.org/doc/html/rfc5915.html
+          # ECPrivateKey ::= SEQUENCE {
+          #   version        INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
+          #   privateKey     OCTET STRING,
+          #   parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
+          #   publicKey  [1] BIT STRING OPTIONAL
+          # }
+
+          sequence = OpenSSL::ASN1::Sequence([
+                                               OpenSSL::ASN1::Integer(1),
+                                               OpenSSL::ASN1::OctetString(OpenSSL::BN.new(decode_octets(jwk_d), 2).to_s(2)),
+                                               OpenSSL::ASN1::ObjectId(point.group.curve_name, 0, :EXPLICIT),
+                                               OpenSSL::ASN1::BitString(point.to_octet_string(:uncompressed), 1, :EXPLICIT)
+                                             ])
           OpenSSL::PKey::EC.new(sequence.to_der)
         end
       else
         def create_ec_key(jwk_crv, jwk_x, jwk_y, jwk_d)
-          curve = EC.to_openssl_curve(jwk_crv)
-
-          x_octets = decode_octets(jwk_x)
-          y_octets = decode_octets(jwk_y)
-
-          key = OpenSSL::PKey::EC.new(curve)
-
-          # The details of the `Point` instantiation are covered in:
-          # - https://docs.ruby-lang.org/en/2.4.0/OpenSSL/PKey/EC.html
-          # - https://www.openssl.org/docs/manmaster/man3/EC_POINT_new.html
-          # - https://tools.ietf.org/html/rfc5480#section-2.2
-          # - https://www.secg.org/SEC1-Ver-1.0.pdf
-          # Section 2.3.3 of the last of these references specifies that the
-          # encoding of an uncompressed point consists of the byte `0x04` followed
-          # by the x value then the y value.
-          point = OpenSSL::PKey::EC::Point.new(
-            OpenSSL::PKey::EC::Group.new(curve),
-            OpenSSL::BN.new([0x04, x_octets, y_octets].pack('Ca*a*'), 2)
-          )
-
-          key.public_key = point
-          key.private_key = OpenSSL::BN.new(decode_octets(jwk_d), 2) if jwk_d
-
-          key
+          point = create_point(jwk_crv, jwk_x, jwk_y)
+
+          ::JWT::JWA::Ecdsa.create_public_key_from_point(point).tap do |key|
+            key.private_key = OpenSSL::BN.new(decode_octets(jwk_d), 2) if jwk_d
+          end
         end
       end
 
@@ -205,7 +192,7 @@ def decode_octets(base64_encoded_coordinate)
         # Some base64 encoders on some platform omit a single 0-byte at
         # the start of either Y or X coordinate of the elliptic curve point.
         # This leads to an encoding error when data is passed to OpenSSL BN.
-        # It is know to have happend to exported JWKs on a Java application and
+        # It is know to have happened to exported JWKs on a Java application and
         # on a Flutter/Dart application (both iOS and Android). All that is
         # needed to fix the problem is adding a leading 0-byte. We know the
         # required byte is 0 because with any other byte the point is no longer

spec/jwt/jwa/ecdsa_spec.rb

@@ -35,18 +35,19 @@
   let(:ecdsa_key) { test_pkey('ec256-private.pem') }
   let(:data) { 'test data' }
   let(:instance) { described_class.new('ES256', 'sha256') }
+  let(:signature) { instance.sign(data: data, signing_key: ecdsa_key) }
 
   describe '#verify' do
     context 'when the verification key is valid' do
       it 'returns true for a valid signature' do
-        signature = instance.sign(data: data, signing_key: ecdsa_key)
         expect(instance.verify(data: data, signature: signature, verification_key: ecdsa_key)).to be true
       end
 
       it 'returns false for an invalid signature' do
         expect(instance.verify(data: data, signature: 'invalid_signature', verification_key: ecdsa_key)).to be false
       end
     end
+
     context 'when verification results in a OpenSSL::PKey::PKeyError error' do
       it 'raises a JWT::VerificationError' do
         allow(ecdsa_key).to receive(:dsa_verify_asn1).and_raise(OpenSSL::PKey::PKeyError.new('Error'))
@@ -60,25 +61,40 @@
       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.to raise_error(JWT::DecodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance')
+      end
+    end
+
+    context 'when the verification key is a point' do
+      it 'verifies the signature' do
+        expect(ecdsa_key.public_key).to be_a(OpenSSL::PKey::EC::Point)
+        expect(instance.verify(data: data, signature: signature, verification_key: ecdsa_key.public_key)).to be(true)
       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 a public key' do
+      it 'raises a JWT::DecodeError' do
+        public_key = test_pkey('ec256-public.pem')
+        expect do
+          instance.sign(data: data, signing_key: public_key)
+        end.to raise_error(JWT::EncodeError, 'The given key is not a private key')
+      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.to raise_error(JWT::EncodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance')
       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(JWT::DecodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance.')
+            raise_error(JWT::DecodeError, 'The given key is a String. It has to be an OpenSSL::PKey::EC instance')
           )
         end
       end