pysaml2 c14n canonicalization validation looks too restrictive

Code Version

pysaml2==7.1.2

Expected Behavior

Current Behavior

[ERROR 2022-07-26 13:35:06,844 saml2.entity _parse_response 1499: MainProcess] Signature Error: {'message': 'Signature failed to meet constraints on xmldsig', 'validators': {'signatures must have a single reference element': True, 'the Reference element must have a URI attribute': True, 'the URI attribute contains an anchor': True, 'the anchor points to the enclosing element ID attribute': True, 'canonicalization method is c14n': False, 'the number of transforms is one or two': True, 'all transform algs are allowed': True, 'the enveloped signature transform is defined': True, 'object element is not present': True}

The initial part of the XML response that it's been parsed follows:

b'<?xml version=\"1.0\" encoding=\"UTF-8\"?><saml2p:Response xmlns:saml2p=\"urn:oasis:names:tc:SAML:2.0:protocol\" Destination=\"https://acme.com/api/saml2_auth/acs/\" ID=\"xxx\" IssueInstant=\"2022-07-26T13:35:06.746Z\" Version=\"2.0\"><saml2:Issuer xmlns:saml2=\"urn:oasis:names:tc:SAML:2.0:assertion\">REDACTED</saml2:Issuer><ds:Signature xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\"><ds:SignedInfo><ds:CanonicalizationMethod Algorithm=\"http://www.w3.org/TR/2001/REC-xml-c14n-20010315\"/><ds:SignatureMethod Algorithm=\"http://www.w3.org/2001/04/xmldsig-more#rsa-sha256\"/><ds:Reference URI=\"#xxx\"><ds:Transforms><ds:Transform Algorithm=\"http://www.w3.org/2000/09/xmldsig#enveloped-signature\"/><ds:Transform Algorithm=\"http://www.w3.org/2001/10/xml-exc-c14n#\"/></ds:Transforms><ds:DigestMethod Algorithm=\"http://www.w3.org/2001/04/xmlenc#sha256\"/><ds:DigestValue>zzz</ds:DigestValue></ds:Reference></ds:SignedInfo>

Possible solution

At the moment the validator is looking exactly for:

http://www.w3.org/2001/10/xml-exc-c14n#, or
http://www.w3.org/2001/10/xml-exc-c14n#WithComments

It looks like there are other valid values out there, for example:

http://www.w3.org/TR/2001/REC-xml-c14n-20010315
http://www.w3.org/TR/2008/REC-xml-c14n11-20080502
http://www.w3.org/TR/2008/PR-xml-c14n11-20080129
http://www.w3.org/TR/2007/CR-xml-c14n11-20070621

Jul 29 '22 15:07 bugant

The SAML core specification defines:

5.4.3 Canonicalization Method

SAML implementations SHOULD use Exclusive Canonicalization [Excl-C14N], with or without comments, both in the <ds:CanonicalizationMethod> element of <ds:SignedInfo>, and as a <ds:Transform> algorithm. [E83]Use of Exclusive Canonicalization facilitates the verification of signatures created over SAML messages when placed into a different XML context than present during signing.

This is what is followed by the library. The requirement is a "should" but canonicalization is a complex operation/topic and it can affect the safety of the signatures. There are certainly many more canonicalization methods out there, but in practice I have never seen any other method being used for SAML messages. Are you using SAML and are in need of allowing a different canonicalization method?

Jul 31 '22 12:07 c00kiemon5ter

Thanks @c00kiemon5ter for the above reply. Yes, we are integrating with a client to provide them SSO, and we are not in control of the IdP configuration. In the SAML response we are getting the following element:

<ds:CanonicalizationMethod Algorithm=\"http://www.w3.org/TR/2001/REC-xml-c14n-20010315\"/>

and thus getting a validation error. Not sure what is the best way of handling this case. Will try to see if this can be fixed on the IdP side tweaking some configuration.

Aug 08 '22 08:08 bugant

So, http://www.w3.org/TR/2001/REC-xml-c14n-20010315 is inclusive c14n without comments.

Atm, we only allow exclusive c14n (with or without comments):

https://github.com/IdentityPython/pysaml2/blob/1e59eaa09a0b7185705004188fbbed0b53681a23/src/saml2/xmldsig/init.py#L62-L65

This is in line with the SAML core recommendations

5.4 XML Signature Profile

...

5.4.3 Canonicalization Method

SAML implementations SHOULD use Exclusive Canonicalization [Excl-C14N], with or without comments, both in the <ds:CanonicalizationMethod> element of <ds:SignedInfo>, and as a <ds:Transform> algorithm. [E83]Use of Exclusive Canonicalization facilitates the verification of signatures created over SAML messages when placed into a different XML context than present during signing. Note that use of this algorithm alone does not guarantee that a particular signed object can be moved from one context to another safely, nor is that a requirement of signed SAML objects in general, though it MAY be required by particular profiles

This is a SHOULD and in practice that's the only CanonicalizationMethod I've seen used. Making the set of allowed CanonicalizationMethods configurable would solve this and would allow us to keep the core restricted/minimal, while users can set more methods.

Could you try to modify this set to add http://www.w3.org/TR/2001/REC-xml-c14n-20010315 and http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments? This should just work.

 ALLOWED_CANONICALIZATIONS = {
     TRANSFORM_C14N,
     TRANSFORM_C14N_WITH_COMMENTS,
+    "http://www.w3.org/TR/2001/REC-xml-c14n-20010315",
+    "http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments",
 }

References:

https://www.w3.org/TR/2001/REC-xml-c14n-20010315
https://www.w3.org/TR/xml-c14n/
https://www.w3.org/TR/xml-c14n11/
https://stackoverflow.com/questions/29283218/jboss-invalid-algorithm-http-www-w3-org-tr-2001-rec-xml-c14n-20010315-inclus
https://stackoverflow.com/questions/69384889/how-to-properly-change-canonicalization-method-in-this-java-code
https://github.com/yaronn/xml-crypto/pull/116/files
https://santuario.apache.org/Java/api/constant-values.html#org.apache.xml.security.c14n.Canonicalizer.ALGO_ID_C14N_EXCL_OMIT_COMMENTS

Aug 12 '22 00:08 c00kiemon5ter

any news on this @bugant ?

Sep 24 '22 17:09 c00kiemon5ter

any news on this @bugant ?

@c00kiemon5ter we managed to work around the issue with the client, by using a different IdP.

Sep 26 '22 11:09 bugant