Fixes for canonicalization and signing

- removed an old (~5y ago) hack for windows store XML canonicalization
  (and removed the test)
- fixed an issue with over-eager prefix inclusion in non-exclusive
  c14n algorithm (+ test)
- replaced a test for empty-URI that is impossible to update (because
  the private key is missing)

Author: Sri Raghavan

lib/c14n-canonicalization.js

@@ -95,8 +95,8 @@ C14nCanonicalization.prototype.renderNs = function(node, prefixesInScope, defaul
       attr = node.attributes[i];
 
       //handle all prefixed attributes that are included in the prefix list and where
-      //the prefix is not defined already
-      if (attr.prefix && prefixesInScope.indexOf(attr.localName) === -1) {
+      //the prefix is not defined already. New prefixes can only be defined by `xmlns:`.
+      if (attr.prefix === "xmlns" && prefixesInScope.indexOf(attr.localName) === -1) {
         nsListToRender.push({"prefix": attr.localName, "namespaceURI": attr.value});
         prefixesInScope.push(attr.localName);
       }

lib/signed-xml.js

@@ -570,10 +570,6 @@ SignedXml.prototype.loadReference = function(ref) {
     });
   }
 
-  //***workaround for validating windows mobile store signatures - it uses c14n but does not state it in the transforms
-  if (!hasImplicitTransforms && transforms.length==1 && transforms[0]=="http://www.w3.org/2000/09/xmldsig#enveloped-signature")
-    transforms.push("http://www.w3.org/2001/10/xml-exc-c14n#")
-
   this.addReference(null, transforms, digestAlgo, utils.findAttr(ref, "URI").value, digestValue, inclusiveNamespacesPrefixList, false)
 }
 

test/c14n-non-exclusive-unit-test.js

@@ -23,11 +23,7 @@ var test_findAncestorNs = function(test, xml, xpath, expected){
   test.done();
 };
 
-
-
-
-
-
+// Tests for findAncestorNs
 exports["findAncestorNs: Correctly picks up root ancestor namespace"] = function(test){
   var xml = "<root xmlns:aaa='bbb'><child1><child2></child2></child1></root>";
   var xpath = "/root/child1/child2";
@@ -108,15 +104,7 @@ exports["findAncestorNs: Ignores namespace declared in the target xpath node"] =
   test_findAncestorNs(test, xml, xpath, expected);
 };
 
-
-
-
-
-
-
-
-
-
+// Tests for c14nCanonicalization
 exports["C14n: Correctly picks up root ancestor namespace"] = function(test){
   var xml = "<root xmlns:aaa='bbb'><child1><child2></child2></child1></root>";
   var xpath = "/root/child1/child2";
@@ -179,4 +167,12 @@ exports["C14n: Preserve namespace declared in the target xpath node"] = function
   var expected = '<child2 xmlns:aaa="bbb" xmlns:ccc="ddd"></child2>';
 
   test_C14nCanonicalization(test, xml, xpath, expected);
-};
+};
+
+exports["C14n: Don't redeclare an attribute's namespace prefix if already in scope"] = function(test) {
+  var xml = "<root xmlns:aaa='bbb'><child1><child2 xmlns:aaa='bbb' aaa:foo='bar'></child2></child1></root>"
+  var xpath = "/root/child1/child2";
+  var expected = '<child2 xmlns:aaa="bbb" aaa:foo="bar"></child2>';
+
+  test_C14nCanonicalization(test, xml, xpath, expected);
+}

test/signature-integration-tests.js

