XML security get params (#3)

* Pass options with :get_params to signature validation

* Flag options_have_signature helper method as private

* Small refactor to make more methods private

lib/saml_idp.rb

@@ -69,9 +69,14 @@ def signed?
         !!xpath("//ds:Signature", ds: signature_namespace).first
       end
 
-      def valid_signature?(fingerprint)
-        signed? &&
-          signed_document.validate(fingerprint, :soft)
+      def options_have_signature(options)
+        options[:get_params] && options[:get_params][:Signature]
+      end
+      private :options_have_signature
+
+      def valid_signature?(fingerprint, options = {})
+        (signed? || options_have_signature(options)) &&
+          signed_document.validate(fingerprint, :soft, options)
       end
 
       def signed_document

lib/saml_idp/controller.rb

@@ -24,7 +24,7 @@ def validate_saml_request(raw_saml_request = params[:SAMLRequest])
     end
 
     def decode_request(raw_saml_request)
-      self.saml_request = Request.from_deflated_request(raw_saml_request)
+      self.saml_request = Request.from_deflated_request(raw_saml_request, get_params: params)
     end
 
     def authn_context_classref

lib/saml_idp/request.rb

@@ -2,7 +2,7 @@
 require 'saml_idp/service_provider'
 module SamlIdp
   class Request
-    def self.from_deflated_request(raw)
+    def self.from_deflated_request(raw, options = {})
       if raw
         decoded = Base64.decode64(raw)
         zstream = Zlib::Inflate.new(-Zlib::MAX_WBITS)
@@ -17,17 +17,18 @@ def self.from_deflated_request(raw)
       else
         inflated = ""
       end
-      new(inflated)
+      new(inflated, options)
     end
 
-    attr_accessor :raw_xml
+    attr_accessor :raw_xml, :options
 
     delegate :config, to: :SamlIdp
     private :config
     delegate :xpath, to: :document
     private :xpath
 
-    def initialize(raw_xml = "")
+    def initialize(raw_xml = "", options = {})
+      self.options = options
       self.raw_xml = raw_xml
     end
 
@@ -111,7 +112,7 @@ def valid?
     def valid_signature?
       # Force signatures for logout requests because there is no other
       # protection against a cross-site DoS.
-      service_provider.valid_signature?(document, logout_request?)
+      service_provider.valid_signature?(document, logout_request?, self.options)
     end
 
     def service_provider?

lib/saml_idp/service_provider.rb

@@ -19,9 +19,9 @@ def valid?
       attributes.present?
     end
 
-    def valid_signature?(doc, require_signature = false)
+    def valid_signature?(doc, require_signature = false, options = {})
       if require_signature || should_validate_signature?
-        doc.valid_signature?(fingerprint)
+        doc.valid_signature?(fingerprint, options.merge(cert: cert))
       else
         true
       end

lib/saml_idp/xml_security.rb

@@ -43,40 +43,93 @@ def initialize(response)
         extract_signed_element_id
       end
 
-      def validate(idp_cert_fingerprint, soft = true)
-        # get cert from response
-        cert_element = REXML::XPath.first(self, "//ds:X509Certificate", { "ds"=>DSIG })
-        raise ValidationError.new("Certificate element missing in response (ds:X509Certificate)") unless cert_element
-        base64_cert  = cert_element.text
-        cert_text    = Base64.decode64(base64_cert)
-        cert         = OpenSSL::X509::Certificate.new(cert_text)
+      def validate(idp_cert_fingerprint, soft = true, options = {})
+        base64_cert = find_base64_cert(options)
+        cert_text   = Base64.decode64(base64_cert)
+        cert        = OpenSSL::X509::Certificate.new(cert_text)
 
         # check cert matches registered idp cert
-        fingerprint = fingerprint_cert(cert)
+        fingerprint = fingerprint_cert(cert, options)
         sha1_fingerprint = fingerprint_cert_sha1(cert)
         plain_idp_cert_fingerprint = idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
 
         if fingerprint != plain_idp_cert_fingerprint && sha1_fingerprint != plain_idp_cert_fingerprint
           return soft ? false : (raise ValidationError.new("Fingerprint mismatch"))
         end
 
-        validate_doc(base64_cert, soft)
+        validate_doc(base64_cert, soft, options)
+      end
+
+      def validate_doc(base64_cert, soft = true, options = {})
+        if options[:get_params]
+          validate_doc_params_signature(base64_cert, soft, options[:get_params])
+        else
+          validate_doc_embedded_signature(base64_cert, soft)
+        end
+      end
+
+      private
+
+      def signature_algorithm(options)
+        if options[:get_params] && options[:get_params][:SigAlg]
+          algorithm(options[:get_params][:SigAlg])
+        else
+          ref_elem = REXML::XPath.first(self, "//ds:Reference", {"ds"=>DSIG})
+          algorithm(REXML::XPath.first(ref_elem, "//ds:DigestMethod"))
+        end
       end
 
-      def fingerprint_cert(cert)
-        # pick algorithm based on the doc's digest algorithm
-        ref_elem = REXML::XPath.first(self, "//ds:Reference", {"ds"=>DSIG})
-        digest_algorithm = algorithm(REXML::XPath.first(ref_elem, "//ds:DigestMethod"))
+      def fingerprint_cert(cert, options)
+        digest_algorithm = signature_algorithm(options)
         digest_algorithm.hexdigest(cert.to_der)
       end
 
       def fingerprint_cert_sha1(cert)
         OpenSSL::Digest::SHA1.hexdigest(cert.to_der)
       end
 
-      def validate_doc(base64_cert, soft = true)
-        # validate references
+      def find_base64_cert(options)
+        cert_element = REXML::XPath.first(self, "//ds:X509Certificate", { "ds"=>DSIG })
+        if cert_element
+          base64_cert = cert_element.text
+        elsif options[:cert]
+          if options[:cert].is_a?(String)
+            base64_cert = options[:cert]
+          elsif options[:cert].is_a?(OpenSSL::X509::Certificate)
+            base64_cert = Base64.encode64(options[:cert].to_pem)
+          else
+            raise ValidationError.new("options[:cert] must be Base64-encoded String or OpenSSL::X509::Certificate")
+          end
+        else
+          raise ValidationError.new("Certificate element missing in response (ds:X509Certificate) and not provided in options[:cert]")
+        end
+      end
+
+      def request?
+        root.name != 'Response'
+      end
+
+      # matches RubySaml::Utils
+      def build_query(params)
+        type, data, relay_state, sig_alg = [:type, :data, :relay_state, :sig_alg].map { |k| params[k]}
+
+        url_string = "#{type}=#{CGI.escape(data)}"
+        url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
+        url_string << "&SigAlg=#{CGI.escape(sig_alg)}"
+      end
 
+      def validate_doc_params_signature(base64_cert, soft = true, params)
+        document_type = request? ? :SAMLRequest : :SAMLResponse
+        canon_string = build_query(
+          type: document_type,
+          data: params[document_type.to_sym],
+          relay_state: params[:RelayState],
+          sig_alg: params[:SigAlg]
+        )
+        verify_signature(base64_cert, params[:SigAlg], Base64.decode64(params[:Signature]), canon_string, soft)
+      end
+
+      def validate_doc_embedded_signature(base64_cert, soft = true)
         # check for inclusive namespaces
         inclusive_namespaces = extract_inclusive_namespaces
 
@@ -120,13 +173,15 @@ def validate_doc(base64_cert, soft = true)
 
         base64_signature        = REXML::XPath.first(@sig_element, "//ds:SignatureValue", {"ds"=>DSIG}).text
         signature               = Base64.decode64(base64_signature)
+        sig_alg                 = REXML::XPath.first(signed_info_element, "//ds:SignatureMethod", {"ds"=>DSIG})
 
-        # get certificate object
-        cert_text               = Base64.decode64(base64_cert)
-        cert                    = OpenSSL::X509::Certificate.new(cert_text)
+        verify_signature(base64_cert, sig_alg, signature, canon_string, soft)
+      end
 
-        # signature method
-        signature_algorithm     = algorithm(REXML::XPath.first(signed_info_element, "//ds:SignatureMethod", {"ds"=>DSIG}))
+      def verify_signature(base64_cert, sig_alg, signature, canon_string, soft)
+        cert_text           = Base64.decode64(base64_cert)
+        cert                = OpenSSL::X509::Certificate.new(cert_text)
+        signature_algorithm = algorithm(sig_alg)
 
         unless cert.public_key.verify(signature_algorithm.new, signature, canon_string)
           return soft ? false : (raise ValidationError.new("Key validation error"))
@@ -135,8 +190,6 @@ def validate_doc(base64_cert, soft = true)
         return true
       end
 
-      private
-
       def digests_match?(hash, digest_value)
         hash == digest_value
       end
@@ -157,8 +210,11 @@ def canon_algorithm(element)
       end
 
       def algorithm(element)
-        algorithm = element.attribute("Algorithm").value if element
-        algorithm = algorithm && algorithm =~ /sha(.*?)$/i && $1.to_i
+        algorithm = element
+        if algorithm.is_a?(REXML::Element)
+          algorithm = element.attribute("Algorithm").value
+        end
+        algorithm = algorithm && algorithm =~ /(rsa-)?sha(.*?)$/i && $2.to_i
         case algorithm
         when 256 then OpenSSL::Digest::SHA256
         when 384 then OpenSSL::Digest::SHA384

spec/lib/saml_idp/controller_spec.rb

@@ -18,6 +18,26 @@ def params
     saml_acs_url.should == requested_saml_acs_url
   end
 
+  context "SP-initiated logout" do
+    it "should respect Logout Request signed w/o embed" do
+      SamlIdp.configure do |config|
+        config.service_provider.finder = lambda do |_|
+          {
+            cert: SamlIdp::Default::X509_CERTIFICATE,
+            private_key: SamlIdp::Default::SECRET_KEY,
+            fingerprint: SamlIdp::Default::FINGERPRINT,
+            assertion_consumer_logout_service_url: 'http://foo.example.com/sp-initiated/slo'
+          }
+        end
+      end
+      request_url = URI.parse(make_sp_logout_request)
+      params.merge!(Rack::Utils.parse_nested_query(request_url.query)).symbolize_keys!
+      decode_request(params[:SAMLRequest])
+      expect(saml_request.logout_request?).to eq true
+      expect(valid_saml_request?).to eq true
+    end
+  end
+
   context "SAML Responses" do
     before(:each) do
       params[:SAMLRequest] = make_saml_request

spec/support/saml_request_macros.rb

@@ -19,13 +19,29 @@ def make_saml_logout_request(requested_saml_logout_url = 'https://foo.example.co
     request_builder.encoded
   end
 
+  def make_sp_logout_request(requested_saml_logout_url = 'https://foo.example.com/saml/logout')
+    settings = saml_settings.dup
+    settings.assertion_consumer_logout_service_url = requested_saml_logout_url
+    settings.name_identifier_value = 'some-user-id'
+    OneLogin::RubySaml::Logoutrequest.new.create(settings)
+  end
+
   def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume")
     settings = OneLogin::RubySaml::Settings.new
     settings.assertion_consumer_service_url = saml_acs_url
     settings.issuer = "http://example.com/issuer"
     settings.idp_sso_target_url = "http://idp.com/saml/idp"
+    settings.idp_slo_target_url = "http://idp.com/saml/idp-slo"
     settings.idp_cert_fingerprint = SamlIdp::Default::FINGERPRINT
     settings.name_identifier_format = SamlIdp::Default::NAME_ID_FORMAT
+    settings.certificate = SamlIdp::Default::X509_CERTIFICATE
+    settings.private_key = SamlIdp::Default::SECRET_KEY
+    settings.security = {
+      embed_sign: false,
+      logout_requests_signed: true,
+      digest_method: 'http://www.w3.org/2001/04/xmlenc#sha256',
+      signature_method: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'
+    }
     settings
   end
 

spec/xml_security_spec.rb

@@ -48,7 +48,7 @@ module SamlIdp
       expect { document.validate("a fingerprint", false) }.to(
         raise_error(
           SamlIdp::XMLSecurity::SignedDocument::ValidationError,
-          "Certificate element missing in response (ds:X509Certificate)"
+          "Certificate element missing in response (ds:X509Certificate) and not provided in options[:cert]"
         )
       )
     end