Skip to content

Commit 6c30b52

Browse files
vadimavdeevLoneRifle
vadimavdeev
authored and
LoneRifle
committed
Fix canon xml being computed differently when signing, than when verifying (#183)
When computing the signature, ancestor namespaces of SignedInfo and reference node were not passed into the 'getCanonXml' method like they were during signature verification This attempts to fix #164
1 parent 71adf90 commit 6c30b52

File tree

2 files changed

+75
-54
lines changed

2 files changed

+75
-54
lines changed

‎lib/signed-xml.js

Lines changed: 59 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -357,35 +357,63 @@ SignedXml.prototype.checkSignature = function(xml) {
357357
return true
358358
}
359359

360-
SignedXml.prototype.validateSignatureValue = function(doc) {
360+
SignedXml.prototype.getCanonSignedInfoXml = function(doc) {
361361
var signedInfo = utils.findChilds(this.signatureNode, "SignedInfo")
362362
if (signedInfo.length==0) throw new Error("could not find SignedInfo element in the message")
363363

364364
if(this.canonicalizationAlgorithm === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"
365365
|| this.canonicalizationAlgorithm === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments")
366366
{
367367
if(!doc || typeof(doc) !== "object"){
368-
throw new Error("When canonicalization method is non-exclusive, whole xml dom must be provided as an argument");
368+
throw new Error(
369+
"When canonicalization method is non-exclusive, whole xml dom must be provided as an argument"
370+
);
369371
}
370372
}
371373

372374
/**
373-
* Search for ancestor namespaces before validating signature.
375+
* Search for ancestor namespaces before canonicalization.
374376
*/
375377
var ancestorNamespaces = [];
376378
ancestorNamespaces = findAncestorNs(doc, "//*[local-name()='SignedInfo']");
377379

378380
var c14nOptions = {
379381
ancestorNamespaces: ancestorNamespaces
380382
};
381-
var signedInfoCanon = this.getCanonXml([this.canonicalizationAlgorithm], signedInfo[0], c14nOptions)
383+
return this.getCanonXml([this.canonicalizationAlgorithm], signedInfo[0], c14nOptions)
384+
}
385+
386+
SignedXml.prototype.getCanonReferenceXml = function(doc, ref, node) {
387+
/**
388+
* Search for ancestor namespaces before canonicalization.
389+
*/
390+
if(Array.isArray(ref.transforms)){
391+
ref.ancestorNamespaces = findAncestorNs(doc, ref.xpath)
392+
}
393+
394+
var c14nOptions = {
395+
inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList,
396+
ancestorNamespaces: ref.ancestorNamespaces
397+
}
398+
399+
return this.getCanonXml(ref.transforms, node, c14nOptions)
400+
}
401+
402+
SignedXml.prototype.validateSignatureValue = function(doc) {
403+
var signedInfoCanon = this.getCanonSignedInfoXml(doc)
382404
var signer = this.findSignatureAlgorithm(this.signatureAlgorithm)
383405
var res = signer.verifySignature(signedInfoCanon, this.signingKey, this.signatureValue)
384406
if (!res) this.validationErrors.push("invalid signature: the signature value " +
385407
this.signatureValue + " is incorrect")
386408
return res
387409
}
388410

411+
SignedXml.prototype.calculateSignatureValue = function(doc) {
412+
var signedInfoCanon = this.getCanonSignedInfoXml(doc)
413+
var signer = this.findSignatureAlgorithm(this.signatureAlgorithm)
414+
this.signatureValue = signer.getSignature(signedInfoCanon, this.signingKey)
415+
}
416+
389417
SignedXml.prototype.findSignatureAlgorithm = function(name) {
390418
var algo = SignedXml.SignatureAlgorithms[name]
391419
if (algo) return new algo()
@@ -413,7 +441,6 @@ SignedXml.prototype.validateReferences = function(doc) {
413441

414442
var uri = ref.uri[0]=="#" ? ref.uri.substring(1) : ref.uri
415443
var elem = [];
416-
var elemXpath;
417444

418445
if (uri=="") {
419446
elem = xpath.select("//*", doc)
@@ -423,6 +450,7 @@ SignedXml.prototype.validateReferences = function(doc) {
423450
throw new Error("Cannot validate a uri with quotes inside it");
424451
}
425452
else {
453+
var elemXpath;
426454
var num_elements_for_id = 0;
427455
for (var index in this.idAttributes) {
428456
if (!this.idAttributes.hasOwnProperty(index)) continue;
@@ -439,6 +467,8 @@ SignedXml.prototype.validateReferences = function(doc) {
439467
'same value for the ID / Id / Id attributes, in order to prevent ' +
440468
'signature wrapping attack.');
441469
}
470+
471+
ref.xpath = elemXpath;
442472
}
443473

444474
if (elem.length==0) {
@@ -447,19 +477,7 @@ SignedXml.prototype.validateReferences = function(doc) {
447477
return false
448478
}
449479

450-
/**
451-
* Search for ancestor namespaces before validating references.
452-
*/
453-
if(Array.isArray(ref.transforms)){
454-
ref.ancestorNamespaces = findAncestorNs(doc, elemXpath);
455-
}
456-
457-
var c14nOptions = {
458-
inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList,
459-
ancestorNamespaces: ref.ancestorNamespaces
460-
};
461-
462-
var canonXml = this.getCanonXml(ref.transforms, elem[0], c14nOptions);
480+
var canonXml = this.getCanonReferenceXml(doc, ref, elem[0])
463481

464482
var hash = this.findHashAlgorithm(ref.digestAlgorithm)
465483
var digest = hash.getHash(canonXml)
@@ -679,13 +697,11 @@ SignedXml.prototype.computeSignature = function(xml, opts) {
679697
// add the xml namespace attribute
680698
signatureAttrs.push(xmlNsAttr + "=\"http://www.w3.org/2000/09/xmldsig#\"");
681699

682-
this.signatureXml = "<" + currentPrefix + "Signature " + signatureAttrs.join(" ") + ">"
700+
var signatureXml = "<" + currentPrefix + "Signature " + signatureAttrs.join(" ") + ">"
683701

684-
var signedInfo = this.createSignedInfo(doc, prefix);
685-
this.signatureXml += signedInfo;
686-
this.signatureXml += this.createSignature(signedInfo, prefix);
687-
this.signatureXml += this.getKeyInfo(prefix)
688-
this.signatureXml += "</" + currentPrefix + "Signature>"
702+
signatureXml += this.createSignedInfo(doc, prefix);
703+
signatureXml += this.getKeyInfo(prefix)
704+
signatureXml += "</" + currentPrefix + "Signature>"
689705

690706
this.originalXmlWithIds = doc.toString()
691707

@@ -696,7 +712,7 @@ SignedXml.prototype.computeSignature = function(xml, opts) {
696712

697713
// A trick to remove the namespaces that already exist in the xml
698714
// This only works if the prefix and namespace match with those in te xml
699-
var dummySignatureWrapper = "<Dummy " + existingPrefixesString + ">" + this.signatureXml + "</Dummy>"
715+
var dummySignatureWrapper = "<Dummy " + existingPrefixesString + ">" + signatureXml + "</Dummy>"
700716
var xml = new Dom().parseFromString(dummySignatureWrapper)
701717
var signatureDoc = xml.documentElement.firstChild;
702718

@@ -718,6 +734,16 @@ SignedXml.prototype.computeSignature = function(xml, opts) {
718734
referenceNode.parentNode.insertBefore(signatureDoc, referenceNode.nextSibling);
719735
}
720736

737+
this.signatureNode = signatureDoc
738+
this.calculateSignatureValue(doc)
739+
740+
var signedInfoNode = utils.findChilds(this.signatureNode, "SignedInfo")
741+
if (signedInfoNode.length==0) throw new Error("could not find SignedInfo element in the message")
742+
743+
signedInfoNode = signedInfoNode[0];
744+
signatureDoc.insertBefore(this.createSignature(prefix), signedInfoNode.nextSibling)
745+
746+
this.signatureXml = signatureDoc.toString()
721747
this.signedXml = doc.toString()
722748
}
723749

@@ -778,7 +804,7 @@ SignedXml.prototype.createReferences = function(doc, prefix) {
778804
res += "<" + prefix + "Transform Algorithm=\"" + transform.getAlgorithmName() + "\" />"
779805
}
780806

781-
var canonXml = this.getCanonXml(ref.transforms, node)
807+
var canonXml = this.getCanonReferenceXml(doc, ref, node)
782808

783809
var digestAlgorithm = this.findHashAlgorithm(ref.digestAlgorithm)
784810
res += "</" + prefix + "Transforms>"+
@@ -880,7 +906,7 @@ SignedXml.prototype.createSignedInfo = function(doc, prefix) {
880906
* Create the Signature element
881907
*
882908
*/
883-
SignedXml.prototype.createSignature = function(signedInfo, prefix) {
909+
SignedXml.prototype.createSignature = function(prefix) {
884910
var xmlNsAttr = 'xmlns'
885911

886912
if (prefix) {
@@ -890,20 +916,15 @@ SignedXml.prototype.createSignature = function(signedInfo, prefix) {
890916
prefix = '';
891917
}
892918

919+
var signatureValueXml = "<" + prefix + "SignatureValue>" + this.signatureValue + "</" + prefix + "SignatureValue>"
893920
//the canonicalization requires to get a valid xml node.
894921
//we need to wrap the info in a dummy signature since it contains the default namespace.
895922
var dummySignatureWrapper = "<" + prefix + "Signature " + xmlNsAttr + "=\"http://www.w3.org/2000/09/xmldsig#\">" +
896-
signedInfo +
897-
"</" + prefix + "Signature>"
923+
signatureValueXml +
924+
"</" + prefix + "Signature>"
898925

899-
var xml = new Dom().parseFromString(dummySignatureWrapper)
900-
//get the signedInfo
901-
var node = xml.documentElement.firstChild;
902-
var canAlgorithm = new this.findCanonicalizationAlgorithm(this.canonicalizationAlgorithm)
903-
var canonizedSignedInfo = canAlgorithm.process(node)
904-
var signatureAlgorithm = this.findSignatureAlgorithm(this.signatureAlgorithm)
905-
this.signatureValue = signatureAlgorithm.getSignature(canonizedSignedInfo, this.signingKey)
906-
return "<" + prefix + "SignatureValue>" + this.signatureValue + "</" + prefix + "SignatureValue>"
926+
var doc = new Dom().parseFromString(dummySignatureWrapper)
927+
return doc.documentElement.firstChild;
907928
}
908929

909930

@@ -917,4 +938,4 @@ SignedXml.prototype.getOriginalXmlWithIds = function() {
917938

918939
SignedXml.prototype.getSignedXml = function() {
919940
return this.signedXml
920-
}
941+
}

‎test/signature-unit-tests.js

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -194,27 +194,27 @@ module.exports = {
194194
var signature = sig.getSignatureXml()
195195
var expected = "<Signature xmlns=\"http://www.w3.org/2000/09/xmldsig#\">"+
196196
"<SignedInfo>"+
197-
"<CanonicalizationMethod Algorithm=\"dummy canonicalization\" />"+
198-
"<SignatureMethod Algorithm=\"dummy algorithm\" />"+
197+
"<CanonicalizationMethod Algorithm=\"dummy canonicalization\"/>"+
198+
"<SignatureMethod Algorithm=\"dummy algorithm\"/>"+
199199
"<Reference URI=\"#_0\">"+
200200
"<Transforms>"+
201-
"<Transform Algorithm=\"dummy transformation\" />"+
201+
"<Transform Algorithm=\"dummy transformation\"/>"+
202202
"</Transforms>"+
203-
"<DigestMethod Algorithm=\"dummy digest algorithm\" />"+
203+
"<DigestMethod Algorithm=\"dummy digest algorithm\"/>"+
204204
"<DigestValue>dummy digest</DigestValue>"+
205205
"</Reference>"+
206206
"<Reference URI=\"#_1\">"+
207207
"<Transforms>"+
208-
"<Transform Algorithm=\"dummy transformation\" />"+
208+
"<Transform Algorithm=\"dummy transformation\"/>"+
209209
"</Transforms>"+
210-
"<DigestMethod Algorithm=\"dummy digest algorithm\" />"+
210+
"<DigestMethod Algorithm=\"dummy digest algorithm\"/>"+
211211
"<DigestValue>dummy digest</DigestValue>"+
212212
"</Reference>"+
213213
"<Reference URI=\"#_2\">"+
214214
"<Transforms>"+
215-
"<Transform Algorithm=\"dummy transformation\" />"+
215+
"<Transform Algorithm=\"dummy transformation\"/>"+
216216
"</Transforms>"+
217-
"<DigestMethod Algorithm=\"dummy digest algorithm\" />"+
217+
"<DigestMethod Algorithm=\"dummy digest algorithm\"/>"+
218218
"<DigestValue>dummy digest</DigestValue>"+
219219
"</Reference>"+
220220
"</SignedInfo>"+
@@ -347,27 +347,27 @@ module.exports = {
347347

348348
var expected = "<ds:Signature xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\">"+
349349
"<ds:SignedInfo>"+
350-
"<ds:CanonicalizationMethod Algorithm=\"dummy canonicalization\" />"+
351-
"<ds:SignatureMethod Algorithm=\"dummy algorithm\" />"+
350+
"<ds:CanonicalizationMethod Algorithm=\"dummy canonicalization\"/>"+
351+
"<ds:SignatureMethod Algorithm=\"dummy algorithm\"/>"+
352352
"<ds:Reference URI=\"#_0\">"+
353353
"<ds:Transforms>"+
354-
"<ds:Transform Algorithm=\"dummy transformation\" />"+
354+
"<ds:Transform Algorithm=\"dummy transformation\"/>"+
355355
"</ds:Transforms>"+
356-
"<ds:DigestMethod Algorithm=\"dummy digest algorithm\" />"+
356+
"<ds:DigestMethod Algorithm=\"dummy digest algorithm\"/>"+
357357
"<ds:DigestValue>dummy digest</ds:DigestValue>"+
358358
"</ds:Reference>"+
359359
"<ds:Reference URI=\"#_1\">"+
360360
"<ds:Transforms>"+
361-
"<ds:Transform Algorithm=\"dummy transformation\" />"+
361+
"<ds:Transform Algorithm=\"dummy transformation\"/>"+
362362
"</ds:Transforms>"+
363-
"<ds:DigestMethod Algorithm=\"dummy digest algorithm\" />"+
363+
"<ds:DigestMethod Algorithm=\"dummy digest algorithm\"/>"+
364364
"<ds:DigestValue>dummy digest</ds:DigestValue>"+
365365
"</ds:Reference>"+
366366
"<ds:Reference URI=\"#_2\">"+
367367
"<ds:Transforms>"+
368-
"<ds:Transform Algorithm=\"dummy transformation\" />"+
368+
"<ds:Transform Algorithm=\"dummy transformation\"/>"+
369369
"</ds:Transforms>"+
370-
"<ds:DigestMethod Algorithm=\"dummy digest algorithm\" />"+
370+
"<ds:DigestMethod Algorithm=\"dummy digest algorithm\"/>"+
371371
"<ds:DigestValue>dummy digest</ds:DigestValue>"+
372372
"</ds:Reference>"+
373373
"</ds:SignedInfo>"+

0 commit comments

Comments
 (0)