@@ -33,60 +33,38 @@ module.exports = {
 
 
 
-  "empty URI reference should consider the whole document": function(test) {    
-
-    var sampleXml=["<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
-                           "<root>",
-                           "    <a>",
-                           "        <b/>",
-                           "    </a>",
-                           "    <Seal><Signature xmlns=\"http://www.w3.org/2000/09/xmldsig#\"><SignedInfo><CanonicalizationMethod Algorithm=\"http://www.w3.org/2001/10/xml-exc-c14n#\"/><SignatureMethod Algorithm=\"http://www.w3.org/2001/04/xmldsig-more#rsa-sha256\"/><Reference URI=\"\"><Transforms><Transform Algorithm=\"http://www.w3.org/2000/09/xmldsig#enveloped-signature\"/></Transforms><DigestMethod Algorithm=\"http://www.w3.org/2001/04/xmlenc#sha256\"/><DigestValue>FOezc5yb1O+LfQaD4UBKEUphrGzFAq5DM9B9ll37JOA=</DigestValue></Reference></SignedInfo><SignatureValue>AjkQ5NF71bwJ2YHIs8jbqva9qaNv66BYZiZw0JJZ1cW6jf3mjWShIMQZWcw78QGpzzr+ZspzUbs4",
-                           "6VAnHApJElOTDylSf3rDSvzsklKcFpHJ9yCJV+PnipEsY8qWhzKHlKCdtEn1xH0BCP/2JfMYgLQl",
-                           "PCvaR8XrgdODeQ2Gn6g=</SignatureValue><KeyInfo><KeyValue><RSAKeyValue><Modulus>t+qknJd/Kdo09fvQrRThqh/3EyDQj8zT1ZT7uXmivni4Vaysf6zHv+oORIvAt9ntZE2ZCif9v6CC",
-                           "W+hmRFkdgRoVpmD2TErjykzowx6Ffyf5BkVnVB89+g/ZqNyyvXiBe8SmpBrRLOMifnbacyrJcsrH",
-                           "fwlCnuyGKXj1LfzDcR8=</Modulus><Exponent>AQAB</Exponent></RSAKeyValue></KeyValue></KeyInfo></Signature></Seal>",
-                           "    <c>",
-                           "        <d e=\"f\"/>",
-                           "    </c>",
-                           "</root>"].join("\n");
-    
-    var doc = new Dom().parseFromString(sampleXml);    
+  "empty URI reference should consider the whole document": function(test) {
+    var xml = "<library>" +
+                "<book>" +
+                  "<name>Harry Potter</name>" +
+                "</book>" +
+              "</library>";
+
+    var signature = '<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">' + 
+                  '<SignedInfo>' +
+                    '<CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>' +
+                    '<SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>' +
+                    '<Reference URI="">' +
+                      '<Transforms>' +
+                        '<Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>' +
+                      '</Transforms>' +
+                      '<DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>' +
+                      '<DigestValue>1tjZsV007JgvE1YFe1C8sMQ+iEg=</DigestValue>' +
+                    '</Reference>' +
+                  '</SignedInfo>' +
+                  '<SignatureValue>FONRc5/nnQE2GMuEV0wK5/ofUJMHH7dzZ6VVd+oHDLfjfWax/lCMzUahJxW1i/dtm9Pl0t2FbJONVd3wwDSZzy6u5uCnj++iWYkRpIEN19RAzEMD1ejfZET8j3db9NeBq2JjrPbw81Fm7qKvte6jGa9ThTTB+1MHFRkC8qjukRM=</SignatureValue>' +
+                '</Signature>';
+
+    var sig = new crypto.SignedXml()
+    sig.keyInfoProvider = new crypto.FileKeyInfo("./test/static/client_public.pem")
+    sig.loadSignature(signature);
 
-    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/empty_uri.pem");
-    sig.loadSignature(signature);    
-    var result = sig.checkSignature(sampleXml);
-    test.equal(result, true);
-    test.done();
-  },
-
-
-
-  "windows store signature": 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();
   },
 
 
-
   "signature with inclusive namespaces": function(test) {    
 
     var xml = fs.readFileSync('./test/static/signature_with_inclusivenamespaces.xml', 'utf-8');        
@@ -190,7 +168,7 @@ function verifySignature(test, xml, expected, xpath) {
 
   var sig = new SignedXml()
   sig.signingKey = fs.readFileSync("./test/static/client.pem")
-  sig.keyInfoCaluse = null
+  sig.keyInfo = null;
 
   xpath.map(function(n) { sig.addReference(n) })