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

Ignore the Advice section when looking for signatures. #247

Closed

Conversation

flupzor
Copy link
Contributor

@flupzor flupzor commented Feb 18, 2021

There can be legal Signature blocks in the 'Advice' element of a response. Since python3-saml does not suport doing
anything with the advice section, skipping it altogether seems. like the best solution.

This is part of #237 as well.

There can be legal Signature blocks in the 'Advice' element of a response. Since python3-saml does not suport doing
anything with the advice section, skipping it altogether seems. like the best solution.
@pitbulk
Copy link
Contributor

pitbulk commented Feb 22, 2021

In the Signarture validation, the right xpath is provided
https://github.com/onelogin/python3-saml/blob/c25df8164563df28ec94dd6f5d7a9a6c65c06e83/src/onelogin/saml2/response.py#L307

The process_signed_elements inspect what elements contain a signature, I agree the Advice is a legal element that can contain a signature, but most of the time was used as a way to execute signature wrapping attacks, so better not to whitelist the Advice element at all, the toolkit does not expect an Advice element and I'm not aware of any Identity Provider using such element neither.

@flupzor
Copy link
Contributor Author

flupzor commented Feb 25, 2021

Aha, thanks for the explanation. I also read up a bit on the signature wrapping attacks, and now it makes sense.

I'm going to discuss with my Identity Provider and see if they can omit the Advice element.

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