Deal with cases where there is no SubjectConfirmationData. - #530
jeremyfrench wants to merge 2 commits into
Conversation
|
The SAML errata documentation at http://docs.oasis-open.org/security/saml/v2.0/errata05/os/saml-v2.0-errata05-os.pdf seems to clarify this:
To me, it looks like the proposed fix covers this as there still must be a valid <SubjectConfirmation> and they are still evaluated, this PR simply skips the case where an empty one is encountered. |
…ate error must be thrown. This resolves #530. There are two problems here: - When only one SubjectConfirmation is received and it is empty, an error should be thrown. However, the error would be a not very descriptive message warning about access to a non-property in a null object. Something more descriptive should be in place. - Additionally, in PHP 7.0 this is an error and not an exception, and then the code continues to execute, effectively allowing assertions without a proper SubjectConfirmation element. This is wrong according to the standard. We take the chance also to fix indentation in the whole file, as there were a few spaces mixed with tabs.
|
Hi guys! @longwave is right. The fix is needed not only to avoid failures when an IdP sends an empty I've committed fixes for this both in master and in the 1.14 branch (with a slight difference between them and your PR, that being explicit checking of the Thanks for your help! |
…ate error must be thrown. This resolves simplesamlphp#530. There are two problems here: - When only one SubjectConfirmation is received and it is empty, an error should be thrown. However, the error would be a not very descriptive message warning about access to a non-property in a null object. Something more descriptive should be in place. - Additionally, in PHP 7.0 this is an error and not an exception, and then the code continues to execute, effectively allowing assertions without a proper SubjectConfirmation element. This is wrong according to the standard.
3 participants
I had a case where a saml packet contained this
<SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer" />This caused SSP to fail. As subject confirmation data is not defined.
I have looked at the spec (well the xml schema) and it seems to be valid.
<element ref="saml:SubjectConfirmationData" minOccurs="0"/>I'll agree that it doesn't make any sense to have such a element but if it is not invalid we should not be having a fatal error.