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

validate_document_with_cert uses local cert when no cert in response #400

Merged
merged 2 commits into from
Jul 24, 2017

Conversation

Adam21e
Copy link
Contributor

@Adam21e Adam21e commented May 26, 2017

I hit an issue when you have 2 certs signing and encryption and the cert is not included in the response.

After tracing it down, it appears that the validate_document_with_cert behaves differently to the validate_document method in that it won't fall back to the supplied cert when there was no cert included in the response.

I tried to write tests, but wasn't sure how to generate an signing/encryption pair of certs and wasn't sure I was allowed to use what my SP was using.

@pitbulk
Copy link
Collaborator

pitbulk commented May 26, 2017

@Adam21e you can generate self-signed cert for the SP with that tool:
https://www.samltool.com/self_signed_certs.php

If there is no x509Cert in the SAMLResponse, then you are not able to verify the signature if your SP settings only contains the fingerprint of the IdP Public cert (since you need the cert to execute the signature validation).

@Adam21e
Copy link
Contributor Author

Adam21e commented May 27, 2017

Thanks, I had a lot more time today so poked around a bit more and found a similar test already and added a new test based on this.

response_test has a "return true when no X509Certificate and the cert provided at settings matches" which is the situation that I have.

The difference for me is that I am using idp_cert_multi and not idp_cert.

Using idp_cert_multi triggers validate_document_with_cert which is where the issue for me was.

@pitbulk pitbulk merged commit ca03435 into SAML-Toolkits:master Jul 24, 2017
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

Successfully merging this pull request may close these issues.

2 participants