search

SAML-Toolkits / python3-saml

MIT License
683 stars 304 forks source link

Ignore the Advice section when looking for signatures. #247

Closed flupzor closed 3 years ago

flupzor commented 3 years ago

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.

pitbulk commented 3 years ago

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 commented 3 years ago

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.