diff --git a/lib/xml_security.rb b/lib/xml_security.rb index 80355c482..c7225868e 100644 --- a/lib/xml_security.rb +++ b/lib/xml_security.rb @@ -227,42 +227,48 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {}) end def validate_signature(base64_cert, soft = true) - # validate references - - # check for inclusive namespaces - inclusive_namespaces = extract_inclusive_namespaces document = Nokogiri.parse(self.to_s) do |options| options = XMLSecurity::BaseDocument::NOKOGIRI_OPTIONS end - # create a working copy so we don't modify the original + # create a rexml document @working_copy ||= REXML::Document.new(self.to_s).root - # store and remove signature node - @sig_element ||= begin - element = REXML::XPath.first( + # get signature node + sig_element = REXML::XPath.first( @working_copy, "//ds:Signature", {"ds"=>DSIG} - ) - element.remove - end + ) - # verify signature - signed_info_element = REXML::XPath.first( - @sig_element, - "//ds:SignedInfo", + # signature method + sig_alg_value = REXML::XPath.first( + sig_element, + "./ds:SignedInfo/ds:SignatureMethod", {"ds"=>DSIG} ) - noko_sig_element = document.at_xpath('//ds:Signature', 'ds' => DSIG) - noko_signed_info_element = noko_sig_element.at_xpath('./ds:SignedInfo', 'ds' => DSIG) + signature_algorithm = algorithm(sig_alg_value) + + # get signature + base64_signature = REXML::XPath.first( + sig_element, + "./ds:SignatureValue", + {"ds" => DSIG} + ).text + signature = Base64.decode64(base64_signature) + + # canonicalization method canon_algorithm = canon_algorithm REXML::XPath.first( - @sig_element, - '//ds:CanonicalizationMethod', + sig_element, + './ds:SignedInfo/ds:CanonicalizationMethod', 'ds' => DSIG ) + noko_sig_element = document.at_xpath('//ds:Signature', 'ds' => DSIG) + noko_signed_info_element = noko_sig_element.at_xpath('./ds:SignedInfo', 'ds' => DSIG) + + # Handle when no URI noko_signed_info_reference_element_uri_attr = noko_signed_info_element.at_xpath('./ds:Reference', 'ds' => DSIG).attributes["URI"] if (noko_signed_info_reference_element_uri_attr.value.empty?) noko_signed_info_reference_element_uri_attr.value = "##{document.root.attribute('ID')}" @@ -271,8 +277,11 @@ def validate_signature(base64_cert, soft = true) canon_string = noko_signed_info_element.canonicalize(canon_algorithm) noko_sig_element.remove + # get inclusive namespaces + inclusive_namespaces = extract_inclusive_namespaces + # check digests - REXML::XPath.each(@sig_element, "//ds:Reference", {"ds"=>DSIG}) do |ref| + REXML::XPath.each(sig_element, "//ds:Reference", {"ds"=>DSIG}) do |ref| uri = ref.attributes.get_attribute("URI").value hashed_element = uri.empty? ? document : document.at_xpath("//*[@ID=$uri]", nil, { 'uri' => uri[1..-1] }) @@ -303,26 +312,11 @@ def validate_signature(base64_cert, soft = true) end end - base64_signature = REXML::XPath.first( - @sig_element, - "//ds:SignatureValue", - {"ds" => DSIG} - ).text - - signature = Base64.decode64(base64_signature) - # get certificate object cert_text = Base64.decode64(base64_cert) cert = OpenSSL::X509::Certificate.new(cert_text) - # signature method - sig_alg_value = REXML::XPath.first( - signed_info_element, - "//ds:SignatureMethod", - {"ds"=>DSIG} - ) - signature_algorithm = algorithm(sig_alg_value) - + # verify signature unless cert.public_key.verify(signature_algorithm.new, signature, canon_string) @errors << "Key validation error" return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Key validation error")) @@ -347,7 +341,7 @@ def extract_signed_element_id return nil if reference_element.nil? sei = reference_element.attribute("URI").value[1..-1] - self.signed_element_id = sei.nil? ? self.root.attribute("ID") : sei + sei.nil? ? self.root.attribute("ID") : sei end def extract_inclusive_namespaces diff --git a/test/response_test.rb b/test/response_test.rb index 5cb5c75bd..25f9ec317 100644 --- a/test/response_test.rb +++ b/test/response_test.rb @@ -11,6 +11,7 @@ class RubySamlTest < Minitest::Test let(:response_without_attributes) { OneLogin::RubySaml::Response.new(response_document_without_attributes) } let(:response_without_reference_uri) { OneLogin::RubySaml::Response.new(response_document_without_reference_uri) } let(:response_with_signed_assertion) { OneLogin::RubySaml::Response.new(response_document_with_signed_assertion) } + let(:response_with_ds_namespace_at_the_root) { OneLogin::RubySaml::Response.new(response_document_with_ds_namespace_at_the_root)} let(:response_unsigned) { OneLogin::RubySaml::Response.new(response_document_unsigned) } 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)) } @@ -673,6 +674,13 @@ class RubySamlTest < Minitest::Test assert_empty response_valid_signed.errors end + it "return true when the signature is valid and ds namespace is at the root" do + settings.idp_cert_fingerprint = '5614657ab692b960480389723a36446a5fe1f7ec' + response_with_ds_namespace_at_the_root.settings = settings + assert response_with_ds_namespace_at_the_root.send(:validate_signature) + assert_empty response_with_ds_namespace_at_the_root.errors + end + it "return false when no fingerprint" do settings.idp_cert_fingerprint = nil settings.idp_cert = nil diff --git a/test/responses/response_with_ds_namespace_at_the_root.xml.base64 b/test/responses/response_with_ds_namespace_at_the_root.xml.base64 new file mode 100644 index 000000000..48893cdab --- /dev/null +++ b/test/responses/response_with_ds_namespace_at_the_root.xml.base64 @@ -0,0 +1 @@ +PHNhbWxwOlJlc3BvbnNlIHhtbG5zOnNhbWxwPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6cHJvdG9jb2wiDQp4bWxuczpkcz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnIyIgeG1sbnM6c2FtbD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmFzc2VydGlvbiIgSUQ9Il84ZThkYzVmNjlhOThjYzRjMWZmMzQyN2U1Y2UzNDYwNmZkNjcyZjkxZTYiIFZlcnNpb249IjIuMCIgSXNzdWVJbnN0YW50PSIyMDE0LTA3LTE3VDAxOjAxOjQ4WiIgRGVzdGluYXRpb249Imh0dHA6Ly9zcC5leGFtcGxlLmNvbS9kZW1vMS9pbmRleC5waHA/YWNzIiBJblJlc3BvbnNlVG89Ik9ORUxPR0lOXzRmZWUzYjA0NjM5NWM0ZTc1MTAxMWU5N2Y4OTAwYjUyNzNkNTY2ODUiPg0KICA8c2FtbDpJc3N1ZXI+aHR0cDovL2lkcC5leGFtcGxlLmNvbS9tZXRhZGF0YS5waHA8L3NhbWw6SXNzdWVyPg0KICA8c2FtbHA6U3RhdHVzPg0KICAgIDxzYW1scDpTdGF0dXNDb2RlIFZhbHVlPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6c3RhdHVzOlN1Y2Nlc3MiLz4NCiAgPC9zYW1scDpTdGF0dXM+DQogIDxzYW1sOkFzc2VydGlvbiB4bWxuczp4c2k9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvWE1MU2NoZW1hLWluc3RhbmNlIiB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIElEPSJwZng2YzViMTVjYy02YzY4LTljMWYtY2QzNy05ZTFmYTFkMmU3YTAiIFZlcnNpb249IjIuMCIgSXNzdWVJbnN0YW50PSIyMDE0LTA3LTE3VDAxOjAxOjQ4WiI+DQogICAgPHNhbWw6SXNzdWVyPmh0dHA6Ly9pZHAuZXhhbXBsZS5jb20vbWV0YWRhdGEucGhwPC9zYW1sOklzc3Vlcj48ZHM6U2lnbmF0dXJlPg0KICA8ZHM6U2lnbmVkSW5mbz48ZHM6Q2Fub25pY2FsaXphdGlvbk1ldGhvZCBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvMTAveG1sLWV4Yy1jMTRuIyIvPg0KICAgIDxkczpTaWduYXR1cmVNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjcnNhLXNoYTEiLz4NCiAgPGRzOlJlZmVyZW5jZSBVUkk9IiNwZng2YzViMTVjYy02YzY4LTljMWYtY2QzNy05ZTFmYTFkMmU3YTAiPjxkczpUcmFuc2Zvcm1zPjxkczpUcmFuc2Zvcm0gQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjZW52ZWxvcGVkLXNpZ25hdHVyZSIvPjxkczpUcmFuc2Zvcm0gQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzEwL3htbC1leGMtYzE0biMiLz48L2RzOlRyYW5zZm9ybXM+PGRzOkRpZ2VzdE1ldGhvZCBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvMDkveG1sZHNpZyNzaGExIi8+PGRzOkRpZ2VzdFZhbHVlPmlHdENJcGpyZ09Hc2FDY0lFOEtwSWEzVzZnVT08L2RzOkRpZ2VzdFZhbHVlPjwvZHM6UmVmZXJlbmNlPjwvZHM6U2lnbmVkSW5mbz48ZHM6U2lnbmF0dXJlVmFsdWU+T0RIU2tzUkl1ZEc3b2s0KzRUZitPRnA0UFJ4Ukc3d3BERGdyVDZwQk5ZbUFKNUN2dFk5UlczRldHYlJtZ2NoSjdhYW9sQ3ViekQ5Qkk4Y01nb3V6ZGErdjcrZ1k4THloVUNmV2RSd0gvSzZlVmYvZUtldjk3aUQ1YVA1TTU1MCtCZ0NVaHVWNFg0MlpUbFhkdjBVKzRDQ1VYWnFuLzRjczBxNXhkeTBoM0JzPTwvZHM6U2lnbmF0dXJlVmFsdWU+DQo8ZHM6S2V5SW5mbz48ZHM6WDUwOURhdGE+PGRzOlg1MDlDZXJ0aWZpY2F0ZT5NSUlDYWpDQ0FkT2dBd0lCQWdJQkFEQU5CZ2txaGtpRzl3MEJBUTBGQURCU01Rc3dDUVlEVlFRR0V3SjFjekVUTUJFR0ExVUVDQXdLUTJGc2FXWnZjbTVwWVRFVk1CTUdBMVVFQ2d3TVQyNWxiRzluYVc0Z1NXNWpNUmN3RlFZRFZRUUREQTV6Y0M1bGVHRnRjR3hsTG1OdmJUQWVGdzB4TkRBM01UY3hOREV5TlRaYUZ3MHhOVEEzTVRjeE5ERXlOVFphTUZJeEN6QUpCZ05WQkFZVEFuVnpNUk13RVFZRFZRUUlEQXBEWVd4cFptOXlibWxoTVJVd0V3WURWUVFLREF4UGJtVnNiMmRwYmlCSmJtTXhGekFWQmdOVkJBTU1Ebk53TG1WNFlXMXdiR1V1WTI5dE1JR2ZNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0R05BRENCaVFLQmdRRFp4K09ONElVb0lXeGd1a1RiMXRPaVgzYk1ZellRaXdXUFVOTXArRnE4MnhvTm9nc28yYnlrWkcweWlKbTVvOHp2L3NkNnBHb3VheU1na3gvMkZTT2RjMzZUMGpHYkNIdVJTYnRpYTBQRXpOSVJ0bVZpTXJ0M0Flb1dCaWRSWG1ac3hDTkx3Z0lWNmRuMldwdUU1QXowYkhncFpuUXhUS0ZlazBCTUtVL2Q4d0lEQVFBQm8xQXdUakFkQmdOVkhRNEVGZ1FVR0h4WXFaWXlYN2NUeEtWT0RWZ1p3U1RkQ253d0h3WURWUjBqQkJnd0ZvQVVHSHhZcVpZeVg3Y1R4S1ZPRFZnWndTVGRDbnd3REFZRFZSMFRCQVV3QXdFQi96QU5CZ2txaGtpRzl3MEJBUTBGQUFPQmdRQnlGT2wraE1GSUNiZDNESmZucDJSZ2QvZHF0dHNaRy90eWhJTFd2RXJiaW8vREVlOThtWHBvd2hUa0MwNEVOcHJPeVhpN1piVXFpaWNGODl1QUd5dDFvcWdUVUNEMVZzTGFocUljbXJ6Z3VtTnlUd0xHV28xN1dEQWExL3VzRGhldFdBTWhnekYvQ25mNWVrMG5LMDBtMFlaR3ljNEx6Z0QwQ1JPTUFTVFdOZz09PC9kczpYNTA5Q2VydGlmaWNhdGU+PC9kczpYNTA5RGF0YT48L2RzOktleUluZm8+PC9kczpTaWduYXR1cmU+DQogICAgPHNhbWw6U3ViamVjdD4NCiAgICAgIDxzYW1sOk5hbWVJRCBTUE5hbWVRdWFsaWZpZXI9Imh0dHA6Ly9zcC5leGFtcGxlLmNvbS9kZW1vMS9tZXRhZGF0YS5waHAiIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOm5hbWVpZC1mb3JtYXQ6dHJhbnNpZW50Ij5fY2UzZDI5NDhiNGNmMjAxNDZkZWUwYTBiM2RkNmY2OWI2Y2Y4NmY2MmQ3PC9zYW1sOk5hbWVJRD4NCiAgICAgIDxzYW1sOlN1YmplY3RDb25maXJtYXRpb24gTWV0aG9kPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6Y206YmVhcmVyIj4NCiAgICAgICAgPHNhbWw6U3ViamVjdENvbmZpcm1hdGlvbkRhdGEgTm90T25PckFmdGVyPSIyMDI0LTAxLTE4VDA2OjIxOjQ4WiIgUmVjaXBpZW50PSJodHRwOi8vc3AuZXhhbXBsZS5jb20vZGVtbzEvaW5kZXgucGhwP2FjcyIgSW5SZXNwb25zZVRvPSJPTkVMT0dJTl80ZmVlM2IwNDYzOTVjNGU3NTEwMTFlOTdmODkwMGI1MjczZDU2Njg1Ii8+DQogICAgICA8L3NhbWw6U3ViamVjdENvbmZpcm1hdGlvbj4NCiAgICA8L3NhbWw6U3ViamVjdD4NCiAgICA8c2FtbDpDb25kaXRpb25zIE5vdEJlZm9yZT0iMjAxNC0wNy0xN1QwMTowMToxOFoiIE5vdE9uT3JBZnRlcj0iMjAyNC0wMS0xOFQwNjoyMTo0OFoiPg0KICAgICAgPHNhbWw6QXVkaWVuY2VSZXN0cmljdGlvbj4NCiAgICAgICAgPHNhbWw6QXVkaWVuY2U+aHR0cDovL3NwLmV4YW1wbGUuY29tL2RlbW8xL21ldGFkYXRhLnBocDwvc2FtbDpBdWRpZW5jZT4NCiAgICAgIDwvc2FtbDpBdWRpZW5jZVJlc3RyaWN0aW9uPg0KICAgIDwvc2FtbDpDb25kaXRpb25zPg0KICAgIDxzYW1sOkF1dGhuU3RhdGVtZW50IEF1dGhuSW5zdGFudD0iMjAxNC0wNy0xN1QwMTowMTo0OFoiIFNlc3Npb25Ob3RPbk9yQWZ0ZXI9IjIwMjQtMDctMTdUMDk6MDE6NDhaIiBTZXNzaW9uSW5kZXg9Il9iZTk5NjdhYmQ5MDRkZGNhZTNjMGViNDE4OWFkYmUzZjcxZTMyN2NmOTMiPg0KICAgICAgPHNhbWw6QXV0aG5Db250ZXh0Pg0KICAgICAgICA8c2FtbDpBdXRobkNvbnRleHRDbGFzc1JlZj51cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YWM6Y2xhc3NlczpQYXNzd29yZDwvc2FtbDpBdXRobkNvbnRleHRDbGFzc1JlZj4NCiAgICAgIDwvc2FtbDpBdXRobkNvbnRleHQ+DQogICAgPC9zYW1sOkF1dGhuU3RhdGVtZW50Pg0KICAgIDxzYW1sOkF0dHJpYnV0ZVN0YXRlbWVudD4NCiAgICAgIDxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJ1aWQiIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6YmFzaWMiPg0KICAgICAgICA8c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4c2k6dHlwZT0ieHM6c3RyaW5nIj50ZXN0PC9zYW1sOkF0dHJpYnV0ZVZhbHVlPg0KICAgICAgPC9zYW1sOkF0dHJpYnV0ZT4NCiAgICAgIDxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJtYWlsIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4NCiAgICAgICAgPHNhbWw6QXR0cmlidXRlVmFsdWUgeHNpOnR5cGU9InhzOnN0cmluZyI+dGVzdEBleGFtcGxlLmNvbTwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT4NCiAgICAgIDwvc2FtbDpBdHRyaWJ1dGU+DQogICAgICA8c2FtbDpBdHRyaWJ1dGUgTmFtZT0iZWR1UGVyc29uQWZmaWxpYXRpb24iIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6YmFzaWMiPg0KICAgICAgICA8c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4c2k6dHlwZT0ieHM6c3RyaW5nIj51c2Vyczwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT4NCiAgICAgICAgPHNhbWw6QXR0cmlidXRlVmFsdWUgeHNpOnR5cGU9InhzOnN0cmluZyI+ZXhhbXBsZXJvbGUxPC9zYW1sOkF0dHJpYnV0ZVZhbHVlPg0KICAgICAgPC9zYW1sOkF0dHJpYnV0ZT4NCiAgICA8L3NhbWw6QXR0cmlidXRlU3RhdGVtZW50Pg0KICA8L3NhbWw6QXNzZXJ0aW9uPg0KPC9zYW1scDpSZXNwb25zZT4= \ No newline at end of file diff --git a/test/test_helper.rb b/test/test_helper.rb index 78a2008d9..84cada56e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -79,6 +79,10 @@ def response_document_with_signed_assertion_2 @response_document_with_signed_assertion_2 ||= read_response("response_with_signed_assertion_2.xml.base64") end + def response_document_with_ds_namespace_at_the_root + @response_document_with_ds_namespace_at_the_root ||= read_response("response_with_ds_namespace_at_the_root.xml.base64") + end + def response_document_unsigned @response_document_unsigned ||= read_response("response_unsigned_xml_base64") end