Merge pull request #161 from datagovsg/master

Merge changes from datagovsg fork

Author: LoneRifle

README.md

@@ -151,6 +151,30 @@ Note:
 The xml-crypto api requires you to supply it separately the xml signature ("<Signature>...</Signature>", in loadSignature) and the signed xml (in checkSignature). The signed xml may or may not contain the signature in it, but you are still required to supply the signature separately.
 
 
+### Caring for Implicit transform
+If you fail to verify signed XML, then one possible cause is that there are some hidden implicit transforms(#).  
+(#) Normalizing XML document to be verified. i.e. remove extra space within a tag, sorting attributes, importing namespace declared in ancestor nodes, etc.
+
+The reason for these implicit transform might come from [complex xml signature specification](https://www.w3.org/TR/2002/REC-xmldsig-core-20020212),
+which makes XML developers confused and then leads to incorrect implementation for signing XML document.
+
+If you keep failing verification, it is worth trying to guess such a hidden transform and specify it to the option as below:
+
+```javascript
+var option = {implicitTransforms: ["http://www.w3.org/TR/2001/REC-xml-c14n-20010315"]}
+var sig = new SignedXml(null, option)
+sig.keyInfoProvider = new FileKeyInfo("client_public.pem")
+sig.loadSignature(signature)
+var res = sig.checkSignature(xml)
+```
+
+You might find it difficult to guess such transforms, but there are typical transforms you can try.
+
+- http://www.w3.org/TR/2001/REC-xml-c14n-20010315
+- http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments
+- http://www.w3.org/2001/10/xml-exc-c14n#
+- http://www.w3.org/2001/10/xml-exc-c14n#WithComments
+
 ## API
 
 ### xpath

lib/c14n-canonicalization.js

@@ -64,10 +64,12 @@ C14nCanonicalization.prototype.renderAttrs = function(node, defaultNS) {
  * @param {Array} prefixesInScope. The prefixes defined on this node
  *                parents which are a part of the output set
  * @param {String} defaultNs. The current default namespace
+ * @param {String} defaultNsForPrefix.
+ * @param {String} ancestorNamespaces - Import ancestor namespaces if it is specified
  * @return {String}
  * @api private
  */
-C14nCanonicalization.prototype.renderNs = function(node, prefixesInScope, defaultNs, defaultNsForPrefix) {
+C14nCanonicalization.prototype.renderNs = function(node, prefixesInScope, defaultNs, defaultNsForPrefix, ancestorNamespaces) {
   var a, i, p, attr
     , res = []
     , newDefaultNs = defaultNs
@@ -93,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);
       }
@@ -107,6 +109,25 @@ C14nCanonicalization.prototype.renderNs = function(node, prefixesInScope, defaul
       }
     }
   }
+  
+  if(Array.isArray(ancestorNamespaces) && ancestorNamespaces.length > 0){
+    // Remove namespaces which are already present in nsListToRender
+    for(var p1 in ancestorNamespaces){
+      if(!ancestorNamespaces.hasOwnProperty(p1)) continue;
+      var alreadyListed = false;
+      for(var p2 in nsListToRender){
+        if(nsListToRender[p2].prefix === ancestorNamespaces[p1].prefix
+          && nsListToRender[p2].namespaceURI === ancestorNamespaces[p1].namespaceURI)
+        {
+          alreadyListed = true;
+        }
+      }
+      
+      if(!alreadyListed){
+        nsListToRender.push(ancestorNamespaces[p1]);
+      }
+    }
+  }
 
   nsListToRender.sort(this.nsCompare);
 
@@ -121,18 +142,18 @@ C14nCanonicalization.prototype.renderNs = function(node, prefixesInScope, defaul
   return {"rendered": res.join(""), "newDefaultNs": newDefaultNs};
 };
 
-C14nCanonicalization.prototype.processInner = function(node, prefixesInScope, defaultNs, defaultNsForPrefix) {
+C14nCanonicalization.prototype.processInner = function(node, prefixesInScope, defaultNs, defaultNsForPrefix, ancestorNamespaces) {
 
   if (node.nodeType === 8) { return this.renderComment(node); }
   if (node.data) { return utils.encodeSpecialCharactersInText(node.data); }
 
   var i, pfxCopy
-    , ns = this.renderNs(node, prefixesInScope, defaultNs, defaultNsForPrefix)
+    , ns = this.renderNs(node, prefixesInScope, defaultNs, defaultNsForPrefix, ancestorNamespaces)
     , res = ["<", node.tagName, ns.rendered, this.renderAttrs(node, ns.newDefaultNs), ">"];
 
   for (i = 0; i < node.childNodes.length; ++i) {
     pfxCopy = prefixesInScope.slice(0);
-    res.push(this.processInner(node.childNodes[i], pfxCopy, ns.newDefaultNs, defaultNsForPrefix));
+    res.push(this.processInner(node.childNodes[i], pfxCopy, ns.newDefaultNs, defaultNsForPrefix, []));
   }
 
   res.push("</", node.tagName, ">");
@@ -185,8 +206,9 @@ C14nCanonicalization.prototype.process = function(node, options) {
   options = options || {};
   var defaultNs = options.defaultNs || "";
   var defaultNsForPrefix = options.defaultNsForPrefix || {};
+  var ancestorNamespaces = options.ancestorNamespaces || [];
 
-  var res = this.processInner(node, [], defaultNs, defaultNsForPrefix);
+  var res = this.processInner(node, [], defaultNs, defaultNsForPrefix, ancestorNamespaces);
   return res;
 };
 

lib/enveloped-signature.js

@@ -6,7 +6,7 @@ function EnvelopedSignature() {
 }
 
 EnvelopedSignature.prototype.process = function (node) {
-  var signature = xpath(node, ".//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']")[0];  
+  var signature = xpath(node, "./*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']")[0];  
   if (signature) signature.parentNode.removeChild(signature);
   return node;
 };

lib/signed-xml.js

@@ -6,6 +6,7 @@ var select = require('xpath.js')
   , EnvelopedSignature = require('./enveloped-signature').EnvelopedSignature
   , crypto = require('crypto')
   , fs = require('fs')
+  , xpath = require('xpath.js')
 
 exports.SignedXml = SignedXml
 exports.FileKeyInfo = FileKeyInfo
@@ -197,6 +198,88 @@ function HMACSHA1() {
     };
 }
 
+
+
+/**
+ * Extract ancestor namespaces in order to import it to root of document subset
+ * which is being canonicalized for non-exclusive c14n.
+ *
+ * @param {object} doc - Usually a product from `new DOMParser().parseFromString()`
+ * @param {string} docSubsetXpath - xpath query to get document subset being canonicalized
+ * @returns {Array} i.e. [{prefix: "saml", namespaceURI: "urn:oasis:names:tc:SAML:2.0:assertion"}]
+ */
+function findAncestorNs(doc, docSubsetXpath){
+  var docSubset = xpath(doc, docSubsetXpath);
+  
+  if(!Array.isArray(docSubset) || docSubset.length < 1){
+    return [];
+  }
+  
+  // Remove duplicate on ancestor namespace
+  var ancestorNs = collectAncestorNamespaces(docSubset[0]);
+  var ancestorNsWithoutDuplicate = [];
+  for(var i=0;i<ancestorNs.length;i++){
+    var notOnTheList = true;
+    for(var v in ancestorNsWithoutDuplicate){
+      if(ancestorNsWithoutDuplicate[v].prefix === ancestorNs[i].prefix){
+        notOnTheList = false;
+        break;
+      }
+    }
+    
+    if(notOnTheList){
+      ancestorNsWithoutDuplicate.push(ancestorNs[i]);
+    }
+  }
+  
+  // Remove namespaces which are already declared in the subset with the same prefix
+  var returningNs = [];
+  var subsetAttributes = docSubset[0].attributes;
+  for(var j=0;j<ancestorNsWithoutDuplicate.length;j++){
+    var isUnique = true;
+    for(var k=0;k<subsetAttributes.length;k++){
+      var nodeName = subsetAttributes[k].nodeName;
+      if(nodeName.search(/^xmlns:/) === -1) continue;
+      var prefix = nodeName.replace(/^xmlns:/, "");
+      if(ancestorNsWithoutDuplicate[j].prefix === prefix){
+        isUnique = false;
+        break;
+      }
+    }
+  
+    if(isUnique){
+      returningNs.push(ancestorNsWithoutDuplicate[j]);
+    }
+  }
+  
+  return returningNs;
+}
+
+
+
+function collectAncestorNamespaces(node, nsArray){
+  if(!nsArray){
+    nsArray = [];
+  }
+  
+  var parent = node.parentNode;
+  
+  if(!parent){
+    return nsArray;
+  }
+  
+  if(parent.attributes && parent.attributes.length > 0){
+    for(var i=0;i<parent.attributes.length;i++){
+      var attr = parent.attributes[i];
+      if(attr && attr.nodeName && attr.nodeName.search(/^xmlns:/) !== -1){
+        nsArray.push({prefix: attr.nodeName.replace(/^xmlns:/, ""), namespaceURI: attr.nodeValue})
+      }
+    }
+  }
+  
+  return collectAncestorNamespaces(parent, nsArray);
+}
+
 /**
 * Xml signature implementation
 *
@@ -221,6 +304,7 @@ function SignedXml(idMode, options) {
   this.keyInfo = null
   this.idAttributes = [ 'Id', 'ID', 'id' ];
   if (this.options.idAttribute) this.idAttributes.splice(0, 0, this.options.idAttribute);
+  this.implicitTransforms = this.options.implicitTransforms || [];
 }
 
 SignedXml.CanonicalizationAlgorithms = {
@@ -248,6 +332,8 @@ SignedXml.defaultNsForPrefix = {
   ds: 'http://www.w3.org/2000/09/xmldsig#'
 };
 
+SignedXml.findAncestorNs = findAncestorNs;
+
 SignedXml.prototype.checkSignature = function(xml) {
   this.validationErrors = []
   this.signedXml = xml
@@ -264,18 +350,37 @@ SignedXml.prototype.checkSignature = function(xml) {
   if (!this.validateReferences(doc)) {
     return false;
   }
-
-  if (!this.validateSignatureValue()) {
+  
+  if (!this.validateSignatureValue(doc)) {
     return false;
   }
 
   return true
 }
 
-SignedXml.prototype.validateSignatureValue = function() {
+SignedXml.prototype.validateSignatureValue = function(doc) {
   var signedInfo = utils.findChilds(this.signatureNode, "SignedInfo")
   if (signedInfo.length==0) throw new Error("could not find SignedInfo element in the message")
-  var signedInfoCanon = this.getCanonXml([this.canonicalizationAlgorithm], signedInfo[0])
+  
+  /**
+   * When canonicalization algorithm is non-exclusive, search for ancestor namespaces
+   * before validating signature.
+   */
+  var ancestorNamespaces = [];
+  if(this.canonicalizationAlgorithm === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"
+  || this.canonicalizationAlgorithm === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments")
+  {
+    if(!doc || typeof(doc) !== "object"){
+      throw new Error("When canonicalization method is non-exclusive, whole xml dom must be provided as an argument");
+    }
+    
+    ancestorNamespaces = findAncestorNs(doc, "//*[local-name()='SignedInfo']");
+  }
+  
+  var c14nOptions = {
+    ancestorNamespaces: ancestorNamespaces
+  };
+  var signedInfoCanon = this.getCanonXml([this.canonicalizationAlgorithm], signedInfo[0], c14nOptions)
   var signer = this.findSignatureAlgorithm(this.signatureAlgorithm)
   var res = signer.verifySignature(signedInfoCanon, this.signingKey, this.signatureValue)
   if (!res) this.validationErrors.push("invalid signature: the signature value " +
@@ -310,6 +415,7 @@ SignedXml.prototype.validateReferences = function(doc) {
 
     var uri = ref.uri[0]=="#" ? ref.uri.substring(1) : ref.uri
     var elem = [];
+    var elemXpath;
 
     if (uri=="") {
       elem = select(doc, "//*")
@@ -322,11 +428,13 @@ SignedXml.prototype.validateReferences = function(doc) {
       var num_elements_for_id = 0;
       for (var index in this.idAttributes) {
         if (!this.idAttributes.hasOwnProperty(index)) continue;
-        var tmp_elem = select(doc, "//*[@*[local-name(.)='" + this.idAttributes[index] + "']='" + uri + "']")
+        var tmp_elemXpath = "//*[@*[local-name(.)='" + this.idAttributes[index] + "']='" + uri + "']";
+        var tmp_elem = select(doc, tmp_elemXpath)
         num_elements_for_id += tmp_elem.length;
         if (tmp_elem.length > 0) {
           elem = tmp_elem;
-        };
+          elemXpath = tmp_elemXpath;
+        }
       }
       if (num_elements_for_id > 1) {
           throw new Error('Cannot validate a document which contains multiple elements with the ' +
@@ -340,7 +448,34 @@ SignedXml.prototype.validateReferences = function(doc) {
                         ref.uri + " but could not find such element in the xml")
       return false
     }
-    var canonXml = this.getCanonXml(ref.transforms, elem[0], { inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList });
+  
+    /**
+     * When canonicalization algorithm is non-exclusive, search for ancestor namespaces
+     * before validating references.
+     */
+    if(Array.isArray(ref.transforms)){
+      var hasNonExcC14nTransform = false;
+      for(var t in ref.transforms){
+        if(!ref.transforms.hasOwnProperty(t)) continue;
+        
+        if(ref.transforms[t] === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"
+        || ref.transforms[t] === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments")
+        {
+          hasNonExcC14nTransform = true;
+          break;
+        }
+      }
+    
+      if(hasNonExcC14nTransform){
+        ref.ancestorNamespaces = findAncestorNs(doc, elemXpath);
+      }
+    }
+  
+    var c14nOptions = {
+      inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList,
+      ancestorNamespaces: ref.ancestorNamespaces
+    };
+    var canonXml = this.getCanonXml(ref.transforms, elem[0], c14nOptions);
 
     var hash = this.findHashAlgorithm(ref.digestAlgorithm)
     var digest = hash.getHash(canonXml)
@@ -428,9 +563,25 @@ SignedXml.prototype.loadReference = function(ref) {
     }
   }
 
-  //***workaround for validating windows mobile store signatures - it uses c14n but does not state it in the transforms
-  if (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#")
+  var hasImplicitTransforms = (Array.isArray(this.implicitTransforms) && this.implicitTransforms.length > 0);
+  if(hasImplicitTransforms){
+    this.implicitTransforms.forEach(function(t){
+      transforms.push(t);
+    });
+  }
+  
+/**
+ * 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)
 }
@@ -610,7 +761,7 @@ SignedXml.prototype.getCanonXml = function(transforms, node, options) {
   options = options || {};
   options.defaultNsForPrefix = options.defaultNsForPrefix || SignedXml.defaultNsForPrefix;
 
-  var canonXml = node
+  var canonXml = node.cloneNode(true) // Deep clone
 
   for (var t in transforms) {
     if (!transforms.hasOwnProperty(t)) continue;

package.json

@@ -1,6 +1,6 @@
 {
   "name": "xml-crypto",
-  "version": "0.10.1",
+  "version": "0.10.2",
   "description": "Xml digital signature and encryption library for Node.js",
   "engines": {
     "node": ">=0.4.0"
@@ -29,6 +29,6 @@
   ],
   "license": "MIT",
   "scripts": {
-    "test": "nodeunit ./test/canonicalization-unit-tests.js ./test/c14nWithComments-unit-tests.js ./test/signature-unit-tests.js ./test/saml-response-test.js ./test/signature-integration-tests.js ./test/document-test.js ./test/wsfed-metadata-test.js ./test/hmac-tests.js"
+    "test": "nodeunit ./test/canonicalization-unit-tests.js ./test/c14nWithComments-unit-tests.js ./test/signature-unit-tests.js ./test/saml-response-test.js ./test/signature-integration-tests.js ./test/document-test.js ./test/wsfed-metadata-test.js ./test/hmac-tests.js ./test/c14n-non-exclusive-unit-test.js"
   }
 }