fix c14n canonization

[zuckschwerdt]

omit a default namespace if it is not visibly utilized; the specs suggest that we need to remove even a default namespace, if it is no visibly utilized on the parent. I.e. omit a default ns if the parent is prefixed.

[arekinath]

Would you mind adding a test for this? Also, it'd be great if you could double-check that this is what libxml does as well. It's probably more important for esaml that we do what libxml does than what is actually in the spec -- pretty much all of the SAML implementations we need to interoperate with use it.

[zuckschwerdt]

I added a quick test to illustrate the problem. A quick check with libxml2 2.9.2 (using lxml 3.4.1) doesn't agree:/ I noticed the problem with some java library – I'll post a comparison shortly.

[arekinath]

Any update on this?

[srenatus]

:+1: This change actually makes esaml play nice with Azure AD, and thus fixes #20.

Comparing the c14n of the Reponse's SignedInfo with the output of another tool turned out that this is the only difference (this is git diff --word-diff)

diff --git a/sigInfoCann.xml b/sigInfoCann.xml
index 77b4807..75f4b23 100644
--- a/sigInfoCann.xml
+++ b/sigInfoCann.xml
@@ -1 +1 @@
[-<ds:SignedInfo xmlns="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:ds="http://www.w3.org/20
00/09/xmldsig#"><ds:CanonicalizationMethod-]{+<ds:SignedInfo><ds:CanonicalizationMethod+} Alg
orithm="http://www.w3.org/2001/10/xml-exc-c14n#"></ds:CanonicalizationMethod><ds:SignatureMet
hod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"></ds:SignatureMethod><ds:Re
ference URI="#_4e59a276-9960-4fc1-9826-3982374a921e"><ds:Transforms><ds:Transform Algorithm="
http://www.w3.org/2000/09/xmldsig#enveloped-signature"></ds:Transform><ds:Transform Algorithm
="http://www.w3.org/2001/10/xml-exc-c14n#"></ds:Transform></ds:Transforms><ds:DigestMethod Al
gorithm="http://www.w3.org/2001/04/xmlenc#sha256"></ds:DigestMethod><ds:DigestValue>oOBhXzKPK
yKrJe0tdqRo4D0WL/Pk1KnWihZewS7eJ/8=</ds:DigestValue></ds:Reference></ds:SignedInfo>

Thanks!

[zuckschwerdt]

I could only verify with Apache Axis that this change provides compatible behavior. Especially to libxml this is an incompatible change. I'd either keep this change in an "axis-compat" branch for Axis (and posssibly Azure AD) users, or extend the change to be an optional runtime setting. (A third option could be to check if any of the two c14n styles matches with the supplied digest. I guess this would be too much work and a rather dirty approach, though.)

I'm really unhappy the specs are unclear on this and big libraries implement it differently.

[srenatus]

Especially to libxml this is an incompatible change.

I'm not sure I follow: so current esaml (without your fix) matches what libxml does?

Do we know of an IdP that expects this behaviour?

You're right, c14n shouldn't leave any variance, or "styles", though...it'd be a bad spec for this use case...

[zuckschwerdt]

@srenatus I only use esaml with Apache-Axis at this time. Hard to say how other people are using this. The change will only break libxml compatibility (and provide Axis compatibility) if there are unused default namespaces in the document. The provider can just leave those out and everything will be fine. But if there are unused default namespaces Axis (and in my opinon the c14n spec) tells us to remove those before computing the digest. libxml however will compute a digest including the unneeded namespaces.

Summing up:

• libxml gives you xml that should digest "as is" but I'd say the authors didn't read c14n careful enough.
• Axis gives you xml and wants you to remove bits before digest, one could argue according to c14n.
• But both shouldn't produce XML that is ambiguous in the first place, especially Axis, why send XML that is significantly different to the canonical form?

Going forward:

• if we know the provider type we can adapt our canonicalization behaviour, or
• we can try both digests (no harm there in my opinion, but it's rather unexpected), or
• we keep a seperate, up-to-date axis-compat branch for implementors to choose.