Don't send keys in SP Metadata file that you don't want the IdP to use#117
Don't send keys in SP Metadata file that you don't want the IdP to use#117waynewoodfield wants to merge 3 commits into
Conversation
… Don't send the signing key if you aren't going to sign your authnrequests. The IdP will see these as indicators that you want the keys to be used (especially the encrypted assertions).
| keyDescriptorXml.append("</ds:X509Data>"); | ||
| keyDescriptorXml.append("</ds:KeyInfo>"); | ||
| keyDescriptorXml.append("</md:KeyDescriptor>"); | ||
| if (settings.getAuthnRequestsSigned()) { |
There was a problem hiding this comment.
what about LogoutRequest/LogoutResponse signing?
|
Thanks for the call-out. We haven't implemented the SLO yet at my company, so I had a blind spot to that. Added them into the condition now. I don't actually care whether we send the signing key all the time or not. The encryption key is the one I mostly cared about, because there's no "wantResponsesEncrypted" property in the SP Metadata, so the only way I know to tell the IdP that I don't want SAMLResponses to be encrypted is to not send an encryption key. |
|
I agree that the encryption part is the most important. Can you add some unit test as well? |
|
I also noticed that if getWantNameIdEncrypted is true, encryption key should be also sent. Avoid key signing publication is weird for me. I will follow the approach we did on php-saml: |
|
I've updated this fork to include recommendations by Sixto - the signing key is published unconditionally, and encryption key will still be sent when getWantNameIdEncrypted is true. If you'd like to use that fork, I can resubmit a pull request. Otherwise, looking forward to your fix. |
|
Hi @waynewoodfield, I followed what I did on php-saml, adding a boolean to decide when to encrypt or not instead of sending the full settings object. This way we keep the back-compatibility. Thanks for contribute! |
3 participants
Don't send the encryption key if you don't want assertions encrypted. Don't send the signing key if you aren't going to sign your authnrequests. The IdP will see these as indicators that you want the keys to be used (especially the encrypted assertions).