Skip to content

Commit

Permalink
Enforce consistent transform processing (#380)
Browse files Browse the repository at this point in the history
  • Loading branch information
cjbarth authored Aug 24, 2023
1 parent 2e32d50 commit fa2922f
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 10 deletions.
11 changes: 7 additions & 4 deletions src/signed-xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -948,11 +948,14 @@ export class SignedXml {
options.signatureNode = this.signatureNode;

const canonXml = node.cloneNode(true); // Deep clone
let transformedXml: string = canonXml.toString();
let transformedXml: Node | string = canonXml;

transforms.forEach((transformName) => {
const transform = this.findCanonicalizationAlgorithm(transformName);
transformedXml = transform.process(canonXml, options).toString();
if (xpath.isNodeLike(transformedXml)) {
// If, after processing, `transformedNode` is a string, we can't do anymore transforms on it
const transform = this.findCanonicalizationAlgorithm(transformName);
transformedXml = transform.process(transformedXml, options);
}
//TODO: currently transform.process may return either Node or String value (enveloped transformation returns Node, exclusive-canonicalization returns String).
//This either needs to be more explicit in the API, or all should return the same.
//exclusive-canonicalization returns String since it builds the Xml by hand. If it had used xmldom it would incorrectly minimize empty tags
Expand All @@ -962,7 +965,7 @@ export class SignedXml {
//if only y is the node to sign then a string would be <p:y/> without the definition of the p namespace. probably xmldom toString() should have added it.
});

return transformedXml;
return transformedXml.toString();
}

/**
Expand Down
7 changes: 4 additions & 3 deletions test/c14nWithComments-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,9 @@ describe("Exclusive canonicalization with comments", function () {
});

it("Multiple Canonicalization with namespace definition outside of signed element", function () {
//var doc = new Dom().parseFromString("<x xmlns:p=\"myns\"><p:y><ds:Signature xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\"></ds:Signature></p:y></x>")
const doc = new xmldom.DOMParser().parseFromString('<x xmlns:p="myns"><p:y></p:y></x>');
const doc = new xmldom.DOMParser().parseFromString(
'<x xmlns:p="myns"><p:y><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"></ds:Signature></p:y></x>',
);
const node = xpath.select1("//*[local-name(.)='y']", doc);
if (xpath.isNodeLike(node)) {
const sig = new SignedXml();
Expand All @@ -365,7 +366,7 @@ describe("Exclusive canonicalization with comments", function () {
}
});

it("Enveloped-signature canonicalization respects currentnode", function () {
it("Enveloped-signature canonicalization respects current node", function () {
// older versions of enveloped-signature removed the first signature in the whole doc, but should
// be the signature inside the current node if we want to be able to verify multiple signatures
// in a document.
Expand Down
34 changes: 31 additions & 3 deletions test/canonicalization-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,9 @@ describe("Canonicalization unit tests", function () {
});

it("Multiple Canonicalization with namespace definition outside of signed element", function () {
//var doc = new Dom().parseFromString("<x xmlns:p=\"myns\"><p:y><ds:Signature xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\"></ds:Signature></p:y></x>")
const doc = new xmldom.DOMParser().parseFromString('<x xmlns:p="myns"><p:y></p:y></x>');
const doc = new xmldom.DOMParser().parseFromString(
'<x xmlns:p="myns"><p:y><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"></ds:Signature></p:y></x>',
);
const node = xpath.select1("//*[local-name(.)='y']", doc);
if (xpath.isNodeLike(node)) {
const sig = new SignedXml();
Expand All @@ -414,7 +415,34 @@ describe("Canonicalization unit tests", function () {
}
});

it("Enveloped-signature canonicalization respects currentnode", function () {
it("Shouldn't continue processing transforms if we end up with a string as a result of a transform", function () {
const doc = new xmldom.DOMParser().parseFromString(
'<x xmlns:p="myns"><p:y><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"></ds:Signature></p:y></x>',
);
const node1 = xpath.select1("//*[local-name(.)='y']", doc);
const node2 = xpath.select1("//*[local-name(.)='y']", doc);
if (xpath.isNodeLike(node1) && xpath.isNodeLike(node2)) {
const sig = new SignedXml();
const res1 = sig.getCanonXml(
[
"http://www.w3.org/2001/10/xml-exc-c14n#",
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
],
node1,
);
const res2 = sig.getCanonXml(["http://www.w3.org/2001/10/xml-exc-c14n#"], node2);
expect(res1)
.to.equal(res2)
.to.equal(
'<p:y xmlns:p="myns"><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"></ds:Signature></p:y>',
);
} else {
expect(xpath.isNodeLike(node1)).to.be.true;
expect(xpath.isNodeLike(node2)).to.be.true;
}
});

it("Enveloped-signature canonicalization respects current node", function () {
// older versions of enveloped-signature removed the first signature in the whole doc, but should
// be the signature inside the current node if we want to be able to verify multiple signatures
// in a document.
Expand Down

0 comments on commit fa2922f

Please sign in to comment.