Skip to content

Commit c652374

Browse files
Sixto GarciaSixto Garcia
Sixto Garcia
authored and
Sixto Garcia
committed
Add support for certification expiration
1 parent a22cc8d commit c652374

File tree

11 files changed

+142
-15
lines changed

11 files changed

+142
-15
lines changed

‎README.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,8 +391,8 @@ The following attributes are set:
391391
* idp_sso_target_url
392392
* idp_slo_target_url
393393
* idp_attribute_names
394-
* idp_cert
395-
* idp_cert_fingerprint
394+
* idp_cert
395+
* idp_cert_fingerprint
396396
* idp_cert_multi
397397
398398
### 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
561561
# Embeded signature or HTTP GET parameter signature
562562
# Note that metadata signature is always embedded regardless of this value.
563563
settings.security[:embed_sign] = false
564+
settings.security[:check_idp_cert_expiration] = false # Enable or not IdP x509 cert expiration check
565+
settings.security[:check_sp_cert_expiration] = false # Enable or not SP x509 cert expiration check
564566
```
565567
566568
Notice that the RelayState parameter is used when creating the Signature on the HTTP-Redirect Binding.

‎lib/onelogin/ruby-saml/logging.rb‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ class Logging
88

99
def self.logger
1010
@logger ||= begin
11-
(defined?(::Rails) && Rails.respond_to?(:logger) && Rails.logger) ||
12-
DEFAULT_LOGGER
13-
end
11+
(defined?(::Rails) && Rails.respond_to?(:logger) && Rails.logger) ||
12+
DEFAULT_LOGGER
13+
end
1414
end
1515

1616
def self.logger=(logger)

‎lib/onelogin/ruby-saml/logoutresponse.rb‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,19 @@ def validate_signature
228228
:raw_sig_alg => options[:raw_get_params]['SigAlg']
229229
)
230230

231+
expired = false
231232
if idp_certs.nil? || idp_certs[:signing].empty?
232233
valid = OneLogin::RubySaml::Utils.verify_signature(
233-
:cert => settings.get_idp_cert,
234+
:cert => idp_cert,
234235
:sig_alg => options[:get_params]['SigAlg'],
235236
:signature => options[:get_params]['Signature'],
236237
:query_string => query_string
237238
)
239+
if valid && settings.security[:check_idp_cert_expiration]
240+
if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
241+
expired = true
242+
end
243+
end
238244
else
239245
valid = false
240246
idp_certs[:signing].each do |signing_idp_cert|
@@ -245,11 +251,20 @@ def validate_signature
245251
:query_string => query_string
246252
)
247253
if valid
254+
if settings.security[:check_idp_cert_expiration]
255+
if OneLogin::RubySaml::Utils.is_cert_expired(signing_idp_cert)
256+
expired = true
257+
end
258+
end
248259
break
249260
end
250261
end
251262
end
252263

264+
if expired
265+
error_msg = "IdP x509 certificate expired"
266+
return append_error(error_msg)
267+
end
253268
unless valid
254269
error_msg = "Invalid Signature on Logout Response"
255270
return append_error(error_msg)

‎lib/onelogin/ruby-saml/response.rb‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -829,24 +829,43 @@ def validate_signature
829829
return append_error(error_msg)
830830
end
831831

832+
832833
idp_certs = settings.get_idp_cert_multi
833834
if idp_certs.nil? || idp_certs[:signing].empty?
834835
opts = {}
835836
opts[:fingerprint_alg] = settings.idp_cert_fingerprint_algorithm
836-
opts[:cert] = settings.get_idp_cert
837+
idp_cert = settings.get_idp_cert
837838
fingerprint = settings.get_fingerprint
839+
opts[:cert] = idp_cert
838840

839-
unless fingerprint && doc.validate_document(fingerprint, @soft, opts)
841+
if fingerprint && doc.validate_document(fingerprint, @soft, opts)
842+
if settings.security[:check_idp_cert_expiration]
843+
if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
844+
error_msg = "IdP x509 certificate expired"
845+
return append_error(error_msg)
846+
end
847+
end
848+
else
840849
return append_error(error_msg)
841850
end
842851
else
843852
valid = false
853+
expired = false
844854
idp_certs[:signing].each do |idp_cert|
845855
valid = doc.validate_document_with_cert(idp_cert)
846856
if valid
857+
if settings.security[:check_idp_cert_expiration]
858+
if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
859+
expired = true
860+
end
861+
end
847862
break
848863
end
849864
end
865+
if expired
866+
error_msg = "IdP x509 certificate expired"
867+
return append_error(error_msg)
868+
end
850869
unless valid
851870
return append_error(error_msg)
852871
end

‎lib/onelogin/ruby-saml/settings.rb‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
require "xml_security"
22
require "onelogin/ruby-saml/attribute_service"
33
require "onelogin/ruby-saml/utils"
4+
require "onelogin/ruby-saml/validation_error"
45

56
# Only supports SAML 2.0
67
module OneLogin
@@ -188,7 +189,15 @@ def get_sp_cert
188189
return nil if certificate.nil? || certificate.empty?
189190

190191
formatted_cert = OneLogin::RubySaml::Utils.format_cert(certificate)
191-
OpenSSL::X509::Certificate.new(formatted_cert)
192+
cert = OpenSSL::X509::Certificate.new(formatted_cert)
193+
194+
if security[:check_sp_cert_expiration]
195+
if OneLogin::RubySaml::Utils.is_cert_expired(cert)
196+
raise OneLogin::RubySaml::ValidationError.new("The SP certificate expired.")
197+
end
198+
end
199+
200+
cert
192201
end
193202

194203
# @return [OpenSSL::X509::Certificate|nil] Build the New SP certificate from the settings (previously format it)
@@ -218,6 +227,7 @@ def get_sp_key
218227
:compress_request => true,
219228
:compress_response => true,
220229
:soft => true,
230+
:double_quote_xml_attribute_values => false,
221231
:security => {
222232
:authn_requests_signed => false,
223233
:logout_requests_signed => false,
@@ -228,9 +238,10 @@ def get_sp_key
228238
:metadata_signed => false,
229239
:embed_sign => false,
230240
:digest_method => XMLSecurity::Document::SHA1,
231-
:signature_method => XMLSecurity::Document::RSA_SHA1
232-
}.freeze,
233-
:double_quote_xml_attribute_values => false,
241+
:signature_method => XMLSecurity::Document::RSA_SHA1,
242+
:check_idp_cert_expiration => false,
243+
:check_sp_cert_expiration => false
244+
}.freeze
234245
}.freeze
235246
end
236247
end

‎lib/onelogin/ruby-saml/slo_logoutrequest.rb‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,13 +280,19 @@ def validate_signature
280280
:raw_sig_alg => options[:raw_get_params]['SigAlg']
281281
)
282282

283+
expired = false
283284
if idp_certs.nil? || idp_certs[:signing].empty?
284285
valid = OneLogin::RubySaml::Utils.verify_signature(
285-
:cert => settings.get_idp_cert,
286+
:cert => idp_cert,
286287
:sig_alg => options[:get_params]['SigAlg'],
287288
:signature => options[:get_params]['Signature'],
288289
:query_string => query_string
289290
)
291+
if valid && settings.security[:check_idp_cert_expiration]
292+
if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
293+
expired = true
294+
end
295+
end
290296
else
291297
valid = false
292298
idp_certs[:signing].each do |signing_idp_cert|
@@ -297,11 +303,20 @@ def validate_signature
297303
:query_string => query_string
298304
)
299305
if valid
306+
if settings.security[:check_idp_cert_expiration]
307+
if OneLogin::RubySaml::Utils.is_cert_expired(signing_idp_cert)
308+
expired = true
309+
end
310+
end
300311
break
301312
end
302313
end
303314
end
304315

316+
if expired
317+
error_msg = "IdP x509 certificate expired"
318+
return append_error(error_msg)
319+
end
305320
unless valid
306321
return append_error("Invalid Signature on Logout Request")
307322
end

‎lib/onelogin/ruby-saml/utils.rb‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
else
44
require 'securerandom'
55
end
6+
require "openssl"
67

78
module OneLogin
89
module RubySaml
@@ -15,6 +16,18 @@ class Utils
1516
DSIG = "http://www.w3.org/2000/09/xmldsig#"
1617
XENC = "http://www.w3.org/2001/04/xmlenc#"
1718

19+
# Checks if the x509 cert provided is expired
20+
#
21+
# @param cert [Certificate] The x509 certificate
22+
#
23+
def self.is_cert_expired(cert)
24+
if cert.is_a?(String)
25+
cert = OpenSSL::X509::Certificate.new(cert)
26+
end
27+
28+
return cert.not_after < Time.now
29+
end
30+
1831
# Return a properly formatted x509 certificate
1932
#
2033
# @param cert [String] The original certificate

‎test/logoutresponse_test.rb‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class RubySamlTest < Minitest::Test
140140
assert_includes logoutresponse.errors, "Doesn't match the issuer, expected: <#{logoutresponse.settings.idp_entity_id}>, but was: <http://app.muda.no>"
141141
end
142142

143-
it "collect errors when collect_errors=true" do
143+
it "collect errors when collect_errors=true" do
144144
settings.idp_entity_id = 'http://invalid.issuer.example.com/'
145145
logoutresponse = OneLogin::RubySaml::Logoutresponse.new(unsuccessful_logout_response_document, settings)
146146
collect_errors = true
@@ -185,7 +185,7 @@ class RubySamlTest < Minitest::Test
185185
opts = { :matches_request_id => expected_request_id}
186186

187187
logoutresponse = OneLogin::RubySaml::Logoutresponse.new(valid_logout_response_document, settings, opts)
188-
assert_raises(OneLogin::RubySaml::ValidationError) { logoutresponse.validate }
188+
assert_raises(OneLogin::RubySaml::ValidationError) { logoutresponse.validate }
189189
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}"
190190
end
191191

@@ -394,6 +394,21 @@ class RubySamlTest < Minitest::Test
394394
assert logoutresponse_sign_test.send(:validate_signature)
395395
end
396396

397+
it "return false when cert expired and check_idp_cert_expiration expired" do
398+
params['RelayState'] = params[:RelayState]
399+
options = {}
400+
options[:get_params] = params
401+
settings.security[:check_idp_cert_expiration] = true
402+
settings.idp_cert = nil
403+
settings.idp_cert_multi = {
404+
:signing => [ruby_saml_cert_text],
405+
:encryption => []
406+
}
407+
logoutresponse_sign_test = OneLogin::RubySaml::Logoutresponse.new(params['SAMLResponse'], settings, options)
408+
assert !logoutresponse_sign_test.send(:validate_signature)
409+
assert_includes logoutresponse_sign_test.errors, "IdP x509 certificate expired"
410+
end
411+
397412
it "return false when none cert on idp_cert_multi is valid" do
398413
params['RelayState'] = params[:RelayState]
399414
options = {}
@@ -404,6 +419,7 @@ class RubySamlTest < Minitest::Test
404419
}
405420
logoutresponse_sign_test = OneLogin::RubySaml::Logoutresponse.new(params['SAMLResponse'], settings, options)
406421
assert !logoutresponse_sign_test.send(:validate_signature)
422+
assert_includes logoutresponse_sign_test.errors, "Invalid Signature on Logout Response"
407423
end
408424
end
409425
end

‎test/response_test.rb‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,15 @@ def generate_audience_error(expected, actual)
880880
assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response"
881881
end
882882

883+
it "return false when cert expired and check_idp_cert_expiration enabled" do
884+
settings.idp_cert_fingerprint = nil
885+
settings.idp_cert = ruby_saml_cert_text
886+
settings.security[:check_idp_cert_expiration] = true
887+
response_valid_signed.settings = settings
888+
assert !response_valid_signed.send(:validate_signature)
889+
assert_includes response_valid_signed.errors, "IdP x509 certificate expired"
890+
end
891+
883892
it "return false when no X509Certificate and the cert provided at settings mismatches" do
884893
settings.idp_cert_fingerprint = nil
885894
settings.idp_cert = signature_1

‎test/settings_test.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
require File.expand_path(File.join(File.dirname(__FILE__), "test_helper"))
22

33
require 'onelogin/ruby-saml/settings'
4+
require 'onelogin/ruby-saml/validation_error'
45

56
class SettingsTest < Minitest::Test
67

@@ -243,6 +244,13 @@ class SettingsTest < Minitest::Test
243244
}
244245
end
245246

247+
it "raises an error if SP certificate expired and check_sp_cert_expiration enabled" do
248+
@settings.certificate = ruby_saml_cert_text
249+
@settings.security[:check_sp_cert_expiration] = true
250+
assert_raises(OneLogin::RubySaml::ValidationError) {
251+
settings.get_sp_cert
252+
}
253+
end
246254
end
247255

248256
describe "#get_sp_cert_new" do

0 commit comments

Comments
 (0)