Merge pull request #273 from onelogin/no_x509certificate

#220 Support SAMLResponse without ds:x509certificate

Author: pitbulk

lib/onelogin/ruby-saml/response.rb

@@ -572,6 +572,7 @@ def validate_subject_confirmation
       #
       def validate_signature
         fingerprint = settings.get_fingerprint
+        idp_cert = settings.get_idp_cert
 
         # If the response contains the signature, and the assertion was encrypted, validate the original SAML Response
         # otherwise, review if the decrypted assertion contains a signature
@@ -583,7 +584,11 @@ def validate_signature
         )
         doc = (response_signed || decrypted_document.nil?) ? document : decrypted_document
 
-        unless fingerprint && doc.validate_document(fingerprint, :fingerprint_alg => settings.idp_cert_fingerprint_algorithm)
+        opts = {}
+        opts[:fingerprint_alg] = settings.idp_cert_fingerprint_algorithm
+        opts[:cert] = idp_cert
+
+        unless fingerprint && doc.validate_document(fingerprint, @soft, opts)
           error_msg = "Invalid Signature on SAML Response"
           return append_error(error_msg)
         end

lib/xml_security.rb

@@ -199,30 +199,35 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {})
         "//ds:X509Certificate",
         { "ds"=>DSIG }
       )
-      unless cert_element
-        if soft
-          return false
+
+      if cert_element        
+        base64_cert = cert_element.text
+        cert_text = Base64.decode64(base64_cert)
+        cert = OpenSSL::X509::Certificate.new(cert_text)
+
+        if options[:fingerprint_alg]
+          fingerprint_alg = XMLSecurity::BaseDocument.new.algorithm(options[:fingerprint_alg]).new
         else
-          raise OneLogin::RubySaml::ValidationError.new("Certificate element missing in response (ds:X509Certificate)")
+          fingerprint_alg = OpenSSL::Digest::SHA1.new
         end
-      end
-      base64_cert = cert_element.text
-      cert_text = Base64.decode64(base64_cert)
-      cert = OpenSSL::X509::Certificate.new(cert_text)
+        fingerprint = fingerprint_alg.hexdigest(cert.to_der)
 
-      if options[:fingerprint_alg]
-        fingerprint_alg = XMLSecurity::BaseDocument.new.algorithm(options[:fingerprint_alg]).new
+        # check cert matches registered idp cert
+        if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
+          @errors << "Fingerprint mismatch"
+          return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch"))
+        end
       else
-        fingerprint_alg = OpenSSL::Digest::SHA1.new
-      end
-      fingerprint = fingerprint_alg.hexdigest(cert.to_der)
-
-      # check cert matches registered idp cert
-      if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
-        @errors << "Fingerprint mismatch"
-        return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch"))
+        if options[:cert]
+          base64_cert = Base64.encode64(options[:cert].to_pem)
+        else
+          if soft
+            return false
+          else
+            raise OneLogin::RubySaml::ValidationError.new("Certificate element missing in response (ds:X509Certificate) and not cert provided at settings")
+          end          
+        end
       end
-
       validate_signature(base64_cert, soft)
     end
 

test/response_test.rb

@@ -16,6 +16,7 @@ class RubySamlTest < Minitest::Test
     let(:response_wrapped) { OneLogin::RubySaml::Response.new(response_document_wrapped) }
     let(:response_multiple_attr_values) { OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values)) }
     let(:response_valid_signed) { OneLogin::RubySaml::Response.new(response_document_valid_signed) }
+    let(:response_valid_signed_without_x509certificate) { OneLogin::RubySaml::Response.new(response_document_valid_signed_without_x509certificate) }
     let(:response_no_id) { OneLogin::RubySaml::Response.new(read_invalid_response("no_id.xml.base64")) }
     let(:response_no_version) { OneLogin::RubySaml::Response.new(read_invalid_response("no_saml2.xml.base64")) }
     let(:response_multi_assertion) { OneLogin::RubySaml::Response.new(read_invalid_response("multiple_assertions.xml.base64")) }
@@ -695,6 +696,30 @@ class RubySamlTest < Minitest::Test
         assert !response.send(:validate_signature)
         assert_includes response.errors, "Invalid Signature on SAML Response"
       end
+
+      it "return false when no X509Certificate and not cert provided at settings" do
+        settings.idp_cert_fingerprint = ruby_saml_cert_fingerprint
+        settings.idp_cert = nil
+        response_valid_signed_without_x509certificate.settings = settings
+        assert !response_valid_signed_without_x509certificate.send(:validate_signature)
+        assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response"
+      end
+
+      it "return false when no X509Certificate and the cert provided at settings mismatches" do
+        settings.idp_cert_fingerprint = nil
+        settings.idp_cert = signature_1
+        response_valid_signed_without_x509certificate.settings = settings
+        assert !response_valid_signed_without_x509certificate.send(:validate_signature)
+        assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response"        
+      end
+
+      it "return true when no X509Certificate and the cert provided at settings matches" do
+        settings.idp_cert_fingerprint = nil
+        settings.idp_cert = ruby_saml_cert_text
+        response_valid_signed_without_x509certificate.settings = settings
+        assert response_valid_signed_without_x509certificate.send(:validate_signature)
+        assert_empty response_valid_signed_without_x509certificate.errors
+      end
     end
 
     describe "#nameid" do

