Stricter RSA key generation from jwk parameters

lib/jwt/jwk/rsa.rb

@@ -73,65 +73,88 @@ def import(jwk_data)
           new(rsa_pkey(pkey_params), kid: jwk_attributes(jwk_data, :kid)[:kid])
         end
 
-        private
+        RSA_OPT_PARAMS    = %i[p q dp dq qi].freeze
+        RSA_ASN1_SEQUENCE = (%i[n e d] + RSA_OPT_PARAMS).freeze # https://www.rfc-editor.org/rfc/rfc3447#appendix-A.1.2
 
-        def jwk_attributes(jwk_data, *attributes)
-          attributes.each_with_object({}) do |attribute, hash|
-            value = jwk_data[attribute] || jwk_data[attribute.to_s]
-            value = yield(value) if block_given?
-            hash[attribute] = value
+        def create_rsa_key_using_der(rsa_parameters)
+          validate_rsa_parameters!(rsa_parameters)
+
+          sequence = RSA_ASN1_SEQUENCE.each_with_object([]) do |key, arr|
+            next if rsa_parameters[key].nil?
+
+            arr << OpenSSL::ASN1::Integer.new(rsa_parameters[key])
           end
-        end
 
-        def rsa_pkey(rsa_parameters)
-          raise JWT::JWKError, 'Key format is invalid for RSA' unless rsa_parameters[:n] && rsa_parameters[:e]
+          if sequence.size > 2 # Append "two-prime" version for private key
+            sequence.unshift(OpenSSL::ASN1::Integer.new(0))
+          end
 
-          create_rsa_key(rsa_parameters)
+          OpenSSL::PKey::RSA.new(OpenSSL::ASN1::Sequence(sequence).to_der)
         end
 
-        if ::JWT.openssl_3?
-          ASN1_SEQUENCE = %i[n e d p q dp dq qi].freeze
-          def create_rsa_key(rsa_parameters)
-            sequence = ASN1_SEQUENCE.each_with_object([]) do |key, arr|
-              next if rsa_parameters[key].nil?
+        def create_rsa_key_using_sets(rsa_parameters)
+          validate_rsa_parameters!(rsa_parameters)
 
-              arr << OpenSSL::ASN1::Integer.new(rsa_parameters[key])
-            end
-
-            if sequence.size > 2 # For a private key
-              sequence.unshift(OpenSSL::ASN1::Integer.new(0))
-            end
+          OpenSSL::PKey::RSA.new.tap do |rsa_key|
+            rsa_key.set_key(rsa_parameters[:n], rsa_parameters[:e], rsa_parameters[:d])
+            rsa_key.set_factors(rsa_parameters[:p], rsa_parameters[:q]) if rsa_parameters[:p] && rsa_parameters[:q]
+            rsa_key.set_crt_params(rsa_parameters[:dp], rsa_parameters[:dq], rsa_parameters[:qi]) if rsa_parameters[:dp] && rsa_parameters[:dq] && rsa_parameters[:qi]
+          end
+        end
 
-            OpenSSL::PKey::RSA.new(OpenSSL::ASN1::Sequence(sequence).to_der)
+        def create_rsa_key_using_accessors(rsa_parameters) # rubocop:disable Metrics/AbcSize
+          validate_rsa_parameters!(rsa_parameters)
+
+          OpenSSL::PKey::RSA.new.tap do |rsa_key|
+            rsa_key.n = rsa_parameters[:n]
+            rsa_key.e = rsa_parameters[:e]
+            rsa_key.d = rsa_parameters[:d] if rsa_parameters[:d]
+            rsa_key.p = rsa_parameters[:p] if rsa_parameters[:p]
+            rsa_key.q = rsa_parameters[:q] if rsa_parameters[:q]
+            rsa_key.dmp1 = rsa_parameters[:dp] if rsa_parameters[:dp]
+            rsa_key.dmq1 = rsa_parameters[:dq] if rsa_parameters[:dq]
+            rsa_key.iqmp = rsa_parameters[:qi] if rsa_parameters[:qi]
           end
+        end
+
+        if ::JWT.openssl_3?
+          alias create_rsa_key create_rsa_key_using_der
         elsif OpenSSL::PKey::RSA.new.respond_to?(:set_key)
-          def create_rsa_key(rsa_parameters)
-            OpenSSL::PKey::RSA.new.tap do |rsa_key|
-              rsa_key.set_key(rsa_parameters[:n], rsa_parameters[:e], rsa_parameters[:d])
-              rsa_key.set_factors(rsa_parameters[:p], rsa_parameters[:q]) if rsa_parameters[:p] && rsa_parameters[:q]
-              rsa_key.set_crt_params(rsa_parameters[:dp], rsa_parameters[:dq], rsa_parameters[:qi]) if rsa_parameters[:dp] && rsa_parameters[:dq] && rsa_parameters[:qi]
-            end
-          end
+          alias create_rsa_key create_rsa_key_using_sets
         else
