Allow SP config force signature validation

lib/saml_idp/request.rb

@@ -128,13 +128,16 @@ def valid?(external_attributes = {})
 
     def valid_signature?
       # Force signatures for logout requests because there is no other protection against a cross-site DoS.
-      # Validate signature when metadata specify AuthnRequest should be signed
-      metadata = service_provider.current_metadata
-      require_signature = logout_request? || authn_request? && metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request?
-      service_provider.valid_signature?(document, require_signature)
+      if logout_request? || authn_request? && validate_auth_request_signature?
+        document.valid_signature?(service_provider.cert, service_provider.fingerprint)
+      else
+        true
+      end
     end
 
     def valid_external_signature?
+      return true if authn_request? && !validate_auth_request_signature?
+
       cert = OpenSSL::X509::Certificate.new(service_provider.cert)
 
       sha_version = sig_algorithm =~ /sha(.*?)$/i && $1.to_i
@@ -229,5 +232,14 @@ def service_provider_finder
       config.service_provider.finder
     end
     private :service_provider_finder
+
+    def validate_auth_request_signature?
+      # Validate signature when metadata specify AuthnRequest should be signed
+      metadata = service_provider.current_metadata
+      sign_authn_request = metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request?
+      sign_authn_request = service_provider.sign_authn_request unless service_provider.sign_authn_request.nil? 
+      sign_authn_request
+    end
+    private :validate_auth_request_signature?
   end
 end

lib/saml_idp/service_provider.rb

@@ -11,6 +11,7 @@ class ServiceProvider
     attribute :fingerprint
     attribute :metadata_url
     attribute :validate_signature
+    attribute :sign_authn_request
     attribute :acs_url
     attribute :assertion_consumer_logout_service_url
     attribute :response_hosts
@@ -23,7 +24,7 @@ def valid?
 
     def valid_signature?(doc, require_signature = false)
       if require_signature || attributes[:validate_signature]
-        doc.valid_signature?(cert, fingerprint)
+        doc.valid_signature?(fingerprint)
       else
         true
       end

spec/lib/saml_idp/controller_spec.rb

@@ -128,7 +128,7 @@ def params
   context "Single Logout Request" do
     before do
       idp_configure("https://foo.example.com/saml/consume", true)
-      slo_request = make_saml_sp_slo_request
+      slo_request = make_saml_sp_slo_request(security_options: { embed_sign: false })
       params[:SAMLRequest] = slo_request['SAMLRequest']
       params[:RelayState] = slo_request['RelayState']
       params[:SigAlg] = slo_request['SigAlg']

spec/lib/saml_idp/request_spec.rb

@@ -119,6 +119,140 @@ def info(msg); end
           end
         end
       end
+
+      context "when signature provided in authn request" do
+        let(:auth_request) { OneLogin::RubySaml::Authrequest.new }
+        let(:sp_setting) { saml_settings("https://foo.example.com/saml/consume", true) }
+        let(:raw_authn_request) { CGI.unescape(auth_request.create(sp_setting).split("=").last) }
+
+        subject { described_class.from_deflated_request raw_authn_request }
+
+        before do
+          idp_configure("http://localhost:3000/saml/consume", true)
+        end
+
+        context "when AuthnRequest signature validation is enabled" do
+          before do
+            SamlIdp.configure do |config|
+              config.service_provider.finder = lambda { |_issuer_or_entity_id|
+                {
+                  response_hosts: [URI("http://localhost:3000/saml/consume").host],
+                  acs_url: "http://localhost:3000/saml/consume",
+                  cert: sp_x509_cert,
+                  fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert),
+                  assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout',
+                  sign_authn_request: true
+                }
+              }
+            end
+          end
+
+          it "returns true" do
+            expect(subject.send(:validate_auth_request_signature?)).to be true
+          end
+
+          it "validates the signature" do
+            allow(subject).to receive(:signature).and_return(nil)
+            allow(subject).to receive(:valid_signature?).and_call_original
+
+            expect(subject.valid_signature?).to be true
+          end
+        end
+
+        context "when AuthnRequest signature validation is disabled" do
+          before do
+            SamlIdp.configure do |config|
+              config.service_provider.finder = lambda { |_issuer_or_entity_id|
+                {
+                  response_hosts: [URI("http://localhost:3000/saml/consume").host],
+                  acs_url: "http://localhost:3000/saml/consume",
+                  cert: sp_x509_cert,
+                  fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert),
+                  assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout',
+                  sign_authn_request: false
+                }
+              }
+            end
+          end
+
+          it "returns false" do
+            expect(subject.send(:validate_auth_request_signature?)).to be false
+          end
+
+          it "does not validate the signature and return true" do
+            allow(subject).to receive(:signature).and_return(nil)
+            allow(subject).to receive(:valid_signature?).and_call_original
+
+            expect(subject.valid_signature?).to be true
+          end
+        end
+      end
+
+      context "when signature provided as external params" do
+        let(:auth_request) { OneLogin::RubySaml::Authrequest.new }
+        let(:sp_setting) { saml_settings("https://foo.example.com/saml/consume", true, security_options: { embed_sign: false }) }
+        let(:saml_response) { auth_request.create(sp_setting) }
+        let(:query_params) { CGI.parse(URI.parse(saml_response).query) }
+        let(:raw_authn_request) { query_params['SAMLRequest'].first }
+        let(:signature) { query_params['Signature'].first }
+        let(:sig_algorithm) { query_params['SigAlg'].first }
+
+        before do
+          idp_configure("http://localhost:3000/saml/consume", true)
+        end
+
+        subject do
+          described_class.from_deflated_request(
+            raw_authn_request,
+            saml_request: raw_authn_request,
+            relay_state: query_params['RelayState'].first,
+            sig_algorithm: sig_algorithm,
+            signature: signature
+          )
+        end
+
+        context "when AuthnRequest signature validation is enabled" do
+          before do
+            SamlIdp.configure do |config|
+              config.service_provider.finder = lambda { |_issuer_or_entity_id|
+                {
+                  response_hosts: [URI("http://localhost:3000/saml/consume").host],
+                  acs_url: "http://localhost:3000/saml/consume",
+                  cert: sp_x509_cert,
+                  fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert),
+                  assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout',
+                  sign_authn_request: true
+                }
+              }
+            end
+          end
+
+          it "validate certificates and return valid" do
+            expect(subject.valid_external_signature?).to be true
+          end
+        end
+
+        context "when AuthnRequest signature validation is disabled" do
+          before do
+            SamlIdp.configure do |config|
+              config.service_provider.finder = lambda { |_issuer_or_entity_id|
+                {
+                  response_hosts: [URI("http://localhost:3000/saml/consume").host],
+                  acs_url: "http://localhost:3000/saml/consume",
+                  cert: sp_x509_cert,
+                  fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert),
+                  assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout',
+                  sign_authn_request: false
+                }
+              }
+            end
+          end
+
+          it "skip signature validation and return valid" do
+            expect(subject.valid_external_signature?).to be true
+          end
+        end
+      end
     end
 
     describe "logout request" do
