diff --git a/README.md b/README.md index dd4756afd..40edd756e 100644 --- a/README.md +++ b/README.md @@ -391,8 +391,8 @@ The following attributes are set: * idp_sso_target_url * idp_slo_target_url * idp_attribute_names - * idp_cert - * idp_cert_fingerprint + * idp_cert + * idp_cert_fingerprint * idp_cert_multi ### Retrieve one Entity Descriptor when many exist in Metadata @@ -561,6 +561,8 @@ The settings related to sign are stored in the `security` attribute of the setti # Embeded signature or HTTP GET parameter signature # Note that metadata signature is always embedded regardless of this value. settings.security[:embed_sign] = false + settings.security[:check_idp_cert_expiration] = false # Enable or not IdP x509 cert expiration check + settings.security[:check_sp_cert_expiration] = false # Enable or not SP x509 cert expiration check ``` Notice that the RelayState parameter is used when creating the Signature on the HTTP-Redirect Binding. diff --git a/lib/onelogin/ruby-saml/logging.rb b/lib/onelogin/ruby-saml/logging.rb index e2f78c9da..862fcefec 100644 --- a/lib/onelogin/ruby-saml/logging.rb +++ b/lib/onelogin/ruby-saml/logging.rb @@ -8,9 +8,9 @@ class Logging def self.logger @logger ||= begin - (defined?(::Rails) && Rails.respond_to?(:logger) && Rails.logger) || - DEFAULT_LOGGER - end + (defined?(::Rails) && Rails.respond_to?(:logger) && Rails.logger) || + DEFAULT_LOGGER + end end def self.logger=(logger) diff --git a/lib/onelogin/ruby-saml/logoutresponse.rb b/lib/onelogin/ruby-saml/logoutresponse.rb index a3e7844cc..798891435 100644 --- a/lib/onelogin/ruby-saml/logoutresponse.rb +++ b/lib/onelogin/ruby-saml/logoutresponse.rb @@ -228,13 +228,19 @@ def validate_signature :raw_sig_alg => options[:raw_get_params]['SigAlg'] ) + expired = false if idp_certs.nil? || idp_certs[:signing].empty? valid = OneLogin::RubySaml::Utils.verify_signature( - :cert => settings.get_idp_cert, + :cert => idp_cert, :sig_alg => options[:get_params]['SigAlg'], :signature => options[:get_params]['Signature'], :query_string => query_string ) + if valid && settings.security[:check_idp_cert_expiration] + if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert) + expired = true + end + end else valid = false idp_certs[:signing].each do |signing_idp_cert| @@ -245,11 +251,20 @@ def validate_signature :query_string => query_string ) if valid + if settings.security[:check_idp_cert_expiration] + if OneLogin::RubySaml::Utils.is_cert_expired(signing_idp_cert) + expired = true + end + end break end end end + if expired + error_msg = "IdP x509 certificate expired" + return append_error(error_msg) + end unless valid error_msg = "Invalid Signature on Logout Response" return append_error(error_msg) diff --git a/lib/onelogin/ruby-saml/response.rb b/lib/onelogin/ruby-saml/response.rb index 2af76ea4d..315007db8 100644 --- a/lib/onelogin/ruby-saml/response.rb +++ b/lib/onelogin/ruby-saml/response.rb @@ -829,24 +829,43 @@ def validate_signature return append_error(error_msg) end + idp_certs = settings.get_idp_cert_multi if idp_certs.nil? || idp_certs[:signing].empty? opts = {} opts[:fingerprint_alg] = settings.idp_cert_fingerprint_algorithm - opts[:cert] = settings.get_idp_cert + idp_cert = settings.get_idp_cert fingerprint = settings.get_fingerprint + opts[:cert] = idp_cert - unless fingerprint && doc.validate_document(fingerprint, @soft, opts) + if fingerprint && doc.validate_document(fingerprint, @soft, opts) + if settings.security[:check_idp_cert_expiration] + if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert) + error_msg = "IdP x509 certificate expired" + return append_error(error_msg) + end + end + else return append_error(error_msg) end else valid = false + expired = false idp_certs[:signing].each do |idp_cert| valid = doc.validate_document_with_cert(idp_cert) if valid + if settings.security[:check_idp_cert_expiration] + if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert) + expired = true + end + end break end end + if expired + error_msg = "IdP x509 certificate expired" + return append_error(error_msg) + end unless valid return append_error(error_msg) end diff --git a/lib/onelogin/ruby-saml/settings.rb b/lib/onelogin/ruby-saml/settings.rb index 23f7b252b..c5a40caf0 100644 --- a/lib/onelogin/ruby-saml/settings.rb +++ b/lib/onelogin/ruby-saml/settings.rb @@ -1,6 +1,7 @@ require "xml_security" require "onelogin/ruby-saml/attribute_service" require "onelogin/ruby-saml/utils" +require "onelogin/ruby-saml/validation_error" # Only supports SAML 2.0 module OneLogin @@ -188,7 +189,15 @@ def get_sp_cert return nil if certificate.nil? || certificate.empty? formatted_cert = OneLogin::RubySaml::Utils.format_cert(certificate) - OpenSSL::X509::Certificate.new(formatted_cert) + cert = OpenSSL::X509::Certificate.new(formatted_cert) + + if security[:check_sp_cert_expiration] + if OneLogin::RubySaml::Utils.is_cert_expired(cert) + raise OneLogin::RubySaml::ValidationError.new("The SP certificate expired.") + end + end + + cert end # @return [OpenSSL::X509::Certificate|nil] Build the New SP certificate from the settings (previously format it) @@ -218,6 +227,7 @@ def get_sp_key :compress_request => true, :compress_response => true, :soft => true, + :double_quote_xml_attribute_values => false, :security => { :authn_requests_signed => false, :logout_requests_signed => false, @@ -228,9 +238,10 @@ def get_sp_key :metadata_signed => false, :embed_sign => false, :digest_method => XMLSecurity::Document::SHA1, - :signature_method => XMLSecurity::Document::RSA_SHA1 - }.freeze, - :double_quote_xml_attribute_values => false, + :signature_method => XMLSecurity::Document::RSA_SHA1, + :check_idp_cert_expiration => false, + :check_sp_cert_expiration => false + }.freeze }.freeze end end diff --git a/lib/onelogin/ruby-saml/slo_logoutrequest.rb b/lib/onelogin/ruby-saml/slo_logoutrequest.rb index 943408489..fac38f337 100644 --- a/lib/onelogin/ruby-saml/slo_logoutrequest.rb +++ b/lib/onelogin/ruby-saml/slo_logoutrequest.rb @@ -280,13 +280,19 @@ def validate_signature :raw_sig_alg => options[:raw_get_params]['SigAlg'] ) + expired = false if idp_certs.nil? || idp_certs[:signing].empty? valid = OneLogin::RubySaml::Utils.verify_signature( - :cert => settings.get_idp_cert, + :cert => idp_cert, :sig_alg => options[:get_params]['SigAlg'], :signature => options[:get_params]['Signature'], :query_string => query_string ) + if valid && settings.security[:check_idp_cert_expiration] + if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert) + expired = true + end + end else valid = false idp_certs[:signing].each do |signing_idp_cert| @@ -297,11 +303,20 @@ def validate_signature :query_string => query_string ) if valid + if settings.security[:check_idp_cert_expiration] + if OneLogin::RubySaml::Utils.is_cert_expired(signing_idp_cert) + expired = true + end + end break end end end + if expired + error_msg = "IdP x509 certificate expired" + return append_error(error_msg) + end unless valid return append_error("Invalid Signature on Logout Request") end diff --git a/lib/onelogin/ruby-saml/utils.rb b/lib/onelogin/ruby-saml/utils.rb index 12d4cee92..0e9619d61 100644 --- a/lib/onelogin/ruby-saml/utils.rb +++ b/lib/onelogin/ruby-saml/utils.rb @@ -3,6 +3,7 @@ else require 'securerandom' end +require "openssl" module OneLogin module RubySaml @@ -15,6 +16,18 @@ class Utils DSIG = "http://www.w3.org/2000/09/xmldsig#" XENC = "http://www.w3.org/2001/04/xmlenc#" + # Checks if the x509 cert provided is expired + # + # @param cert [Certificate] The x509 certificate + # + def self.is_cert_expired(cert) + if cert.is_a?(String) + cert = OpenSSL::X509::Certificate.new(cert) + end + + return cert.not_after < Time.now + end + # Return a properly formatted x509 certificate # # @param cert [String] The original certificate diff --git a/test/logoutresponse_test.rb b/test/logoutresponse_test.rb index cd7656bde..4f4a300d4 100644 --- a/test/logoutresponse_test.rb +++ b/test/logoutresponse_test.rb @@ -140,7 +140,7 @@ class RubySamlTest < Minitest::Test assert_includes logoutresponse.errors, "Doesn't match the issuer, expected: <#{logoutresponse.settings.idp_entity_id}>, but was: " end - it "collect errors when collect_errors=true" do + it "collect errors when collect_errors=true" do settings.idp_entity_id = 'http://invalid.issuer.example.com/' logoutresponse = OneLogin::RubySaml::Logoutresponse.new(unsuccessful_logout_response_document, settings) collect_errors = true @@ -185,7 +185,7 @@ class RubySamlTest < Minitest::Test opts = { :matches_request_id => expected_request_id} logoutresponse = OneLogin::RubySaml::Logoutresponse.new(valid_logout_response_document, settings, opts) - assert_raises(OneLogin::RubySaml::ValidationError) { logoutresponse.validate } + assert_raises(OneLogin::RubySaml::ValidationError) { logoutresponse.validate } assert_includes logoutresponse.errors, "The InResponseTo of the Logout Response: #{logoutresponse.in_response_to}, does not match the ID of the Logout Request sent by the SP: #{expected_request_id}" end @@ -394,6 +394,21 @@ class RubySamlTest < Minitest::Test assert logoutresponse_sign_test.send(:validate_signature) end + it "return false when cert expired and check_idp_cert_expiration expired" do + params['RelayState'] = params[:RelayState] + options = {} + options[:get_params] = params + settings.security[:check_idp_cert_expiration] = true + settings.idp_cert = nil + settings.idp_cert_multi = { + :signing => [ruby_saml_cert_text], + :encryption => [] + } + logoutresponse_sign_test = OneLogin::RubySaml::Logoutresponse.new(params['SAMLResponse'], settings, options) + assert !logoutresponse_sign_test.send(:validate_signature) + assert_includes logoutresponse_sign_test.errors, "IdP x509 certificate expired" + end + it "return false when none cert on idp_cert_multi is valid" do params['RelayState'] = params[:RelayState] options = {} @@ -404,6 +419,7 @@ class RubySamlTest < Minitest::Test } logoutresponse_sign_test = OneLogin::RubySaml::Logoutresponse.new(params['SAMLResponse'], settings, options) assert !logoutresponse_sign_test.send(:validate_signature) + assert_includes logoutresponse_sign_test.errors, "Invalid Signature on Logout Response" end end end diff --git a/test/response_test.rb b/test/response_test.rb index 251efb72a..f0671c185 100644 --- a/test/response_test.rb +++ b/test/response_test.rb @@ -880,6 +880,15 @@ def generate_audience_error(expected, actual) assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response" end + it "return false when cert expired and check_idp_cert_expiration enabled" do + settings.idp_cert_fingerprint = nil + settings.idp_cert = ruby_saml_cert_text + settings.security[:check_idp_cert_expiration] = true + response_valid_signed.settings = settings + assert !response_valid_signed.send(:validate_signature) + assert_includes response_valid_signed.errors, "IdP x509 certificate expired" + 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 diff --git a/test/settings_test.rb b/test/settings_test.rb index aaff12d08..e2a3b29bc 100644 --- a/test/settings_test.rb +++ b/test/settings_test.rb @@ -1,6 +1,7 @@ require File.expand_path(File.join(File.dirname(__FILE__), "test_helper")) require 'onelogin/ruby-saml/settings' +require 'onelogin/ruby-saml/validation_error' class SettingsTest < Minitest::Test @@ -243,6 +244,13 @@ class SettingsTest < Minitest::Test } end + it "raises an error if SP certificate expired and check_sp_cert_expiration enabled" do + @settings.certificate = ruby_saml_cert_text + @settings.security[:check_sp_cert_expiration] = true + assert_raises(OneLogin::RubySaml::ValidationError) { + settings.get_sp_cert + } + end end describe "#get_sp_cert_new" do diff --git a/test/slo_logoutrequest_test.rb b/test/slo_logoutrequest_test.rb index 661a00e06..f36997ff2 100644 --- a/test/slo_logoutrequest_test.rb +++ b/test/slo_logoutrequest_test.rb @@ -429,6 +429,23 @@ class RubySamlTest < Minitest::Test assert logout_request_sign_test.send(:validate_signature) end + it "return false when cert expired and check_idp_cert_expiration expired" do + params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings, :RelayState => 'http://example.com') + params['RelayState'] = params[:RelayState] + options = {} + options[:get_params] = params + settings.security[:check_idp_cert_expiration] = true + logout_request_sign_test = OneLogin::RubySaml::SloLogoutrequest.new(params['SAMLRequest'], options) + settings.idp_cert = nil + settings.idp_cert_multi = { + :signing => [ruby_saml_cert_text], + :encryption => [] + } + logout_request_sign_test.settings = settings + assert !logout_request_sign_test.send(:validate_signature) + assert_includes logout_request_sign_test.errors, "IdP x509 certificate expired" + end + it "return false when none cert on idp_cert_multi is valid" do params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings, :RelayState => 'http://example.com') params['RelayState'] = params[:RelayState] @@ -442,6 +459,8 @@ class RubySamlTest < Minitest::Test } logout_request_sign_test.settings = settings assert !logout_request_sign_test.send(:validate_signature) + assert_includes logout_request_sign_test.errors, "Invalid Signature on Logout Request" + end end end