-          def create_rsa_key(rsa_parameters) # rubocop:disable Metrics/AbcSize
-            OpenSSL::PKey::RSA.new.tap do |rsa_key|
-              rsa_key.n = rsa_parameters[:n]
-              rsa_key.e = rsa_parameters[:e]
-              rsa_key.d = rsa_parameters[:d] if rsa_parameters[:d]
-              rsa_key.p = rsa_parameters[:p] if rsa_parameters[:p]
-              rsa_key.q = rsa_parameters[:q] if rsa_parameters[:q]
-              rsa_key.dmp1 = rsa_parameters[:dp] if rsa_parameters[:dp]
-              rsa_key.dmq1 = rsa_parameters[:dq] if rsa_parameters[:dq]
-              rsa_key.iqmp = rsa_parameters[:qi] if rsa_parameters[:qi]
-            end
-          end
+          alias create_rsa_key create_rsa_key_using_accessors
         end
 
         def decode_open_ssl_bn(jwk_data)
           return nil unless jwk_data
 
           OpenSSL::BN.new(::JWT::Base64.url_decode(jwk_data), BINARY)
         end
+
+        private
+
+        def validate_rsa_parameters!(rsa_parameters)
+          return unless rsa_parameters[:d]
+
+          return if RSA_OPT_PARAMS.all? { |k| rsa_parameters.keys.include?(k) }
+          return if RSA_OPT_PARAMS.none? { |k| rsa_parameters.keys.include?(k) }
+
+          raise JWT::JWKError, 'When one of p, q, dp, dq or qi is given all the other optimization parameters also needs to be defined'
+        end
+
+        def jwk_attributes(jwk_data, *attributes)
+          attributes.each_with_object({}) do |attribute, hash|
+            value = jwk_data[attribute] || jwk_data[attribute.to_s]
+            value = yield(value) if block_given?
+            hash[attribute] = value
+          end
+        end
+
+        def rsa_pkey(rsa_parameters)
+          raise JWT::JWKError, 'Key format is invalid for RSA' unless rsa_parameters[:n] && rsa_parameters[:e]
+
+          create_rsa_key(rsa_parameters)
+        end
       end
     end
   end

spec/jwk/rsa_spec.rb

@@ -123,4 +123,74 @@
       end
     end
   end
+
+  shared_examples 'creating an RSA object from JWK parameters' do
+    let(:rsa_parameters) { jwk_parameters.transform_values { |value| described_class.decode_open_ssl_bn(value) } }
+    let(:all_jwk_parameters) { described_class.new(rsa_key).export(include_private: true) }
+
+    context 'when e and n is given' do
+      let(:jwk_parameters) { all_jwk_parameters.slice(:e, :n) }
+
+      it 'creates a valid RSA object representing a public key' do
+        expect(subject).to be_a(::OpenSSL::PKey::RSA)
+        expect(subject.private?).to eq(false)
+      end
+    end
+
+    context 'when e, n, d is given' do
+      let(:jwk_parameters) { all_jwk_parameters.slice(:e, :n, :d) }
+
+      it 'creates a valid RSA object representing a private key' do
+        expect(subject).to be_a(::OpenSSL::PKey::RSA)
+        expect(subject.private?).to eq(true)
+      end
+
+      it 'can be used for encryption and decryption' do
+        expect(subject.private_decrypt(subject.public_encrypt('secret'))).to eq('secret')
+      end
+
+      it 'can be used for signing and verification' do
+        data = 'data_to_sign'
+        signature = subject.sign(OpenSSL::Digest.new('SHA512'), data)
+        expect(subject.verify(OpenSSL::Digest.new('SHA512'), signature, data)).to eq(true)
+      end
+    end
+
+    context 'when e, n, d, p and q is given' do
+      let(:jwk_parameters) { all_jwk_parameters.slice(:e, :n, :d, :p, :q) }
+
+      it 'creates a valid RSA object representing a public key' do
+        expect { subject }.to raise_error(JWT::JWKError, 'When one of p, q, dp, dq or qi is given all the other optimization parameters also needs to be defined')
+      end
+    end
+
+    context 'when n, e, d, p, q, dp, dq, qi is given' do
+      let(:jwk_parameters) { all_jwk_parameters.slice(:n, :e, :d, :p, :q, :dp, :dq, :qi) }
+
+      it 'creates a valid RSA object representing a public key' do
+        expect(subject).to be_a(::OpenSSL::PKey::RSA)
+        expect(subject.private?).to eq(true)
+      end
+    end
+  end
+
+  describe '.create_rsa_key_using_der' do
+    subject(:rsa) { described_class.create_rsa_key_using_der(rsa_parameters) }
+
+    include_examples 'creating an RSA object from JWK parameters'
+  end
+
+  if OpenSSL::PKey::RSA.new.respond_to?(:set_key) # Very old OpenSSL versions (pre 1.1.0)
+    describe '.create_rsa_key_using_sets' do
+      subject(:rsa) { described_class.create_rsa_key_using_sets(rsa_parameters) }
+
+      include_examples 'creating an RSA object from JWK parameters'
+    end
+  elsif !::JWK.openssl_3?
+    describe '.create_rsa_key_using_accessors' do
+      subject(:rsa) { described_class.create_rsa_key_using_accessors(rsa_parameters) }
+
+      include_examples 'creating an RSA object from JWK parameters'
+    end
+  end
 end