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

Handle empty URI references #254

Merged
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
2 changes: 1 addition & 1 deletion lib/onelogin/ruby-saml/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module OneLogin
module RubySaml
VERSION = '1.0.0'
VERSION = '1.0.1'
end
end
15 changes: 13 additions & 2 deletions lib/xml_security.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,14 +262,21 @@ def validate_signature(base64_cert, soft = true)
'//ds:CanonicalizationMethod',
'ds' => DSIG
)

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')}"
end

canon_string = noko_signed_info_element.canonicalize(canon_algorithm)
noko_sig_element.remove

# check digests
REXML::XPath.each(@sig_element, "//ds:Reference", {"ds"=>DSIG}) do |ref|
uri = ref.attributes.get_attribute("URI").value

hashed_element = document.at_xpath("//*[@ID=$uri]", nil, { 'uri' => uri[1..-1] })
hashed_element = uri.empty? ? document : document.at_xpath("//*[@ID=$uri]", nil, { 'uri' => uri[1..-1] })
# hashed_element = document.at_xpath("//*[@ID=$uri]", nil, { 'uri' => uri[1..-1] })
canon_algorithm = canon_algorithm REXML::XPath.first(
ref,
'//ds:CanonicalizationMethod',
Expand Down Expand Up @@ -336,7 +343,11 @@ def extract_signed_element_id
"//ds:Signature/ds:SignedInfo/ds:Reference",
{"ds"=>DSIG}
)
self.signed_element_id = reference_element.attribute("URI").value[1..-1] unless reference_element.nil?

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
end

def extract_inclusive_namespaces
Expand Down
10 changes: 10 additions & 0 deletions test/response_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class RubySamlTest < Minitest::Test
let(:settings) { OneLogin::RubySaml::Settings.new }
let(:response) { OneLogin::RubySaml::Response.new(response_document_without_recipient) }
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_unsigned) { OneLogin::RubySaml::Response.new(response_document_unsigned) }
let(:response_wrapped) { OneLogin::RubySaml::Response.new(response_document_wrapped) }
Expand Down Expand Up @@ -384,6 +385,15 @@ class RubySamlTest < Minitest::Test
response_no_version.is_valid?
assert_includes response_no_version.errors, "Unsupported SAML version"
end

it "return true when a nil URI is given in the ds:Reference" do

response_without_reference_uri.stubs(:conditions).returns(nil)
response_without_reference_uri.settings = settings
response_without_reference_uri.settings.idp_cert_fingerprint = "19:4D:97:E4:D8:C9:C8:CF:A4:B7:21:E5:EE:49:7F:D9:66:0E:52:13"
assert response_without_reference_uri.is_valid?
assert_empty response_without_reference_uri.errors
end
end
end

