Skip to content

Commit 3c9dd5e

Browse files
committed
#220 Support SAMLResponse without ds:x509certificate
1 parent 3736861 commit 3c9dd5e

File tree

6 files changed

+69
-22
lines changed

6 files changed

+69
-22
lines changed

lib/onelogin/ruby-saml/response.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,7 @@ def validate_subject_confirmation
574574
#
575575
def validate_signature
576576
fingerprint = settings.get_fingerprint
577+
idp_cert = settings.get_idp_cert
577578

578579
# If the response contains the signature, and the assertion was encrypted, validate the original SAML Response
579580
# otherwise, review if the decrypted assertion contains a signature
@@ -585,7 +586,11 @@ def validate_signature
585586
)
586587
doc = (response_signed || decrypted_document.nil?) ? document : decrypted_document
587588

588-
unless fingerprint && doc.validate_document(fingerprint, :fingerprint_alg => settings.idp_cert_fingerprint_algorithm)
589+
opts = {}
590+
opts[:fingerprint_alg] = settings.idp_cert_fingerprint_algorithm
591+
opts[:cert] = idp_cert
592+
593+
unless fingerprint && doc.validate_document(fingerprint, @soft, opts)
589594
error_msg = "Invalid Signature on SAML Response"
590595
return append_error(error_msg)
591596
end

lib/xml_security.rb

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -199,30 +199,35 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {})
199199
"//ds:X509Certificate",
200200
{ "ds"=>DSIG }
201201
)
202-
unless cert_element
203-
if soft
204-
return false
202+
203+
if cert_element
204+
base64_cert = cert_element.text
205+
cert_text = Base64.decode64(base64_cert)
206+
cert = OpenSSL::X509::Certificate.new(cert_text)
207+
208+
if options[:fingerprint_alg]
209+
fingerprint_alg = XMLSecurity::BaseDocument.new.algorithm(options[:fingerprint_alg]).new
205210
else
206-
raise OneLogin::RubySaml::ValidationError.new("Certificate element missing in response (ds:X509Certificate)")
211+
fingerprint_alg = OpenSSL::Digest::SHA1.new
207212
end
208-
end
209-
base64_cert = cert_element.text
210-
cert_text = Base64.decode64(base64_cert)
211-
cert = OpenSSL::X509::Certificate.new(cert_text)
213+
fingerprint = fingerprint_alg.hexdigest(cert.to_der)
212214

213-
if options[:fingerprint_alg]
214-
fingerprint_alg = XMLSecurity::BaseDocument.new.algorithm(options[:fingerprint_alg]).new
215+
# check cert matches registered idp cert
216+
if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
217+
@errors << "Fingerprint mismatch"
218+
return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch"))
219+
end
215220
else
216-
fingerprint_alg = OpenSSL::Digest::SHA1.new
217-
end
218-
fingerprint = fingerprint_alg.hexdigest(cert.to_der)
219-
220-
# check cert matches registered idp cert
221-
if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
222-
@errors << "Fingerprint mismatch"
223-
return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch"))
221+
if options[:cert]
222+
base64_cert = Base64.encode64(options[:cert].to_pem)
223+
else
224+
if soft
225+
return false
226+
else
227+
raise OneLogin::RubySaml::ValidationError.new("Certificate element missing in response (ds:X509Certificate) and not cert provided at settings")
228+
end
229+
end
224230
end
225-
226231
validate_signature(base64_cert, soft)
227232
end
228233

test/response_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class RubySamlTest < Minitest::Test
1515
let(:response_wrapped) { OneLogin::RubySaml::Response.new(response_document_wrapped) }
1616
let(:response_multiple_attr_values) { OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values)) }
1717
let(:response_valid_signed) { OneLogin::RubySaml::Response.new(response_document_valid_signed) }
18+
let(:response_valid_signed_without_x509certificate) { OneLogin::RubySaml::Response.new(response_document_valid_signed_without_x509certificate) }
1819
let(:response_no_id) { OneLogin::RubySaml::Response.new(read_invalid_response("no_id.xml.base64")) }
1920
let(:response_no_version) { OneLogin::RubySaml::Response.new(read_invalid_response("no_saml2.xml.base64")) }
2021
let(:response_multi_assertion) { OneLogin::RubySaml::Response.new(read_invalid_response("multiple_assertions.xml.base64")) }
@@ -687,6 +688,30 @@ class RubySamlTest < Minitest::Test
687688
assert !response.send(:validate_signature)
688689
assert_includes response.errors, "Invalid Signature on SAML Response"
689690
end
691+
692+
it "return false when no X509Certificate and not cert provided at settings" do
693+
settings.idp_cert_fingerprint = ruby_saml_cert_fingerprint
694+
settings.idp_cert = nil
695+
response_valid_signed_without_x509certificate.settings = settings
696+
assert !response_valid_signed_without_x509certificate.send(:validate_signature)
697+
assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response"
698+
end
699+
700+
it "return false when no X509Certificate and the cert provided at settings mismatches" do
701+
settings.idp_cert_fingerprint = nil
702+
settings.idp_cert = signature_1
703+
response_valid_signed_without_x509certificate.settings = settings
704+
assert !response_valid_signed_without_x509certificate.send(:validate_signature)
705+
assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response"
706+
end
707+
708+
it "return true when no X509Certificate and the cert provided at settings matches" do
709+
settings.idp_cert_fingerprint = nil
710+
settings.idp_cert = ruby_saml_cert_text
711+
response_valid_signed_without_x509certificate.settings = settings
712+
assert response_valid_signed_without_x509certificate.send(:validate_signature)
713+
assert_empty response_valid_signed_without_x509certificate.errors
714+
end
690715
end
691716

