[feat] External Signature param for Redirect Binding (#203)

* Squash commits for saml_idp gem

* Add explanation for external attributes of decode_request method

---------

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

README.md

@@ -24,9 +24,13 @@ Add this to your Gemfile:
 
 Include `SamlIdp::Controller` and see the examples that use rails. It should be straightforward for you.
 
-Basically you call `decode_request(params[:SAMLRequest])` on an incoming request and then use the value
-`saml_acs_url` to determine the source for which you need to authenticate a user. How you authenticate
-a user is entirely up to you.
+Basically, you call `decode_request(params[:SAMLRequest])` on an incoming request and then use the value 
+`saml_acs_url` to determine the source for which you need to authenticate a user. 
+If the signature (`Signature`) and signing algorithm (`SigAlg`) are provided as external parameters in the request,
+you can pass those parameters as `decode_request(params[:SAMLRequest], params[:Signature], params[:SigAlg], params[:RelayState])`.
+Then, you can verify the request signature with the `valid?` method.
+
+How you authenticate a user is entirely up to you.
 
 Once a user has successfully authenticated on your system send the Service Provider a SAMLResponse by
 posting to `saml_acs_url` the parameter `SAMLResponse` with the return value from a call to

lib/saml_idp.rb

@@ -70,9 +70,9 @@ def signed?
         !!xpath("//ds:Signature", ds: signature_namespace).first
       end
 
-      def valid_signature?(fingerprint)
+      def valid_signature?(certificate, fingerprint)
         signed? &&
-          signed_document.validate(fingerprint, :soft)
+          signed_document.validate(certificate, fingerprint, :soft)
       end
 
       def signed_document

lib/saml_idp/controller.rb

@@ -33,15 +33,21 @@ def acs_url
     end
 
     def validate_saml_request(raw_saml_request = params[:SAMLRequest])
-      decode_request(raw_saml_request)
+      decode_request(raw_saml_request, params[:Signature], params[:SigAlg], params[:RelayState])
       return true if valid_saml_request?
 
       head :forbidden if defined?(::Rails)
       false
     end
 
-    def decode_request(raw_saml_request)
-      @saml_request = Request.from_deflated_request(raw_saml_request)
+    def decode_request(raw_saml_request, signature, sig_algorithm, relay_state)
+      @saml_request = Request.from_deflated_request(
+        raw_saml_request,
+        saml_request: raw_saml_request,
+        signature: signature,
+        sig_algorithm: sig_algorithm,
+        relay_state: relay_state
+      )
     end
 
     def authn_context_classref

lib/saml_idp/request.rb

@@ -3,7 +3,7 @@
 require 'logger'
 module SamlIdp
   class Request
-    def self.from_deflated_request(raw)
+    def self.from_deflated_request(raw, external_attributes = {})
       if raw
         decoded = Base64.decode64(raw)
         zstream = Zlib::Inflate.new(-Zlib::MAX_WBITS)
@@ -18,18 +18,22 @@ def self.from_deflated_request(raw)
       else
         inflated = ""
       end
-      new(inflated)
+      new(inflated, external_attributes)
     end
 
-    attr_accessor :raw_xml
+    attr_accessor :raw_xml, :saml_request, :signature, :sig_algorithm, :relay_state
 
     delegate :config, to: :SamlIdp
     private :config
     delegate :xpath, to: :document
     private :xpath
 
-    def initialize(raw_xml = "")
+    def initialize(raw_xml = "", external_attributes = {})
       self.raw_xml = raw_xml
+      self.saml_request = external_attributes[:saml_request]
+      self.relay_state = external_attributes[:relay_state]
+      self.sig_algorithm = external_attributes[:sig_algorithm]
+      self.signature = external_attributes[:signature]
     end
 
     def logout_request?
@@ -85,7 +89,7 @@ def log(msg)
       end
     end
 
-    def valid?
+    def valid?(external_attributes = {})
       unless service_provider?
         log "Unable to find service provider for issuer #{issuer}"
         return false
@@ -96,8 +100,15 @@ def valid?
         return false
       end
 
-      unless valid_signature?
-        log "Signature is invalid in #{raw_xml}"
+      # XML embedded signature
+      if signature.nil? && !valid_signature?
+        log "Requested document signature is invalid in #{raw_xml}"
+        return false
+      end
+
+      # URI query signature
+      if signature.present? && !valid_external_signature?
+        log "Requested URI signature is invalid in #{raw_xml}"
         return false
       end
 
@@ -120,12 +131,29 @@ def valid_signature?
       # Validate signature when metadata specify AuthnRequest should be signed
       metadata = service_provider.current_metadata
       if logout_request? || authn_request? && metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request?
-        document.valid_signature?(service_provider.fingerprint)
+        document.valid_signature?(service_provider.cert, service_provider.fingerprint)
       else
         true
       end
     end
 
+    def valid_external_signature?
+      cert = OpenSSL::X509::Certificate.new(service_provider.cert)
+
+      sha_version = sig_algorithm =~ /sha(.*?)$/i && $1.to_i
+      raw_signature = Base64.decode64(signature)
+
+      signature_algorithm = case sha_version
+      when 256 then OpenSSL::Digest::SHA256
+      when 384 then OpenSSL::Digest::SHA384
+      when 512 then OpenSSL::Digest::SHA512
+      else
+        OpenSSL::Digest::SHA1
+      end
+
+      cert.public_key.verify(signature_algorithm.new, raw_signature, query_request_string)
+    end
+
     def service_provider?
       service_provider && service_provider.valid?
     end
@@ -148,6 +176,13 @@ def session_index
       @_session_index ||= xpath("//samlp:SessionIndex", samlp: samlp).first.try(:content)
     end
 
+    def query_request_string
+      url_string = "SAMLRequest=#{CGI.escape(saml_request)}"
+      url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
+      url_string << "&SigAlg=#{CGI.escape(sig_algorithm)}"
+    end
+    private :query_request_string
+
     def response_host
       uri = URI(response_url)
       if uri

lib/saml_idp/xml_security.rb

@@ -43,24 +43,29 @@ def initialize(response)
         extract_signed_element_id
       end
 
-      def validate(idp_cert_fingerprint, soft = true)
+      def validate(idp_base64_cert, 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)
-
-        # check cert matches registered idp cert
-        fingerprint = fingerprint_cert(cert)
-        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"))
+        if cert_element
+          idp_base64_cert = cert_element.text
+          cert_text    = Base64.decode64(idp_base64_cert)
+          cert         = OpenSSL::X509::Certificate.new(cert_text)
+
+          # check cert matches registered idp cert
+          fingerprint = fingerprint_cert(cert)
+          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
+        end
+
+        if idp_base64_cert.nil? || idp_base64_cert.empty?
+          raise ValidationError.new("Certificate validation is required, but it doesn't exist.")
         end
 
-        validate_doc(base64_cert, soft)
+        validate_doc(idp_base64_cert, soft)
       end
 
       def fingerprint_cert(cert)

spec/lib/saml_idp/controller_spec.rb

@@ -124,4 +124,19 @@ def params
       end
     end
   end
+
+  context "Single Logout Request" do
+    before do
+      idp_configure("https://foo.example.com/saml/consume", true)
+      slo_request = make_saml_sp_slo_request
+      params[:SAMLRequest] = slo_request['SAMLRequest']
+      params[:RelayState] = slo_request['RelayState']
+      params[:SigAlg] = slo_request['SigAlg']
+      params[:Signature] = slo_request['Signature']
+    end
+
+    it 'should successfully validate signature' do
+      expect(validate_saml_request).to eq(true)
+    end
+  end
 end

spec/lib/saml_idp/incoming_metadata_spec.rb

@@ -33,7 +33,6 @@ module SamlIdp
     it 'should properly set sign_assertions to false' do
       metadata = SamlIdp::IncomingMetadata.new(metadata_1)
       expect(metadata.sign_assertions).to eq(false)
-      expect(metadata.sign_authn_request).to eq(false)
     end
 
     it 'should properly set entity_id as https://test-saml.com/saml' do

spec/lib/saml_idp/metadata_builder_spec.rb

@@ -6,7 +6,7 @@ module SamlIdp
     end
 
     it "signs valid xml" do
-      expect(Saml::XML::Document.parse(subject.signed).valid_signature?(Default::FINGERPRINT)).to be_truthy
+      expect(Saml::XML::Document.parse(subject.signed).valid_signature?("", Default::FINGERPRINT)).to be_truthy
     end
 
     it "includes logout element" do

spec/lib/saml_idp/request_spec.rb

@@ -122,36 +122,68 @@ def info(msg); end
     end
 
     describe "logout request" do
-      let(:raw_logout_request) { "<LogoutRequest ID='_some_response_id' Version='2.0' IssueInstant='2010-06-01T13:00:00Z' Destination='http://localhost:3000/saml/logout' xmlns='urn:oasis:names:tc:SAML:2.0:protocol'><Issuer xmlns='urn:oasis:names:tc:SAML:2.0:assertion'>http://example.com</Issuer><NameID xmlns='urn:oasis:names:tc:SAML:2.0:assertion' Format='urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'>some_name_id</NameID><SessionIndex>abc123index</SessionIndex></LogoutRequest>" }
+      context 'when POST binding' do
+        let(:raw_logout_request) { "<LogoutRequest ID='_some_response_id' Version='2.0' IssueInstant='2010-06-01T13:00:00Z' Destination='http://localhost:3000/saml/logout' xmlns='urn:oasis:names:tc:SAML:2.0:protocol'><Issuer xmlns='urn:oasis:names:tc:SAML:2.0:assertion'>http://example.com</Issuer><NameID xmlns='urn:oasis:names:tc:SAML:2.0:assertion' Format='urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'>some_name_id</NameID><SessionIndex>abc123index</SessionIndex></LogoutRequest>" }
 
-      subject { described_class.new raw_logout_request }
+        subject { described_class.new raw_logout_request }
 
-      it "has a valid request_id" do
-        expect(subject.request_id).to eq('_some_response_id')
-      end
+        it "has a valid request_id" do
+          expect(subject.request_id).to eq('_some_response_id')
+        end
 
-      it "should be flagged as a logout_request" do
-        expect(subject.logout_request?).to eq(true)
-      end
+        it "should be flagged as a logout_request" do
+          expect(subject.logout_request?).to eq(true)
+        end
 
-      it "should have a valid name_id" do
-        expect(subject.name_id).to eq('some_name_id')
-      end
+        it "should have a valid name_id" do
+          expect(subject.name_id).to eq('some_name_id')
+        end
 
-      it "should have a session index" do
-        expect(subject.session_index).to eq('abc123index')
-      end
+        it "should have a session index" do
+          expect(subject.session_index).to eq('abc123index')
+        end
 
-      it "should have a valid issuer" do
-        expect(subject.issuer).to eq('http://example.com')
-      end
+        it "should have a valid issuer" do
+          expect(subject.issuer).to eq('http://example.com')
+        end
 
-      it "fetches internal request" do
-        expect(subject.request['ID']).to eq(subject.request_id)
+        it "fetches internal request" do
+          expect(subject.request['ID']).to eq(subject.request_id)
+        end
+
+        it "should return logout_url for response_url" do
+          expect(subject.response_url).to eq(subject.logout_url)
+        end
       end
 
-      it "should return logout_url for response_url" do
-        expect(subject.response_url).to eq(subject.logout_url)
+      context 'when signature provided as external param' do
+        let!(:uri_query) { make_saml_sp_slo_request }
+        let(:raw_saml_request) { uri_query['SAMLRequest'] }
+        let(:relay_state) { uri_query['RelayState'] }
+        let(:siging_algorithm) { uri_query['SigAlg'] }
+        let(:signature) { uri_query['Signature'] }
+
+        subject do
+          described_class.from_deflated_request(
+            raw_saml_request,
+            saml_request: raw_saml_request,
+            relay_state: relay_state,
+            sig_algorithm: siging_algorithm,
+            signature: signature
+          )
+        end
+
+        it "should validate the request" do
+          allow(ServiceProvider).to receive(:new).and_return(
+            ServiceProvider.new(
+              issuer: "http://example.com/issuer",
+              cert: sp_x509_cert,
+              response_hosts: ["example.com"],
+              assertion_consumer_logout_service_url: "http://example.com/logout"
+            )
+          )
+          expect(subject.valid?).to be true
+        end
       end
     end
   end

spec/rails_app/app/views/saml_idp/idp/new.html.erb

@@ -4,6 +4,9 @@
 <%= form_tag do %>
   <%= hidden_field_tag("SAMLRequest", params[:SAMLRequest]) %>
   <%= hidden_field_tag("RelayState", params[:RelayState]) %>
+  <%= hidden_field_tag("SigAlg", params[:SigAlg]) %>
+  <%= hidden_field_tag("Signature", params[:Signature]) %>
+
   <p>
     <%= label_tag :email %>
     <%= email_field_tag :email, params[:email], :autocapitalize => "off", :autocorrect => "off", :autofocus => "autofocus", :spellcheck => "false", :size => 30, :class => "email_pwd txt" %>

spec/support/saml_request_macros.rb

@@ -18,6 +18,17 @@ def make_saml_logout_request(requested_saml_logout_url = 'https://foo.example.co
     Base64.strict_encode64(request_builder.signed)
   end
 
+  def make_saml_sp_slo_request(param_type: true, embed_sign: false)
+    logout_request = OneLogin::RubySaml::Logoutrequest.new
+    saml_sp_setting = saml_settings("https://foo.example.com/saml/consume")
+    add_securty_options(saml_sp_setting, embed_sign: embed_sign)
+    if param_type
+      logout_request.create_params(saml_sp_setting, 'RelayState' => 'https://foo.example.com/home')
+    else
+      logout_request.create(saml_sp_setting, 'RelayState' => 'https://foo.example.com/home')
+    end
+  end
+
   def generate_sp_metadata(saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false)
     sp_metadata = OneLogin::RubySaml::Metadata.new
     sp_metadata.generate(saml_settings(saml_acs_url, enable_secure_options), true)
@@ -28,6 +39,7 @@ def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_
     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/slo"
     settings.assertion_consumer_logout_service_url = 'https://foo.example.com/saml/logout'
     settings.idp_cert_fingerprint = SamlIdp::Default::FINGERPRINT
     settings.name_identifier_format = SamlIdp::Default::NAME_ID_FORMAT
@@ -84,7 +96,8 @@ def idp_configure(saml_acs_url = "https://foo.example.com/saml/consume", enable_
           response_hosts: [URI(saml_acs_url).host],
           acs_url: saml_acs_url,
           cert: sp_x509_cert,
-          fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert)
+          fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert),
+          assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout'
         }
       }
     end