test/responses/valid_response_without_x509certificate.xml.base64

@@ -0,0 +1 @@
+pVZdd9o4EH3fc/Y/+LiPOcayDTb4BLoU0oSWfBDTbpuXPbI0Bie25Egi0Pz6lQ04kJI03X2CGY/u3LkjjXT8fpVnxgMImXLWNZ0GMt/3/vzjWOI8K8JrkAVnEgwdxGRYOrvmQrCQY5nKkOEcZKhIGPXPx6HbQCGWEoTSUObOkuL1NYXgihOemcZo2DWLZBWTtuvjhFrQScCiXhJbtB00LdIMECIeJIi0TePrlrPG0EulXMCISYWZ0i7kNC3kW6g5RW7ouiFyb0xjCFKlDKtq1VypIrRtXBSNfEFxg3FbSm4TXe4iBw3ItsVPedf8JyFN7DVjZAWg6SDHBYtgDFbQdmMSO14ce22zV8kWVlxEr8wgNyk4g4zPUtYgPLfLIPfY3o09pjKM0pkmtxBbtamsWS6Xy8bSa3Axs12EkI06to6hMp29M3W3DGO7HuiIJbyCG2DGWUpwlj5WJZ+DmnNq9LMZF6ma5y+AO7aDSnALVsQiTpO9M+0qxVOSiuQb4fa4CoktOcfOBrHEu4YEBDACxpfrUdd899b2VyVOBWYy4SKX++bvsQL2oJtTALXktjhN8PcAD6p2bP/McZjO9C78L+JthHsC+YqzBfRGj82bSSa9/qebYezJ9gP58s32rsnRx0m3IrAbXDlqydfms21TN3i9YjL//Cnn7Ie8cu5zhoPoCE6cMXHF5/7t7cC9PBWTy1l0e1VMOVnkE3/+/ezucRrdiEDZI/vz9DJ6OBtM7oeOmDezCN0Or+yTxdlFf6DuOt+DLHO//JgtAnd8FH9U9zduZ8g6YnwX0VORHo2Cs/lq7PkBTfpRNPeP5I/gb3g4oXfnyRKGE7f/TR8i/8OH09ljMD3p1uXs8NejbM/b20y2SGlT7lsDTsGolr0+sGQVHUYLQkDKqtE/g4b97SjcHOfVS8fZsb+djyMyhxybdWz662ArrcYdgZ9m4XqMdlqOH6PEs5otz7cclPgW8RNsdShtuR44zU6bvGlw/o+xVhnRIr4FojbWhdZzNDQ+6iOB1ctCOw2n8qTUSqrQUJecZn1KRSl6T+lN/ddu/k3mNfx+5gFnSVpilN1Yn73XO0zyMAYsQJgvAw2xwsYFV5fsUvQTBaJUz0M76gXra+caSFqkUMortn/rTXMI+dmnDQUdQdPysyyzfgCtClQNc55SOpuUb6C13aULmpazQF92SqRknX7vS91wyXPQgjdghfMig6rneBO0YVyveWbvodvPyqnzqTkrTxDkWiCjMn9xoUd6J2iEF6ptHQgdMQorfZ07ftIKUBIT5DkthAKMvFaLJoD92CNNn5i7pDRVBSt1wDXI9INHz9Peq28iEpIyTruv9M+SC3qlnzy6s0Cr26HgQtWCHQA/8G3PV4tWe7ejp55M27dM718=

test/test_helper.rb

@@ -52,6 +52,10 @@ def response_document_valid_signed
     @response_document_valid_signed ||= read_response("valid_response.xml.base64")
   end
 
+  def response_document_valid_signed_without_x509certificate
+    @response_document_valid_signed_without_x509certificate ||= read_response("valid_response_without_x509certificate.xml.base64")
+  end
+
   def response_document_without_recipient
     @response_document_without_recipient ||= read_response("response_with_undefined_recipient.xml.base64")
   end

test/xml_security_test.rb

@@ -68,13 +68,20 @@ class XmlSecurityTest < Minitest::Test
       assert adfs_document.validate_signature(base64cert, false)
     end
 
-    it "raise validation error when the X509Certificate is missing" do
+    it "raise validation error when the X509Certificate is missing and no cert provided" do
       decoded_response.sub!(/<ds:X509Certificate>.*<\/ds:X509Certificate>/, "")
       mod_document = XMLSecurity::SignedDocument.new(decoded_response)
       exception = assert_raises(OneLogin::RubySaml::ValidationError) do
         mod_document.validate_document("a fingerprint", false) # The fingerprint isn't relevant to this test
       end
-      assert_equal("Certificate element missing in response (ds:X509Certificate)", exception.message)
+      assert_equal("Certificate element missing in response (ds:X509Certificate) and not cert provided at settings", exception.message)
+    end
+
+    it "invalidaties when the X509Certificate is missing and the cert is provided but mismatches" do
+      decoded_response.sub!(/<ds:X509Certificate>.*<\/ds:X509Certificate>/, "")
+      mod_document = XMLSecurity::SignedDocument.new(decoded_response)
+      cert = OpenSSL::X509::Certificate.new(ruby_saml_cert)
+      assert !mod_document.validate_document("a fingerprint", true, :cert => cert) # The fingerprint isn't relevant to this test
     end
   end