Method to Match certs including SHA256 errors

lib/saml_idp.rb

@@ -92,6 +92,10 @@ def gather_errors(fingerprint, options = {})
         { cert: options[:cert].serial.to_s, error_code: e.error_code }
       end
 
+      def valid_sig_with_sha256?(cert, options = {})
+        signed_document.validate_with_sha256(cert, options)
+      end
+
       def to_xml
         super(
           save_with: Nokogiri::XML::Node::SaveOptions::AS_XML | Nokogiri::XML::Node::SaveOptions::NO_DECLARATION

lib/saml_idp/request.rb

@@ -157,6 +157,15 @@ def matching_cert
       end
     end
 
+    def sha256_validation_matching_cert
+      return nil unless signed?
+
+      Array(service_provider.certs).find do |cert|
+        document.valid_sig_with_sha256?(cert, options)
+      rescue SamlIdp::XMLSecurity::SignedDocument::ValidationError
+      end
+    end
+
     def cert_errors
       return nil unless signed?
 

lib/saml_idp/version.rb

@@ -1,3 +1,3 @@
 module SamlIdp
-  VERSION = '0.23.1-18f'.freeze
+  VERSION = '0.23.2-18f'.freeze
 end

lib/saml_idp/xml_security.rb

@@ -53,16 +53,13 @@ def validate(idp_cert_fingerprint, soft = true, options = {})
         log 'Validate the fingerprint'
         base64_cert = find_base64_cert(options)
         cert_text = Base64.decode64(base64_cert)
-        cert =
-          begin
-            OpenSSL::X509::Certificate.new(cert_text)
-          rescue OpenSSL::X509::CertificateError
-            return false if soft
-            raise ValidationError.new(
-              'Invalid certificate',
-              :invalid_certificate_in_request
-            )
-          end
+
+        cert, error = valid_cert(cert_text)
+        if error.present?
+          return false if soft
+
+          raise error
+        end
 
         # check cert matches registered idp cert
         fingerprint = fingerprint_cert(cert, options)
@@ -77,6 +74,32 @@ def validate(idp_cert_fingerprint, soft = true, options = {})
         validate_doc(base64_cert, soft, options)
       end
 
+      def validate_with_sha256(idp_certificate, options = {})
+        if request_cert && Base64.decode64(request_cert) != idp_certificate.to_der
+          raise ValidationError.new('Request certificate not valid or registered', :request_cert_not_registered)
+        end
+
+        validate_doc(request_cert, false, options)
+      end
+
+      def request_cert
+        if cert_element.text.blank?
+          raise ValidationError.new(
+            'Certificate element present in response (ds:X509Certificate) but evaluating to nil',
+            :no_certificate_in_request
+          )
+        end
+
+        cert_element.text
+      end
+
+      # @return [Array(OpenSSL::X509::Certificate, false), Array(nil, Error)] tuple of (cert, error)
+      def valid_cert(cert_text)
+        return OpenSSL::X509::Certificate.new(cert_text), false
+      rescue OpenSSL::X509::CertificateError
+        return nil, ValidationError.new('Invalid certificate', :invalid_certificate_in_request)
+      end
+
       def validate_doc(base64_cert, soft = true, options = {})
         if options[:get_params] && options[:get_params][:Signature]
           validate_doc_params_signature(base64_cert, soft, options[:get_params])
@@ -111,7 +134,6 @@ def fingerprint_cert_sha1(cert)
       end
 
       def find_base64_cert(options)
-        cert_element = document.at_xpath('//ds:X509Certificate | //X509Certificate', DS_NS)
         if cert_element
           return cert_element.text if cert_element.text.present?
 
@@ -236,7 +258,8 @@ def verify_signature(base64_cert, sig_alg, signature, canon_string, soft)
         cert = OpenSSL::X509::Certificate.new(cert_text)
         signature_algorithm = algorithm(sig_alg)
 
-        if signature_algorithm != SamlIdp.config.algorithm
+        # remove if !soft when we are sure this wont break partners
+        if !soft && signature_algorithm != SamlIdp.config.algorithm
           return false if soft
 
           raise ValidationError.new(
@@ -298,6 +321,10 @@ def extract_inclusive_namespaces
         )&.attr('PrefixList')&.split(' ') || []
       end
 
+      def cert_element
+        @cert_element ||= document.at_xpath('//ds:X509Certificate | //X509Certificate', DS_NS)
+      end
+
       def log(msg, level: :debug)
         if Rails && Rails.logger
           Rails.logger.send(level, msg)

spec/lib/saml_idp/request_spec.rb

@@ -449,6 +449,80 @@ module SamlIdp
       end
     end
 
+    describe '#sha256_validation_matching_cert' do
+      context 'when document is not signed' do
+        it 'returns nil' do
+          expect(subject.matching_cert).to be_nil
+        end
+      end
+
+      context 'when document is signed' do
+        let(:signed) { true }
+        let(:service_provider) { subject.service_provider }
+        let(:cert) { saml_settings.get_sp_cert }
+
+        describe 'the service provider has no registered certs' do
+          before { subject.service_provider.certs = [] }
+
+          it 'returns nil' do
+            expect(subject.sha256_validation_matching_cert).to be_nil
+          end
+        end
+
+        describe 'the service provider has one registered cert' do
+          before { subject.service_provider.certs = [cert] }
+
+          describe 'the cert matches the assertion cert' do
+            it 'returns the cert' do
+              expect(subject.sha256_validation_matching_cert).to eq cert
+            end
+
+            context 'when the signature algorithm is not right' do
+              let(:security_overrides) do
+                {
+                  signature_method: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha1"
+                }
+              end
+
+              it 'returns nil' do
+                expect(subject.sha256_validation_matching_cert).to eq nil
+              end
+            end
+          end
+
+          describe 'the cert does not match the assertion cert' do
+            let(:cert) { OpenSSL::X509::Certificate.new(custom_idp_x509_cert) }
+
+            it 'returns nil' do
+              expect(subject.sha256_validation_matching_cert).to be_nil
+            end
+          end
+        end
+
+        describe 'multiple certs' do
+          let(:not_matching_cert) { OpenSSL::X509::Certificate.new(custom_idp_x509_cert) }
+
+          before { subject.service_provider.certs = [not_matching_cert, invalid_cert, cert] }
+
+          it 'returns the matching cert' do
+            expect(subject.sha256_validation_matching_cert).to eq cert
+          end
+
+          context 'when the signature algorithm is not right' do
+            let(:security_overrides) do
+              {
+                signature_method: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha1"
+              }
+            end
+
+            it 'returns nil' do
+              expect(subject.sha256_validation_matching_cert).to eq nil
+            end
+          end
+        end
+      end
+    end
+
     describe '#cert_errors' do
       describe 'document is not signed' do
         let(:signed) { false }

spec/support/certificate_helpers.rb

@@ -28,14 +28,6 @@ def invalid_cert
     OpenSSL::X509::Certificate.new(File.read('spec/support/certificates/too_short_cert.crt'))
   end
 
-  def add_cert_boundaries(cert_text)
-    <<~TEXT
-      -----BEGIN CERTIFICATE-----
-      #{cert_text}
-      -----END CERTIFICATE-----
-    TEXT
-  end
-
   def remove_cert_boundaries(cert)
     cert.
       gsub("-----BEGIN CERTIFICATE-----\n", '').

spec/xml_security_spec.rb

@@ -8,7 +8,7 @@ module SamlIdp
     let(:auth_request) { custom_saml_request }
     let(:request) { Request.from_deflated_request(auth_request) }
     let(:base64_cert_text) { saml_settings.certificate }
-    let(:base64_cert) { OpenSSL::X509::Certificate.new(add_cert_boundaries(saml_settings.certificate)) }
+    let(:base64_cert) { OpenSSL::X509::Certificate.new(Base64.decode64(saml_settings.certificate)) }
 
     subject do
       request.send(:document).signed_document
@@ -170,21 +170,16 @@ module SamlIdp
         context 'when using SHA1 as a signing algorithm' do
           let(:algorithm) { '1' }
 
-          it 'raises an error' do
+          it 'validates using SHA1' do
             fingerprint = OpenSSL::Digest::SHA1.new(base64_cert.to_der).hexdigest
-            expect { subject.validate(fingerprint, false) }.to(
-              raise_error(
-                SamlIdp::XMLSecurity::SignedDocument::ValidationError,
-                'Signature Algorithm needs to be SHA256'
-              )
-            )
+            expect(subject.validate(fingerprint)).to be true
           end
         end
 
         context 'when using SHA256 as a signing algorithm' do
           let(:algorithm) { '256' }
 
-          it 'validate using SHA256' do
+          it 'validates using SHA256' do
             fingerprint = OpenSSL::Digest::SHA256.new(base64_cert.to_der).hexdigest
             expect(subject.validate(fingerprint)).to be true
           end
@@ -193,30 +188,131 @@ module SamlIdp
         context 'when using SHA384 as a signing algorithm' do
           let(:algorithm) { '384' }
 
-          it 'raises an error' do
+          it 'validates using SHA384' do
             fingerprint = OpenSSL::Digest::SHA384.new(base64_cert.to_der).hexdigest
-            expect { subject.validate(fingerprint, false) }.to(
+            expect(subject.validate(fingerprint)).to be true
+          end
+        end
+
+        context 'when using SHA512 as a signing algorithm' do
+          let(:algorithm) { '512' }
+
+          it 'validates using SHA512' do
+            fingerprint = OpenSSL::Digest::SHA512.new(base64_cert.to_der).hexdigest
+            expect(subject.validate(fingerprint)).to be true
+          end
+        end
+      end
+    end
+
+    describe '#validate_with_sha256' do
+      context 'with an embedded request' do
+
+        context 'when the request certificate does not match an idp certificate' do
+          let(:cert_element) { double Nokogiri::XML::Element }
+          let(:wrong_cert) { remove_cert_boundaries(custom_idp_x509_cert) }
+
+          before do
+            allow(subject.document).to receive(:at_xpath).
+              with('//ds:X509Certificate | //X509Certificate', ds_namespace).
+              and_return(cert_element)
+
+            allow(cert_element).to receive(:text).and_return(wrong_cert)
+          end
+
+          it 'raises an error' do
+            expect { subject.validate_with_sha256(base64_cert) }.to(
               raise_error(
                 SamlIdp::XMLSecurity::SignedDocument::ValidationError,
-                'Signature Algorithm needs to be SHA256'
+                'Request certificate not valid or registered'
               )
             )
           end
         end
 
-        context 'when using SHA512 as a signing algorithm' do
-          let(:algorithm) { '512' }
+        context 'when the request certificate is invalid' do
+          let(:cert_element) { double Nokogiri::XML::Element }
+          let(:wrong_cert) { 'invalid_cert' }
+
+          before do
+            allow(subject.document).to receive(:at_xpath).
+              with('//ds:X509Certificate | //X509Certificate', ds_namespace).
+              and_return(cert_element)
+
+            allow(cert_element).to receive(:text).and_return(wrong_cert)
+          end
 
           it 'raises an error' do
-            fingerprint = OpenSSL::Digest::SHA512.new(base64_cert.to_der).hexdigest
-            expect { subject.validate(fingerprint, false) }.to(
+            expect { subject.validate_with_sha256(base64_cert) }.to(
               raise_error(
                 SamlIdp::XMLSecurity::SignedDocument::ValidationError,
-                'Signature Algorithm needs to be SHA256'
+                'Request certificate not valid or registered'
               )
             )
           end
         end
+
+        context 'with different signing algorithms' do
+          let(:signature_method) { "http://www.w3.org/2001/04/xmldsig-more#rsa-sha#{algorithm}" }
+          let(:digest_method) { "http://www.w3.org/2001/04/xmldsig-more#rsa-sha#{algorithm}" }
+
+          let(:auth_request) do
+            custom_saml_request(
+              security_overrides: {
+                signature_method:,
+                digest_method:,
+              }
+            )
+          end
+
+          context 'when using SHA1 as a signing algorithm' do
+            let(:algorithm) { '1' }
+
+            it 'raises an error' do
+              fingerprint = OpenSSL::Digest::SHA1.new(base64_cert.to_der).hexdigest
+              expect { subject.validate_with_sha256(base64_cert) }.to(
+                raise_error(
+                  SamlIdp::XMLSecurity::SignedDocument::ValidationError,
+                  'Signature Algorithm needs to be SHA256'
+                )
+              )
+            end
+          end
+
+          context 'when using SHA256 as a signing algorithm' do
+            let(:algorithm) { '256' }
+
+            it 'validate using SHA256' do
+              expect(subject.validate_with_sha256(base64_cert)).to be true
+            end
+          end
+
+          context 'when using SHA384 as a signing algorithm' do
+            let(:algorithm) { '384' }
+
+            it 'raises an error' do
+              expect { subject.validate_with_sha256(base64_cert) }.to(
+                raise_error(
+                  SamlIdp::XMLSecurity::SignedDocument::ValidationError,
+                  'Signature Algorithm needs to be SHA256'
+                )
+              )
+            end
+          end
+
+          context 'when using SHA512 as a signing algorithm' do
+            let(:algorithm) { '512' }
+
+            it 'raises an error' do
+              expect { subject.validate_with_sha256(base64_cert) }.to(
+                raise_error(
+                  SamlIdp::XMLSecurity::SignedDocument::ValidationError,
+                  'Signature Algorithm needs to be SHA256'
+                )
+              )
+            end
+          end
+        end
       end
     end