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

can not verify signature from xmlseclibs with python signxml library - simplesaml compatibility issue #242

Open
Glocktober opened this issue May 21, 2022 · 0 comments

Comments

@Glocktober
Copy link

I have a python SP that can not validate SAMLResponse tokens generated by simplesamlphp.

Specifically, a simplesaml SAMLResponse token generated by xmlseclibs does not signature validate using the signxml python library.

I chased this down to the template in XMLSecurityDSig.php - these have newlines and spaces that are not removed before signing the <ds:SignedInfo> section. I verified the whitespace remain in the serialized XML.
signedInfoSection.txt

The signxml library on the other hand is doing C14N canonicalization of the tags that removes the \n and spaces. It does this both when it signs and when it verifies, so this whitespace is removed before signature calculation.

Hence even though the <ds:DigestValue> of both libraries are identical the signatures differ because xmlseclibs is including the extra whitespace in Signed Info in its signature calculation.

I hope that's a clear enough explanation.

I was able to achieve compatibility between these libraries by removing the whitespace from the templates in XMLSecurityDSIG.php - no 'tail' spaces or newlines between element tags.

(Markdown is not helpful here showing this, wrapping these lines, but I'm including the diff as well.)

const template = '<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:SignedInfo<ds:SignatureMethod /></ds:SignedInfo></ds:Signature>';

const BASE_TEMPLATE = '<Signature xmlns="http://www.w3.org/2000/09/xmldsig#"><SignedInfo><SignatureMethod /></SignedInfo></Signature>';

patch.txt

I'll ask you consider this change for compatibility.

I am not familiar enough with the requisite standards to conclude if xmlseclibs or signxml are calculating the SignedInfo section correctly, but I do think the <ds:CanonicalizationMethod> tag refers to the <ds:SignedInfo> section (while the <ds:Transforms> refer to the digest calculation)

In any event the change seems an easy enough to assure compatibility.

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

No branches or pull requests

1 participant