@@ -157,7 +291,7 @@ def info(msg); end
       end
 
       context 'when signature provided as external param' do
-        let!(:uri_query) { make_saml_sp_slo_request }
+        let!(:uri_query) { make_saml_sp_slo_request(security_options: { embed_sign: false }) }
         let(:raw_saml_request) { uri_query['SAMLRequest'] }
         let(:relay_state) { uri_query['RelayState'] }
         let(:siging_algorithm) { uri_query['SigAlg'] }

spec/support/saml_request_macros.rb

@@ -18,10 +18,9 @@ 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)
+  def make_saml_sp_slo_request(param_type: true, security_options: {})
     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)
+    saml_sp_setting = saml_settings("https://foo.example.com/saml/consume", true, security_options: security_options)
     if param_type
       logout_request.create_params(saml_sp_setting, 'RelayState' => 'https://foo.example.com/home')
     else
@@ -34,7 +33,7 @@ def generate_sp_metadata(saml_acs_url = "https://foo.example.com/saml/consume",
     sp_metadata.generate(saml_settings(saml_acs_url, enable_secure_options), true)
   end
 
-  def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false)
+  def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false, security_options: {})
     settings = OneLogin::RubySaml::Settings.new
     settings.assertion_consumer_service_url = saml_acs_url
     settings.issuer = "http://example.com/issuer"
@@ -43,28 +42,22 @@ def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_
     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
-    add_securty_options(settings) if enable_secure_options
+    add_securty_options(settings, default_sp_security_options.merge!(security_options)) if enable_secure_options
     settings
   end
 
-  def add_securty_options(settings, authn_requests_signed: true, 
-                                    embed_sign: true, 
-                                    logout_requests_signed: true, 
-                                    logout_responses_signed: true,
-                                    digest_method: XMLSecurity::Document::SHA256,
-                                    signature_method: XMLSecurity::Document::RSA_SHA256,
-                                    assertions_signed: true)
+  def add_securty_options(settings, options = default_sp_security_options)
     # Security section
     settings.idp_cert = SamlIdp::Default::X509_CERTIFICATE
     # Signed embedded singature
-    settings.security[:authn_requests_signed] = authn_requests_signed
-    settings.security[:embed_sign] = embed_sign
-    settings.security[:logout_requests_signed] = logout_requests_signed
-    settings.security[:logout_responses_signed] = logout_responses_signed
-    settings.security[:metadata_signed] = digest_method
-    settings.security[:digest_method] = digest_method
-    settings.security[:signature_method] = signature_method
-    settings.security[:want_assertions_signed] = assertions_signed
+    settings.security[:authn_requests_signed] = options[:authn_requests_signed]
+    settings.security[:embed_sign] = options[:embed_sign]
+    settings.security[:logout_requests_signed] = options[:logout_requests_signed]
+    settings.security[:logout_responses_signed] = options[:logout_responses_signed]
+    settings.security[:metadata_signed] = options[:digest_method]
+    settings.security[:digest_method] = options[:digest_method]
+    settings.security[:signature_method] = options[:signature_method]
+    settings.security[:want_assertions_signed] = options[:assertions_signed]
     settings.private_key = sp_pv_key
     settings.certificate = sp_x509_cert
   end
@@ -109,4 +102,16 @@ def print_pretty_xml(xml_string)
     doc.write(outbuf, 1)
     puts outbuf
   end
+
+  def default_sp_security_options
+    {
+      authn_requests_signed: true, 
+      embed_sign: true, 
+      logout_requests_signed: true, 
+      logout_responses_signed: true,
+      digest_method: XMLSecurity::Document::SHA256,
+      signature_method: XMLSecurity::Document::RSA_SHA256,
+      assertions_signed: true
+    }
+  end
 end

spec/support/security_helpers.rb

@@ -51,7 +51,7 @@ def signature_fingerprint_1
     @signature_fingerprint1 ||= "C5:19:85:D9:47:F1:BE:57:08:20:25:05:08:46:EB:27:F6:CA:B7:83"
   end
 
-  def signature_1
+  def certificate_1
     @signature1 ||= File.read(File.join(File.dirname(__FILE__), 'certificates', 'certificate1'))
   end