Merge pull request #4 from Asana/megandaly-fix-auto-canonicalization

Fix default canonicalization

Summary:
According to the Digest Method Spec [1], the DigestMethod takes a octet stream, ad if the output of the URI dereference is a node set instead, it needs to be converted as described in the Reference Processing Spec [2] to an octet stream. The Reference Processing Spec says to use Canonical XML. Enveloped Signature returns a node-set, as does dereferencing a same-document reference [3], [4]. Canonicalization algorithms are the only other transforms we support, and they all return an octet stream, so the only times we need to add canonicalization are if we haven't done any transforms or if the last one is enveloped signature.

https://www.w3.org/TR/xmldsig-core1/#sec-DigestMethod
https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel
https://www.w3.org/TR/xmldsig-core1/#sec-EnvelopedSignature
https://www.w3.org/TR/xmldsig-core1/#sec-Same-Document
Tests:
Added back the test from when this was implemented slightly differently for Windows Mobile, checked against a SamlResponse without a canonicalization method in the transforms

Reviewer: @srir

Author: meganmd

lib/signed-xml.js

@@ -570,6 +570,19 @@ SignedXml.prototype.loadReference = function(ref) {
     });
   }
 
+/**
+ * DigestMethods take an octet stream rather than a node set. If the output of the last transform is a node set, we
+ * need to canonicalize the node set to an octet stream using non-exclusive canonicalization. If there are no
+ * transforms, we need to canonicalize because URI dereferencing for a same-document reference will return a node-set.
+ * See:
+ * https://www.w3.org/TR/xmldsig-core1/#sec-DigestMethod
+ * https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel
+ * https://www.w3.org/TR/xmldsig-core1/#sec-Same-Document
+ */
+  if (transforms.length === 0 || transforms[transforms.length - 1] === "http://www.w3.org/2000/09/xmldsig#enveloped-signature") {
+      transforms.push("http://www.w3.org/TR/2001/REC-xml-c14n-20010315")
+  }
+
   this.addReference(null, transforms, digestAlgo, utils.findAttr(ref, "URI").value, digestValue, inclusiveNamespacesPrefixList, false)
 }
 

test/signature-integration-tests.js

@@ -58,7 +58,28 @@ module.exports = {
     var sig = new crypto.SignedXml()
     sig.keyInfoProvider = new crypto.FileKeyInfo("./test/static/client_public.pem")
     sig.loadSignature(signature);
-    
+    var result = sig.checkSignature(xml);
+    test.equal(result, true);
+    test.done();
+  },
+
+  "add canonicalization if output of transforms will be a node-set rather than an octet stream": function(test) {
+
+    var xml = fs.readFileSync('./test/static/windows_store_signature.xml', 'utf-8');
+
+    // Make sure that whitespace in the source document is removed -- see xml-crypto issue #23 and post at
+    //   http://webservices20.blogspot.co.il/2013/06/validating-windows-mobile-app-store.html
+    // This regex is naive but works for this test case; for a more general solution consider
+    //   the xmldom-fork-fixed library which can pass {ignoreWhiteSpace: true} into the Dom constructor.
+    xml = xml.replace(/>\s*</g, '><');
+
+    var doc = new Dom().parseFromString(xml);
+    xml = doc.firstChild.toString();
+
+    var signature = crypto.xpath(doc, "//*//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']")[0];
+    var sig = new crypto.SignedXml();
+    sig.keyInfoProvider = new crypto.FileKeyInfo("./test/static/windows_store_certificate.pem");
+    sig.loadSignature(signature);
     var result = sig.checkSignature(xml);
     test.equal(result, true);
     test.done();