692717
describe "#nameid" do
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pVZdd9o4EH3fc/Y/+LiPOcayDTb4BLoU0oSWfBDTbpuXPbI0Bie25Egi0Pz6lQ04kJI03X2CGY/u3LkjjXT8fpVnxgMImXLWNZ0GMt/3/vzjWOI8K8JrkAVnEgwdxGRYOrvmQrCQY5nKkOEcZKhIGPXPx6HbQCGWEoTSUObOkuL1NYXgihOemcZo2DWLZBWTtuvjhFrQScCiXhJbtB00LdIMECIeJIi0TePrlrPG0EulXMCISYWZ0i7kNC3kW6g5RW7ouiFyb0xjCFKlDKtq1VypIrRtXBSNfEFxg3FbSm4TXe4iBw3ItsVPedf8JyFN7DVjZAWg6SDHBYtgDFbQdmMSO14ce22zV8kWVlxEr8wgNyk4g4zPUtYgPLfLIPfY3o09pjKM0pkmtxBbtamsWS6Xy8bSa3Axs12EkI06to6hMp29M3W3DGO7HuiIJbyCG2DGWUpwlj5WJZ+DmnNq9LMZF6ma5y+AO7aDSnALVsQiTpO9M+0qxVOSiuQb4fa4CoktOcfOBrHEu4YEBDACxpfrUdd899b2VyVOBWYy4SKX++bvsQL2oJtTALXktjhN8PcAD6p2bP/McZjO9C78L+JthHsC+YqzBfRGj82bSSa9/qebYezJ9gP58s32rsnRx0m3IrAbXDlqydfms21TN3i9YjL//Cnn7Ie8cu5zhoPoCE6cMXHF5/7t7cC9PBWTy1l0e1VMOVnkE3/+/ezucRrdiEDZI/vz9DJ6OBtM7oeOmDezCN0Or+yTxdlFf6DuOt+DLHO//JgtAnd8FH9U9zduZ8g6YnwX0VORHo2Cs/lq7PkBTfpRNPeP5I/gb3g4oXfnyRKGE7f/TR8i/8OH09ljMD3p1uXs8NejbM/b20y2SGlT7lsDTsGolr0+sGQVHUYLQkDKqtE/g4b97SjcHOfVS8fZsb+djyMyhxybdWz662ArrcYdgZ9m4XqMdlqOH6PEs5otz7cclPgW8RNsdShtuR44zU6bvGlw/o+xVhnRIr4FojbWhdZzNDQ+6iOB1ctCOw2n8qTUSqrQUJecZn1KRSl6T+lN/ddu/k3mNfx+5gFnSVpilN1Yn73XO0zyMAYsQJgvAw2xwsYFV5fsUvQTBaJUz0M76gXra+caSFqkUMortn/rTXMI+dmnDQUdQdPysyyzfgCtClQNc55SOpuUb6C13aULmpazQF92SqRknX7vS91wyXPQgjdghfMig6rneBO0YVyveWbvodvPyqnzqTkrTxDkWiCjMn9xoUd6J2iEF6ptHQgdMQorfZ07ftIKUBIT5DkthAKMvFaLJoD92CNNn5i7pDRVBSt1wDXI9INHz9Peq28iEpIyTruv9M+SC3qlnzy6s0Cr26HgQtWCHQA/8G3PV4tWe7ejp55M27dM718=

test/test_helper.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ def response_document_valid_signed
5252
@response_document_valid_signed ||= read_response("valid_response.xml.base64")
5353
end
5454

55+
def response_document_valid_signed_without_x509certificate
56+
@response_document_valid_signed_without_x509certificate ||= read_response("valid_response_without_x509certificate.xml.base64")
57+
end
58+
5559
def response_document_without_recipient
5660
@response_document_without_recipient ||= read_response("response_with_undefined_recipient.xml.base64")
5761
end

test/xml_security_test.rb

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,20 @@ class XmlSecurityTest < Minitest::Test
6868
assert adfs_document.validate_signature(base64cert, false)
6969
end
7070

71-
it "raise validation error when the X509Certificate is missing" do
71+
it "raise validation error when the X509Certificate is missing and no cert provided" do
7272
decoded_response.sub!(/<ds:X509Certificate>.*<\/ds:X509Certificate>/, "")
7373
mod_document = XMLSecurity::SignedDocument.new(decoded_response)
7474
exception = assert_raises(OneLogin::RubySaml::ValidationError) do
7575
mod_document.validate_document("a fingerprint", false) # The fingerprint isn't relevant to this test
7676
end
77-
assert_equal("Certificate element missing in response (ds:X509Certificate)", exception.message)
77+
assert_equal("Certificate element missing in response (ds:X509Certificate) and not cert provided at settings", exception.message)
78+
end
79+
80+
it "invalidaties when the X509Certificate is missing and the cert is provided but mismatches" do
81+
decoded_response.sub!(/<ds:X509Certificate>.*<\/ds:X509Certificate>/, "")
82+
mod_document = XMLSecurity::SignedDocument.new(decoded_response)
83+
cert = OpenSSL::X509::Certificate.new(ruby_saml_cert)
84+
assert !mod_document.validate_document("a fingerprint", true, :cert => cert) # The fingerprint isn't relevant to this test
7885
end
7986
end
8087

0 commit comments

Comments
 (0)