Skip to content

Commit

Permalink
Fix transform processing regression
Browse files Browse the repository at this point in the history
  • Loading branch information
cjbarth committed Aug 20, 2023
1 parent 2e32d50 commit 86dbb8e
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 50 deletions.
4 changes: 2 additions & 2 deletions src/c14n-canonicalization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export class C14nCanonicalization implements CanonicalizationOrTransformationAlg
if (xpath.isComment(node)) {
return this.renderComment(node);
}
if (xpath.isComment(node)) {
if (xpath.isComment(node) || xpath.isTextNode(node)) {
return utils.encodeSpecialCharactersInText(node.data);
}

Expand Down Expand Up @@ -247,7 +247,7 @@ export class C14nCanonicalization implements CanonicalizationOrTransformationAlg
* @param node
* @api public
*/
process(node: Node, options: CanonicalizationOrTransformationAlgorithmProcessOptions) {
process(node: Node, options: CanonicalizationOrTransformationAlgorithmProcessOptions): string {
options = options || {};
const defaultNs = options.defaultNs || "";
const defaultNsForPrefix = options.defaultNsForPrefix || {};
Expand Down
2 changes: 1 addition & 1 deletion src/enveloped-signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type {

export class EnvelopedSignature implements CanonicalizationOrTransformationAlgorithm {
includeComments = false;
process(node: Node, options: CanonicalizationOrTransformationAlgorithmProcessOptions) {
process(node: Node, options: CanonicalizationOrTransformationAlgorithmProcessOptions): Node {
if (null == options.signatureNode) {
const signature = xpath.select1(
"./*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
Expand Down
12 changes: 5 additions & 7 deletions src/exclusive-canonicalization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,11 @@ export class ExclusiveCanonicalization implements CanonicalizationOrTransformati
}

/**
* Perform canonicalization of the given node
* Perform canonicalization of the given element node
*
* @param {Node} node
* @return {String}
* @api public
*/
process(node, options: CanonicalizationOrTransformationAlgorithmProcessOptions) {
process(elem: Element, options: CanonicalizationOrTransformationAlgorithmProcessOptions): string {
options = options || {};
let inclusiveNamespacesPrefixList = options.inclusiveNamespacesPrefixList || [];
const defaultNs = options.defaultNs || "";
Expand All @@ -267,7 +265,7 @@ export class ExclusiveCanonicalization implements CanonicalizationOrTransformati
* If the inclusiveNamespacesPrefixList has not been explicitly provided then look it up in CanonicalizationMethod/InclusiveNamespaces
*/
if (!utils.isArrayHasLength(inclusiveNamespacesPrefixList)) {
const CanonicalizationMethod = utils.findChildren(node, "CanonicalizationMethod");
const CanonicalizationMethod = utils.findChildren(elem, "CanonicalizationMethod");
if (CanonicalizationMethod.length !== 0) {
const inclusiveNamespaces = utils.findChildren(
CanonicalizationMethod[0],
Expand All @@ -289,7 +287,7 @@ export class ExclusiveCanonicalization implements CanonicalizationOrTransformati
if (ancestorNamespaces) {
ancestorNamespaces.forEach(function (ancestorNamespace) {
if (prefix === ancestorNamespace.prefix) {
node.setAttributeNS(
elem.setAttributeNS(
"http://www.w3.org/2000/xmlns/",
`xmlns:${prefix}`,
ancestorNamespace.namespaceURI,
Expand All @@ -301,7 +299,7 @@ export class ExclusiveCanonicalization implements CanonicalizationOrTransformati
}

const res = this.processInner(
node,
elem,
[],
defaultNs,
defaultNsForPrefix,
Expand Down
57 changes: 28 additions & 29 deletions src/signed-xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -595,39 +595,38 @@ export class SignedXml {
.flatMap((namespace) => (namespace.getAttribute("PrefixList") ?? "").split(" "))
.filter((value) => value.length > 0);
}
}

if (utils.isArrayHasLength(this.implicitTransforms)) {
this.implicitTransforms.forEach(function (t) {
transforms.push(t);
});
}
if (utils.isArrayHasLength(this.implicitTransforms)) {
this.implicitTransforms.forEach(function (t) {
transforms.push(t);

Check warning on line 602 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L601-L602

Added lines #L601 - L602 were not covered by tests
});
}

/**
* 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");
}
/**
* 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({
transforms,
digestAlgorithm: digestAlgo,
uri: xpath.isElement(refNode) ? utils.findAttr(refNode, "URI")?.value : undefined,
digestValue,
inclusiveNamespacesPrefixList,
isEmptyUri: false,
});
}
transforms,
digestAlgorithm: digestAlgo,
uri: xpath.isElement(refNode) ? utils.findAttr(refNode, "URI")?.value : undefined,
digestValue,
inclusiveNamespacesPrefixList,
isEmptyUri: false,
});
}

/**
Expand Down
9 changes: 6 additions & 3 deletions test/c14nWithComments-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ const compare = function (xml, xpathArg, expected, inclusiveNamespacesPrefixList
const doc = new xmldom.DOMParser().parseFromString(xml);
const elem = xpath.select1(xpathArg, doc);
const can = new c14nWithComments();
const result = can.process(elem, { inclusiveNamespacesPrefixList }).toString();

expect(result).to.equal(expected);
if (xpath.isElement(elem)) {
const result = can.process(elem, { inclusiveNamespacesPrefixList }).toString();
expect(result).to.equal(expected);
} else {
throw new Error("Element not found.");
}
};

describe("Exclusive canonicalization with comments", function () {
Expand Down
20 changes: 12 additions & 8 deletions test/canonicalization-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@ const compare = function (
const doc = new xmldom.DOMParser().parseFromString(xml);
const elem = xpath.select1(xpathArg, doc);
const can = new ExclusiveCanonicalization();
const result = can
.process(elem, {
inclusiveNamespacesPrefixList,
defaultNsForPrefix,
})
.toString();

expect(expected).to.equal(result);
if (xpath.isElement(elem)) {
const result = can
.process(elem, {
inclusiveNamespacesPrefixList,
defaultNsForPrefix,
})
.toString();

expect(expected).to.equal(result);
} else {
throw new Error("Invalid element");
}
};

describe("Canonicalization unit tests", function () {
Expand Down
8 changes: 8 additions & 0 deletions test/signature-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,10 @@ describe("Signature unit tests", function () {
passValidSignature("./test/static/valid_signature_with_unused_prefixes.xml");
});

it("verifies valid signature without transforms element", function () {
passValidSignature("./test/static/valid_signature_without_transforms_element.xml");
});

it("fails invalid signature - signature value", function () {
failInvalidSignature("./test/static/invalid_signature - signature value.xml");
});
Expand Down Expand Up @@ -918,6 +922,10 @@ describe("Signature unit tests", function () {
);
});

it("fails invalid signature without transforms element", function () {
failInvalidSignature("./test/static/invalid_signature_without_transforms_element.xml");
});

it("allow empty reference uri when signing", function () {
const xml = "<root><x /></root>";
const sig = new SignedXml();
Expand Down
4 changes: 4 additions & 0 deletions test/static/invalid_signature_without_transforms_element.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0"?>
<library><book ID="bookid"><name>some tampered text</name></book><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="#bookid"><DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><DigestValue>LsMoqo1d6Sqh8DKLp00MK0fSBDA=</DigestValue></Reference></SignedInfo><SignatureValue>OR1SYcyU18qELj+3DX/bW/r5DqueuyPAnNFEh3hNKFaj8ZKLB/mdsR9w8GDBCmZ2
lsCTEvJqWC37oF8rm2eBSonNbdBnA+TM6Y22C8rffVzaoM3zpNoeWMH2LwFmpdKB
UXOMWVExEaz/s4fOcyv1ajVuk42I3nl0xcD95+i7PjY=</SignatureValue></Signature></library>
4 changes: 4 additions & 0 deletions test/static/valid_signature_without_transforms_element.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0"?>
<library><book ID="bookid"><name>some text</name></book><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="#bookid"><DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><DigestValue>LsMoqo1d6Sqh8DKLp00MK0fSBDA=</DigestValue></Reference></SignedInfo><SignatureValue>OR1SYcyU18qELj+3DX/bW/r5DqueuyPAnNFEh3hNKFaj8ZKLB/mdsR9w8GDBCmZ2
lsCTEvJqWC37oF8rm2eBSonNbdBnA+TM6Y22C8rffVzaoM3zpNoeWMH2LwFmpdKB
UXOMWVExEaz/s4fOcyv1ajVuk42I3nl0xcD95+i7PjY=</SignatureValue></Signature></library>

0 comments on commit 86dbb8e

Please sign in to comment.