Skip to content

Commit

Permalink
Add additional type checking (#369)
Browse files Browse the repository at this point in the history
  • Loading branch information
@cjbarth
cjbarth authored Jul 31, 2023
1 parent 2e9cbf1 commit 226e0b2
Show file tree
Hide file tree
Showing 9 changed files with 340 additions and 252 deletions.
45 changes: 24 additions & 21 deletions src/c14n-canonicalization.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,34 +167,38 @@ export class C14nCanonicalization implements CanonicalizationOrTransformationAlg
return { rendered: res.join(""), newDefaultNs };
}

processInner(node, prefixesInScope, defaultNs, defaultNsForPrefix, ancestorNamespaces) {
processInner(node: Node, prefixesInScope, defaultNs, defaultNsForPrefix, ancestorNamespaces) {
if (xpath.isComment(node)) {
return this.renderComment(node);
}
if (node.data) {
if (xpath.isComment(node)) {
return utils.encodeSpecialCharactersInText(node.data);
}

let i;
let pfxCopy;
const ns = this.renderNs(
node,
prefixesInScope,
defaultNs,
defaultNsForPrefix,
ancestorNamespaces,
);
const res = ["<", node.tagName, ns.rendered, this.renderAttrs(node), ">"];

for (i = 0; i < node.childNodes.length; ++i) {
pfxCopy = prefixesInScope.slice(0);
res.push(
this.processInner(node.childNodes[i], pfxCopy, ns.newDefaultNs, defaultNsForPrefix, []),
if (xpath.isElement(node)) {
let i;
let pfxCopy;
const ns = this.renderNs(
node,
prefixesInScope,
defaultNs,
defaultNsForPrefix,
ancestorNamespaces,
);
const res = ["<", node.tagName, ns.rendered, this.renderAttrs(node), ">"];

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("</", node.tagName, ">");
return res.join("");
}

res.push("</", node.tagName, ">");
return res.join("");
return "";
}

// Thanks to deoxxa/xml-c14n for comment renderer
Expand Down Expand Up @@ -240,8 +244,7 @@ export class C14nCanonicalization implements CanonicalizationOrTransformationAlg
/**
* Perform canonicalization of the given node
*
* @param {Node} node
* @return {String}
* @param node
* @api public
*/
process(node: Node, options: CanonicalizationOrTransformationAlgorithmProcessOptions) {
Expand Down
31 changes: 15 additions & 16 deletions src/signed-xml.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ export class SignedXml {
return this.getCanonXml([this.canonicalizationAlgorithm], signedInfo[0], c14nOptions);
}

getCanonReferenceXml(doc, ref, node) {
getCanonReferenceXml(doc: Document, ref: Reference, node: Node) {
/**
* Search for ancestor namespaces before canonicalization.
*/
Expand Down Expand Up @@ -505,7 +505,6 @@ export class SignedXml {

const keyInfo = xpath.select1(".//*[local-name(.)='KeyInfo']", signatureNode);

// TODO: should this just be a single return instead of an array that we always take the first entry of?
if (xpath.isNodeLike(keyInfo)) {
this.keyInfo = keyInfo;
}
Expand All @@ -515,10 +514,10 @@ export class SignedXml {
* Load the reference xml node to a model
*
*/
loadReference(ref) {
let nodes = utils.findChildren(ref, "DigestMethod");
loadReference(refNode: Node) {
let nodes = utils.findChildren(refNode, "DigestMethod");
if (nodes.length === 0) {
throw new Error(`could not find DigestMethod in reference ${ref.toString()}`);
throw new Error(`could not find DigestMethod in reference ${refNode.toString()}`);
}
const digestAlgoNode = nodes[0];

Expand All @@ -528,9 +527,9 @@ export class SignedXml {
}
const digestAlgo = attr.value;

nodes = utils.findChildren(ref, "DigestValue");
nodes = utils.findChildren(refNode, "DigestValue");
if (nodes.length === 0) {
throw new Error(`could not find DigestValue node in reference ${ref.toString()}`);
throw new Error(`could not find DigestValue node in reference ${refNode.toString()}`);
}
const firstChild = nodes[0].firstChild;
if (!firstChild || !("data" in firstChild)) {
Expand All @@ -540,7 +539,7 @@ export class SignedXml {

const transforms: string[] = [];
let inclusiveNamespacesPrefixList: string[] = [];
nodes = utils.findChildren(ref, "Transforms");
nodes = utils.findChildren(refNode, "Transforms");
if (nodes.length !== 0) {
const transformsNode = nodes[0];
const transformsAll = utils.findChildren(transformsNode, "Transform");
Expand Down Expand Up @@ -590,7 +589,7 @@ export class SignedXml {
this.addReference({
transforms,
digestAlgorithm: digestAlgo,
uri: utils.findAttr(ref, "URI")?.value,
uri: xpath.isElement(refNode) ? utils.findAttr(refNode, "URI")?.value : undefined,
digestValue,
inclusiveNamespacesPrefixList,
isEmptyUri: false,
Expand Down Expand Up @@ -908,19 +907,19 @@ export class SignedXml {
}

getCanonXml(
transforms: CanonicalizationAlgorithmType[],
node,
options: CanonicalizationOrTransformationAlgorithmProcessOptions,
transforms: Reference["transforms"],
node: Node,
options: CanonicalizationOrTransformationAlgorithmProcessOptions = {},
) {
options = options || {};
options.defaultNsForPrefix = options.defaultNsForPrefix ?? SignedXml.defaultNsForPrefix;
options.signatureNode = this.signatureNode;

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

transforms.forEach((transformName) => {
const transform = this.findCanonicalizationAlgorithm(transformName);
canonXml = transform.process(canonXml, options);
transformedXml = transform.process(canonXml, options).toString();
//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 @@ -930,7 +929,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 canonXml.toString();
return transformedXml;
}

/**
Expand Down
21 changes: 12 additions & 9 deletions src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export function encodeSpecialCharactersInAttribute(attributeValue) {
});
}

export function encodeSpecialCharactersInText(text) {
export function encodeSpecialCharactersInText(text: string): string {
return text.replace(/([&<>\r])/g, function (str, item) {
// Special character normalization. See:
// - https://www.w3.org/TR/xml-c14n#ProcessingModel (Text Nodes)
Expand Down Expand Up @@ -232,16 +232,20 @@ function isElementSubset(docSubset: Node[]): docSubset is Element[] {
* 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 xmldom.DOMParser().parseFromString()`
* @param {string} docSubsetXpath - xpath query to get document subset being canonicalized
* @param {object} namespaceResolver - xpath namespace resolver
* @returns {Array} i.e. [{prefix: "saml", namespaceURI: "urn:oasis:names:tc:SAML:2.0:assertion"}]
* @param doc - Usually a product from `new xmldom.DOMParser().parseFromString()`
* @param docSubsetXpath - xpath query to get document subset being canonicalized
* @param namespaceResolver - xpath namespace resolver
* @returns i.e. [{prefix: "saml", namespaceURI: "urn:oasis:names:tc:SAML:2.0:assertion"}]
*/
export function findAncestorNs(
doc: Node,
docSubsetXpath: string,
doc: Document,
docSubsetXpath?: string,
namespaceResolver?: XPathNSResolver,
) {
): NamespacePrefix[] {
if (docSubsetXpath == null) {
return [];
}

const docSubset = xpath.selectWithResolver(docSubsetXpath, doc, namespaceResolver);

if (!isArrayHasLength(docSubset)) {
Expand Down Expand Up @@ -289,7 +293,6 @@ export function validateDigestValue(digest, expectedDigest) {
return buffer.equals(expectedBuffer);
}

// Compatibility with Node < 0.11.13
if (buffer.length !== expectedBuffer.length) {
return false;
}
Expand Down
32 changes: 19 additions & 13 deletions test/c14nWithComments-unit-tests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,19 @@ describe("Exclusive canonicalization with comments", 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 node = xpath.select1("//*[local-name(.)='y']", doc);
const sig = new SignedXml();
// @ts-expect-error FIXME
const res = sig.getCanonXml(
[
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
"http://www.w3.org/2001/10/xml-exc-c14n#",
],
node,
);
expect(res).to.equal('<p:y xmlns:p="myns"></p:y>');
if (xpath.isNodeLike(node)) {
const sig = new SignedXml();
const res = sig.getCanonXml(
[
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
"http://www.w3.org/2001/10/xml-exc-c14n#",
],
node,
);
expect(res).to.equal('<p:y xmlns:p="myns"></p:y>');
} else {
expect(xpath.isNodeLike(node)).to.be.true;
}
});

it("Enveloped-signature canonicalization respects currentnode", function () {
Expand All @@ -372,9 +375,12 @@ describe("Exclusive canonicalization with comments", function () {
const node = xpath.select1("//*[local-name(.)='y']", doc);
const sig = new SignedXml();
const transforms = ["http://www.w3.org/2000/09/xmldsig#enveloped-signature"];
// @ts-expect-error FIXME
const res = sig.getCanonXml(transforms, node);
expect(res).to.equal("<y/>");
if (xpath.isNodeLike(node)) {
const res = sig.getCanonXml(transforms, node);
expect(res).to.equal("<y/>");
} else {
expect(xpath.isNodeLike(node)).to.be.true;
}
});

it("The XML canonicalization method processes a node-set by imposing the following additional document order rules on the namespace and attribute nodes of each element: \
Expand Down
36 changes: 21 additions & 15 deletions test/canonicalization-unit-tests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,16 +399,19 @@ describe("Canonicalization unit tests", 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 node = xpath.select1("//*[local-name(.)='y']", doc);
const sig = new SignedXml();
// @ts-expect-error FIXME
const res = sig.getCanonXml(
[
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
"http://www.w3.org/2001/10/xml-exc-c14n#",
],
node,
);
expect(res).to.equal('<p:y xmlns:p="myns"></p:y>');
if (xpath.isNodeLike(node)) {
const sig = new SignedXml();
const res = sig.getCanonXml(
[
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
"http://www.w3.org/2001/10/xml-exc-c14n#",
],
node,
);
expect(res).to.equal('<p:y xmlns:p="myns"></p:y>');
} else {
expect(xpath.isNodeLike(node)).to.be.true;
}
});

it("Enveloped-signature canonicalization respects currentnode", function () {
Expand All @@ -419,11 +422,14 @@ describe("Canonicalization unit tests", function () {
'<x><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#" /><y><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#" /></y></x>';
const doc = new xmldom.DOMParser().parseFromString(xml);
const node = xpath.select1("//*[local-name(.)='y']", doc);
const sig = new SignedXml();
const transforms = ["http://www.w3.org/2000/09/xmldsig#enveloped-signature"];
// @ts-expect-error FIXME
const res = sig.getCanonXml(transforms, node);
expect(res).to.equal("<y/>");
if (xpath.isNodeLike(node)) {
const sig = new SignedXml();
const transforms = ["http://www.w3.org/2000/09/xmldsig#enveloped-signature"];
const res = sig.getCanonXml(transforms, node);
expect(res).to.equal("<y/>");
} else {
expect(xpath.isNodeLike(node)).to.be.true;
}
});

it("The XML canonicalization method processes a node-set by imposing the following additional document order rules on the namespace and attribute nodes of each element: \
Expand Down
6 changes: 5 additions & 1 deletion test/key-info-tests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ describe("KeyInfo tests", function () {
const doc = new xmldom.DOMParser().parseFromString(signedXml);
const x509 = xpath.select("//*[local-name(.)='X509Certificate']", doc.documentElement);
// @ts-expect-error FIXME
expect(x509.length, "X509Certificate element should exist").to.equal(1);
if (xpath.isArrayOfNodes(x509)) {
expect(x509.length, "X509Certificate element should exist").to.equal(1);
} else {
expect(xpath.isArrayOfNodes(x509)).to.be.true;
}
});

it("make sure private hmac key is not leaked due to key confusion", function () {
Expand Down
Loading

0 comments on commit 226e0b2

Please sign in to comment.