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

Invalid Signature on SAML Response #540

Closed
Bennet-Sunder opened this issue May 25, 2020 · 7 comments
Closed

Invalid Signature on SAML Response #540

Bennet-Sunder opened this issue May 25, 2020 · 7 comments

Comments

@Bennet-Sunder
Copy link

Hello,
I'm verifying a signed SAML response like so response.is_valid? and in the error messages I have ["Invalid Signature on SAML Response"]
It's similar to #442 except that I debugged and found that the x.509 certificate, the algorithm used in the response matches with the one given in the settings.

Could you help me out with that are the other conditions where it might throw up such a validation error?
I was thinking if it might throw up here https://github.com/onelogin/ruby-saml/blob/811618d08ace032c830b62d7ea3a6c2ae32d6c19/lib/onelogin/ruby-saml/response.rb#L840 but the response has just one signature attribute.
or maybe this https://github.com/onelogin/ruby-saml/blob/811618d08ace032c830b62d7ea3a6c2ae32d6c19/lib/onelogin/ruby-saml/response.rb#L866

But I'm not sure what other reasons could make this to have only one such error message.

@pitbulk
Copy link
Collaborator

pitbulk commented May 25, 2020

Cause of the error should be stored in the errors.

While you debugging, what verification raised the exception?

@benneyman
Copy link

So by debugging I meant recreating the same response and calling is_valid? on it. I'm not sure which assertion raised this.

After calling is_valid? This was the the only error in the array of errors. I tried finding the source of the error by reading the implementation on GitHub but since the certificates match in at a kid off what could have raised this.

@pitbulk
Copy link
Collaborator

pitbulk commented May 26, 2020

If you add a debugger on the is_valid? method, you can go step by step and see what check verification raised the issue. See https://github.com/deivid-rodriguez/byebug

@Bennet-Sunder
Copy link
Author

Bennet-Sunder commented May 27, 2020

On debugging this issue, on version 1.7.0 I found when I create the response object like so

saml_response = OneLogin::RubySaml::Response.new(saml_xml, { skip_recipient_check: true })
and when I do a saml_response.to_s I can see in that <ds:SignatureValue> attribute this &#xd; occurs four times. Not sure if this is fine but this was in both 1.7.0 and 1.10.0

I manually removed the four entries, base64 decoded it and ran this while debugging and it got verified successfully in 1.7.0
https://github.com/onelogin/ruby-saml/blob/082249efe498841e0747d2b3452f4ef5e867b025/lib/xml_security.rb#L355

On debugging on both the versions I found out that the error seemed to be in OneLogin::RubySaml::Utils.element_text method . In 1.7.0 it returned with &#xd; present in the signature value and in 1.10.0 it returned with it replaced as \r.
And this particular SAML response worked fine in 1.10.0
Unfortunately, I will not be able to provide the saml_response that caused this, but I think it you could create a response that generates those characters you will see that it fails.

Could you please confirm if this is a bug in 1.7.0

@pitbulk
Copy link
Collaborator

pitbulk commented May 27, 2020

Version 1.7.0 is 2 years old. You should be using the latest.

1.7.0 has an audience validation issue: #444

@Bennet-Sunder
Copy link
Author

thank you. I can see that my issue was fixed in 1.7.2 with this PR #446

@Bennet-Sunder
Copy link
Author

@pitbulk In the readme https://github.com/onelogin/ruby-saml/blob/master/README.md#updating-from-160-to-170
Shouldn't the text suggest to install 1.7.2 as that is the version that fixes https://www.cvedetails.com/cve/CVE-2017-11428/ with #446 as opposed to 1.7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants