Failure to find signature in the response when the assertion is encrypted

[ricardofandrade]

In condition where we have: a) A signed response b) An encrypted assertion without a signature

The processing of the SAML response will fail with either the Response or Assertion must be signed which is incorrect given the condition a) above.

Looking at the implementation, it's possible to see the following:

• The whole response is loaded (note decodedResponseXML): https://github.com/crewjam/saml/blob/63667204bd3c10cea54b351079c8c380b1710d6a/service_provider.go#L577
• The decryption of the assertion takes place: https://github.com/crewjam/saml/blob/63667204bd3c10cea54b351079c8c380b1710d6a/service_provider.go#L592
• The response is replaced with the decrypted assertion (note plaintextAssertion): https://github.com/crewjam/saml/blob/63667204bd3c10cea54b351079c8c380b1710d6a/service_provider.go#L600
• The signature is validated only for the assertion (the response is gone!): https://github.com/crewjam/saml/blob/63667204bd3c10cea54b351079c8c380b1710d6a/service_provider.go#L605

If we read the XMLEnc standard, item 4.2 Decryption (bullet 4), it says:

3. The decryptor SHOULD support the ability to replace the EncryptedData element with the decrypted 'element' or element 'content' represented by the UTF-8 encoded characters.

The replacement is not happening at the EncryptedData element level, but at the whole document level.

Since the standard says "SHOULD" fixes to this package could include either replacing only EncryptedData or alternatively validating the response signature before the encryption happens.

I'm happy to provide a PR with a fix after I hear some thoughts on the analysis above 😄

[crewjam]

Thanks for your report, @ricardofandrade. Sounds like something we should fix, I'd be grateful for your work on a PR.

Ideally a PR would pay careful attention paid to the order of signature validation vs decryption, and also would include a test case for the kind of assertion you are seeing that we cannot currently handle.

Thanks!

[dcepler]

Just ran into this after testing with location of signature with encrypted responses.

One thing to note is that WantAssertionsSigned="true" is hard set in the SP metadata. https://github.com/crewjam/saml/blob/17489b9c3af5c6ca7224d71cf3469bd637410111/service_provider.go#L130 and should impact the validation logic. This probably needs to be its own issue.