Skip to content

Commit

Permalink
Add support for certification expiration
Browse files Browse the repository at this point in the history
  • Loading branch information
Sixto Garcia authored and Sixto Garcia committed Jul 24, 2019
1 parent a22cc8d commit c652374
Show file tree
Hide file tree
Showing 11 changed files with 142 additions and 15 deletions.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions lib/onelogin/ruby-saml/logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 16 additions & 1 deletion lib/onelogin/ruby-saml/logoutresponse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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)
Expand Down
23 changes: 21 additions & 2 deletions lib/onelogin/ruby-saml/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 15 additions & 4 deletions lib/onelogin/ruby-saml/settings.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
17 changes: 16 additions & 1 deletion lib/onelogin/ruby-saml/slo_logoutrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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
Expand Down
13 changes: 13 additions & 0 deletions lib/onelogin/ruby-saml/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
else
require 'securerandom'
end
require "openssl"

module OneLogin
module RubySaml
Expand All @@ -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
Expand Down
20 changes: 18 additions & 2 deletions test/logoutresponse_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: <http://app.muda.no>"
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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 = {}
Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions test/response_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/settings_test.rb
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions test/slo_logoutrequest_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down

0 comments on commit c652374

Please sign in to comment.