Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#220 Support SAMLResponse without ds:x509certificate #273

Merged
merged 1 commit into from
Oct 27, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/onelogin/ruby-saml/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ def validate_subject_confirmation
#
def validate_signature
fingerprint = settings.get_fingerprint
idp_cert = settings.get_idp_cert

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

unless fingerprint && doc.validate_document(fingerprint, :fingerprint_alg => settings.idp_cert_fingerprint_algorithm)
opts = {}
opts[:fingerprint_alg] = settings.idp_cert_fingerprint_algorithm
opts[:cert] = idp_cert

unless fingerprint && doc.validate_document(fingerprint, @soft, opts)
error_msg = "Invalid Signature on SAML Response"
return append_error(error_msg)
end
Expand Down
43 changes: 24 additions & 19 deletions lib/xml_security.rb
Original file line number Diff line number Diff line change
Expand Up @@ -199,30 +199,35 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {})
"//ds:X509Certificate",
{ "ds"=>DSIG }
)
unless cert_element
if soft
return false

if cert_element
base64_cert = cert_element.text
cert_text = Base64.decode64(base64_cert)
cert = OpenSSL::X509::Certificate.new(cert_text)

if options[:fingerprint_alg]
fingerprint_alg = XMLSecurity::BaseDocument.new.algorithm(options[:fingerprint_alg]).new
else
raise OneLogin::RubySaml::ValidationError.new("Certificate element missing in response (ds:X509Certificate)")
fingerprint_alg = OpenSSL::Digest::SHA1.new
end
end
base64_cert = cert_element.text
cert_text = Base64.decode64(base64_cert)
cert = OpenSSL::X509::Certificate.new(cert_text)
fingerprint = fingerprint_alg.hexdigest(cert.to_der)

if options[:fingerprint_alg]
fingerprint_alg = XMLSecurity::BaseDocument.new.algorithm(options[:fingerprint_alg]).new
# check cert matches registered idp cert
if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
@errors << "Fingerprint mismatch"
return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch"))
end
else
fingerprint_alg = OpenSSL::Digest::SHA1.new
end
fingerprint = fingerprint_alg.hexdigest(cert.to_der)

# check cert matches registered idp cert
if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
@errors << "Fingerprint mismatch"
return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch"))
if options[:cert]
base64_cert = Base64.encode64(options[:cert].to_pem)
else
if soft
return false
else
raise OneLogin::RubySaml::ValidationError.new("Certificate element missing in response (ds:X509Certificate) and not cert provided at settings")
end
end
end

validate_signature(base64_cert, soft)
end

Expand Down
25 changes: 25 additions & 0 deletions test/response_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class RubySamlTest < Minitest::Test
let(:response_wrapped) { OneLogin::RubySaml::Response.new(response_document_wrapped) }
let(:response_multiple_attr_values) { OneLogin::RubySaml::Response.new(fixture(:response_with_multiple_attribute_values)) }
let(:response_valid_signed) { OneLogin::RubySaml::Response.new(response_document_valid_signed) }
let(:response_valid_signed_without_x509certificate) { OneLogin::RubySaml::Response.new(response_document_valid_signed_without_x509certificate) }
let(:response_no_id) { OneLogin::RubySaml::Response.new(read_invalid_response("no_id.xml.base64")) }
let(:response_no_version) { OneLogin::RubySaml::Response.new(read_invalid_response("no_saml2.xml.base64")) }
let(:response_multi_assertion) { OneLogin::RubySaml::Response.new(read_invalid_response("multiple_assertions.xml.base64")) }
Expand Down Expand Up @@ -687,6 +688,30 @@ class RubySamlTest < Minitest::Test
assert !response.send(:validate_signature)
assert_includes response.errors, "Invalid Signature on SAML Response"
end

it "return false when no X509Certificate and not cert provided at settings" do
settings.idp_cert_fingerprint = ruby_saml_cert_fingerprint
settings.idp_cert = nil
response_valid_signed_without_x509certificate.settings = settings
assert !response_valid_signed_without_x509certificate.send(:validate_signature)
assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response"
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
response_valid_signed_without_x509certificate.settings = settings
assert !response_valid_signed_without_x509certificate.send(:validate_signature)
assert_includes response_valid_signed_without_x509certificate.errors, "Invalid Signature on SAML Response"
end

it "return true when no X509Certificate and the cert provided at settings matches" do
settings.idp_cert_fingerprint = nil
settings.idp_cert = ruby_saml_cert_text
response_valid_signed_without_x509certificate.settings = settings
assert response_valid_signed_without_x509certificate.send(:validate_signature)
assert_empty response_valid_signed_without_x509certificate.errors
end
end

describe "#nameid" do
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +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=
4 changes: 4 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ def response_document_valid_signed
@response_document_valid_signed ||= read_response("valid_response.xml.base64")
end

def response_document_valid_signed_without_x509certificate
@response_document_valid_signed_without_x509certificate ||= read_response("valid_response_without_x509certificate.xml.base64")
end

def response_document_without_recipient
@response_document_without_recipient ||= read_response("response_with_undefined_recipient.xml.base64")
end
Expand Down
11 changes: 9 additions & 2 deletions test/xml_security_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,20 @@ class XmlSecurityTest < Minitest::Test
assert adfs_document.validate_signature(base64cert, false)
end

it "raise validation error when the X509Certificate is missing" do
it "raise validation error when the X509Certificate is missing and no cert provided" do
decoded_response.sub!(/<ds:X509Certificate>.*<\/ds:X509Certificate>/, "")
mod_document = XMLSecurity::SignedDocument.new(decoded_response)
exception = assert_raises(OneLogin::RubySaml::ValidationError) do
mod_document.validate_document("a fingerprint", false) # The fingerprint isn't relevant to this test
end
assert_equal("Certificate element missing in response (ds:X509Certificate)", exception.message)
assert_equal("Certificate element missing in response (ds:X509Certificate) and not cert provided at settings", exception.message)
end

it "invalidaties when the X509Certificate is missing and the cert is provided but mismatches" do
decoded_response.sub!(/<ds:X509Certificate>.*<\/ds:X509Certificate>/, "")
mod_document = XMLSecurity::SignedDocument.new(decoded_response)
cert = OpenSSL::X509::Certificate.new(ruby_saml_cert)
assert !mod_document.validate_document("a fingerprint", true, :cert => cert) # The fingerprint isn't relevant to this test
end
end

Expand Down