Expand Down
1 change: 1 addition & 0 deletions test/responses/response_without_reference_uri.xml.base64
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PD94bWwgdmVyc2lvbj0iMS4wIj8+DQo8c2FtbHA6UmVzcG9uc2UgeG1sbnM6c2FtbHA9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpwcm90b2NvbCIgSUQ9InBmeGQ1OTQzNDdkLTQ5NWYtYjhkMS0wZWUyLTQxY2ZkYTE0ZGQzNSIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMDEtMDJUMjI6NDg6NDhaIiBEZXN0aW5hdGlvbj0iaHR0cDovL2xvY2FsaG9zdDo5MDAxL3YxL3VzZXJzL2F1dGhvcml6ZS9zYW1sIiBDb25zZW50PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6Y29uc2VudDp1bnNwZWNpZmllZCIgSW5SZXNwb25zZVRvPSJfZWQ5MTVhNDAtNzRmYi0wMTMyLTViMTYtNDhlMGViMTRhMWM3Ij4NCiAgPElzc3VlciB4bWxucz0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmFzc2VydGlvbiI+aHR0cDovL2V4YW1wbGUuY29tPC9Jc3N1ZXI+PGRzOlNpZ25hdHVyZSB4bWxuczpkcz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnIyI+DQogIDxkczpTaWduZWRJbmZvPjxkczpDYW5vbmljYWxpemF0aW9uTWV0aG9kIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8xMC94bWwtZXhjLWMxNG4jIi8+DQogICAgPGRzOlNpZ25hdHVyZU1ldGhvZCBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvMDkveG1sZHNpZyNyc2Etc2hhMSIvPg0KICA8ZHM6UmVmZXJlbmNlIFVSST0iIj48ZHM6VHJhbnNmb3Jtcz48ZHM6VHJhbnNmb3JtIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnI2VudmVsb3BlZC1zaWduYXR1cmUiLz48ZHM6VHJhbnNmb3JtIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8xMC94bWwtZXhjLWMxNG4jIi8+PC9kczpUcmFuc2Zvcm1zPjxkczpEaWdlc3RNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjc2hhMSIvPjxkczpEaWdlc3RWYWx1ZT5qQ2dlWENQREZsd2pUZ3FnUHAwbVUyVHF3OWc9PC9kczpEaWdlc3RWYWx1ZT48L2RzOlJlZmVyZW5jZT48L2RzOlNpZ25lZEluZm8+PGRzOlNpZ25hdHVyZVZhbHVlPkRmdXByMTh3UityRGFndENQRWZRbFNHSHp3NE5kZlBIWjRIc3pGZTFKUENKWGpmYnlFTTFmZytqemdHYk1NdDZYemdDWGNLSk03RS9DUFNURGt2TWUzRFVKbEh1NERodURPQXovRHN5b0J3V3VWK1JmM1dpTmNGNFhDYzl3QlF6dm4vYXREN3pXNnh3TzdOL2hrQVpKcWZ2SmRkbnBNTUhLR1hxRy9aSFpBdz08L2RzOlNpZ25hdHVyZVZhbHVlPg0KPGRzOktleUluZm8+PGRzOlg1MDlEYXRhPjxkczpYNTA5Q2VydGlmaWNhdGU+TUlJQ3FEQ0NBaEdnQXdJQkFnSUJBREFOQmdrcWhraUc5dzBCQVEwRkFEQnhNUXN3Q1FZRFZRUUdFd0oxY3pFVE1CRUdBMVVFQ0F3S1YyRnphR2x1WjNSdmJqRWlNQ0FHQTFVRUNnd1pSbXhoZENCWGIzSnNaQ0JMYm05M2JHVmtaMlVzSUVsdVl6RWNNQm9HQTFVRUF3d1RiR1ZoY200dVpteGhkSGR2Y214a0xtTnZiVEVMTUFrR0ExVUVCd3dDUkVNd0hoY05NVFV3TnpBNE1EazFPVEF6V2hjTk1qVXdOekExTURrMU9UQXpXakJ4TVFzd0NRWURWUVFHRXdKMWN6RVRNQkVHQTFVRUNBd0tWMkZ6YUdsdVozUnZiakVpTUNBR0ExVUVDZ3daUm14aGRDQlhiM0pzWkNCTGJtOTNiR1ZrWjJVc0lFbHVZekVjTUJvR0ExVUVBd3dUYkdWaGNtNHVabXhoZEhkdmNteGtMbU52YlRFTE1Ba0dBMVVFQnd3Q1JFTXdnWjh3RFFZSktvWklodmNOQVFFQkJRQURnWTBBTUlHSkFvR0JBTVBEd3NsNW82eDJRb3VOaTEvRTdJVXFSWWoyWW9jSlJGc3VFR1RldnlVKzJhRkNhQk5WL3R0NnNBYk05V1N1dEx1cWpFL2hmYm5sRWNaMDMrZ24wQ29MbDZZbXdiS0tlUnBrSXplVmhveUoxWVlNUUVBVmhMcmR5OFBvd3U4VUNaMFBiQXorbjlka2lSek01cENDTzc3K2d5Y0ZUQkZLSEFBOXFJcFVaWmtQQWdNQkFBR2pVREJPTUIwR0ExVWREZ1FXQkJRSFU1OGl1R3hGbFp1ckJVSndvbGFsSnIrRlJ6QWZCZ05WSFNNRUdEQVdnQlFIVTU4aXVHeEZsWnVyQlVKd29sYWxKcitGUnpBTUJnTlZIUk1FQlRBREFRSC9NQTBHQ1NxR1NJYjNEUUVCRFFVQUE0R0JBQzZpSGZNbWQraE1TUnpma29zaTNDK3d2cUhDTEVVc2czSEZwa1ptNWp4bVREbEY1cU8rQnQwbjB4bWZvcVdCekJNbE5DOFRzR3JhZmhKM3p1OEdORjBMZW8xMXJmYzFHTUdCdnI1SG9aM1dBQXltbkJFREFBb3N4TjZXWlJtajF4YWdhMTMrNnBXZkdCKysyblB3Y1pXUC84ZGtQY1JvZ2V2VjB4MHA1Njg2PC9kczpYNTA5Q2VydGlmaWNhdGU+PC9kczpYNTA5RGF0YT48L2RzOktleUluZm8+PC9kczpTaWduYXR1cmU+DQogIDxzYW1scDpTdGF0dXM+DQogICAgPHNhbWxwOlN0YXR1c0NvZGUgVmFsdWU9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpzdGF0dXM6U3VjY2VzcyIvPg0KICA8L3NhbWxwOlN0YXR1cz4NCg0KICA8QXNzZXJ0aW9uIHhtbG5zPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiBJRD0iXzcwMGFjMzIwLTc0ZmYtMDEzMi01YjE0LTQ4ZTBlYjE0YTFjNyIgSXNzdWVJbnN0YW50PSIyMDE1LTAxLTAyVDIyOjQ4OjQ4WiIgVmVyc2lvbj0iMi4wIj4NCiAgICA8SXNzdWVyPmh0dHA6Ly9leGFtcGxlLmNvbTwvSXNzdWVyPg0KICAgIDxTdWJqZWN0Pg0KICAgICAgPE5hbWVJRCBGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjEuMTpuYW1laWQtZm9ybWF0OmVtYWlsQWRkcmVzcyI+c2FtbEB1c2VyLmNvbTwvTmFtZUlEPg0KICAgICAgPFN1YmplY3RDb25maXJtYXRpb24gTWV0aG9kPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6Y206YmVhcmVyIj4NCiAgICAgICAgPFN1YmplY3RDb25maXJtYXRpb25EYXRhIEluUmVzcG9uc2VUbz0iX2VkOTE1YTQwLTc0ZmItMDEzMi01YjE2LTQ4ZTBlYjE0YTFjNyIgTm90T25PckFmdGVyPSIyMDM4LTAxLTAyVDIyOjUxOjQ4WiIgUmVjaXBpZW50PSJodHRwOi8vbG9jYWxob3N0OjkwMDEvdjEvdXNlcnMvYXV0aG9yaXplL3NhbWwiLz4NCiAgICAgIDwvU3ViamVjdENvbmZpcm1hdGlvbj4NCiAgICA8L1N1YmplY3Q+DQogICAgPENvbmRpdGlvbnMgTm90QmVmb3JlPSIyMDE1LTAxLTAyVDIyOjQ4OjQzWiIgTm90T25PckFmdGVyPSIyMDM4LTAxLTAyVDIzOjQ4OjQ4WiI+DQogICAgICA8QXVkaWVuY2VSZXN0cmljdGlvbj4NCiAgICAgICAgPEF1ZGllbmNlPmh0dHA6Ly9sb2NhbGhvc3Q6OTAwMS88L0F1ZGllbmNlPg0KICAgICAgICA8QXVkaWVuY2U+ZmxhdF93b3JsZDwvQXVkaWVuY2U+DQogICAgICA8L0F1ZGllbmNlUmVzdHJpY3Rpb24+DQogICAgPC9Db25kaXRpb25zPg0KICAgIDxBdHRyaWJ1dGVTdGF0ZW1lbnQ+DQogICAgICA8QXR0cmlidXRlIE5hbWU9Imh0dHA6Ly9zY2hlbWFzLnhtbHNvYXAub3JnL3dzLzIwMDUvMDUvaWRlbnRpdHkvY2xhaW1zL2VtYWlsYWRkcmVzcyI+DQogICAgICAgIDxBdHRyaWJ1dGVWYWx1ZT5zYW1sQHVzZXIuY29tPC9BdHRyaWJ1dGVWYWx1ZT4NCiAgICAgIDwvQXR0cmlidXRlPg0KICAgIDwvQXR0cmlidXRlU3RhdGVtZW50Pg0KICAgIDxBdXRoblN0YXRlbWVudCBBdXRobkluc3RhbnQ9IjIwMTUtMDEtMDJUMjI6NDg6NDhaIiBTZXNzaW9uSW5kZXg9Il83MDBhYzMyMC03NGZmLTAxMzItNWIxNC00OGUwZWIxNGExYzciPg0KICAgICAgPEF1dGhuQ29udGV4dD4NCiAgICAgICAgPEF1dGhuQ29udGV4dENsYXNzUmVmPnVybjpmZWRlcmF0aW9uOmF1dGhlbnRpY2F0aW9uOndpbmRvd3M8L0F1dGhuQ29udGV4dENsYXNzUmVmPg0KICAgICAgPC9BdXRobkNvbnRleHQ+DQogICAgPC9BdXRoblN0YXRlbWVudD4NCiAgPC9Bc3NlcnRpb24+DQo8L3NhbWxwOlJlc3BvbnNlPg==
4 changes: 4 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ def response_document_without_attributes
@response_document_without_attributes ||= read_response("response_without_attributes.xml.base64")
end

def response_document_without_reference_uri
@response_document_without_reference_uri ||= read_response("response_without_reference_uri.xml.base64")
end

def response_document_with_signed_assertion
@response_document_with_signed_assertion ||= read_response("response_with_signed_assertion.xml.base64")
end
Expand Down