diff --git a/README.md b/README.md index 891e91c..ee3b27f 100644 --- a/README.md +++ b/README.md @@ -267,7 +267,9 @@ To sign xml documents: - `addReference({ xpath, transforms, digestAlgorithm, id, type })` - adds a reference to a xml element where: - `xpath` - a string containing a XPath expression referencing a xml element - - `transforms` - an array of [transform algorithms](#canonicalization-and-transformation-algorithms), the referenced element will be transformed for each value in the array + - `transforms` - an array of [transform algorithms](#canonicalization-and-transformation-algorithms), the referenced element will be transformed for each value in the array. + Omit it, or pass an empty array, to emit no `Transforms` element; the referenced element is then digested after canonicalization alone. + If the referenced element encloses the signature, this array has to include `http://www.w3.org/2000/09/xmldsig#enveloped-signature`, which removes the `Signature` before digesting; `computeSignature()` rejects the reference otherwise rather than emit a signature that cannot verify. Supply your own transform and that check defers to it, since only you know whether it removes the `Signature`. - `digestAlgorithm` - one of the supported [hashing algorithms](#hashing-algorithms) - `id` - an optional `Id` attribute to add to the reference element - `type` - the optional `Type` attribute to add to the reference element (represented as a URI) @@ -342,6 +344,9 @@ Custom transformation algorithm. ```javascript function MyTransformation() { + /*whether this drops nodes from the node-set, rather than only re-serializing it*/ + this.removesNodes = false; + /*given a node (from the xmldom module) return its canonical representation (as string)*/ this.process = function (node) { //you should apply your transformation before returning @@ -354,10 +359,16 @@ function MyTransformation() { } ``` +`removesNodes` is required on every canonicalization and transformation algorithm, including ones written in plain JavaScript. It is checked as the algorithm is instantiated, so a missing or non-boolean value throws from `checkSignature()` just as it does from `computeSignature()` — a verifier whose own registry is misconfigured refuses rather than canonicalizing with an algorithm that never said what it does. Registrations you never use are not checked. + +Declare `false` if the algorithm returns the node-set it was given, and `true` if it filters nodes at all. `computeSignature()` also reads this to decide whether a reference enclosing the signature can ever verify, and the two mistakes cost you different things: declaring `true` when the algorithm actually preserves everything skips that check, so you can sign a reference nobody can verify, while declaring `false` when it actually filters makes the check reject a reference that would have worked. + Custom canonicalization is actually the same as custom transformation. It is applied on the SignedInfo rather than on references. ```javascript function MyCanonicalization() { + this.removesNodes = false; + /*given a node (from the xmldom module) return its canonical representation (as string)*/ this.process = function (node) { //you should apply your transformation before returning diff --git a/src/c14n-canonicalization.ts b/src/c14n-canonicalization.ts index a77bc21..61c0cf8 100644 --- a/src/c14n-canonicalization.ts +++ b/src/c14n-canonicalization.ts @@ -8,6 +8,8 @@ import * as utils from "./utils"; import * as isDomNode from "@xmldom/is-dom-node"; export class C14nCanonicalization implements CanonicalizationOrTransformationAlgorithm { + readonly removesNodes: boolean = false; + protected includeComments = false; constructor() { diff --git a/src/enveloped-signature.ts b/src/enveloped-signature.ts index d234bc5..6d1c102 100644 --- a/src/enveloped-signature.ts +++ b/src/enveloped-signature.ts @@ -9,6 +9,8 @@ import type { } from "./types"; export class EnvelopedSignature implements CanonicalizationOrTransformationAlgorithm { + readonly removesNodes: boolean = true; + protected includeComments = false; constructor() { diff --git a/src/exclusive-canonicalization.ts b/src/exclusive-canonicalization.ts index ea88aa2..062b5e4 100644 --- a/src/exclusive-canonicalization.ts +++ b/src/exclusive-canonicalization.ts @@ -18,6 +18,8 @@ function isPrefixInScope(prefixesInScope, prefix, namespaceURI) { } export class ExclusiveCanonicalization implements CanonicalizationOrTransformationAlgorithm { + readonly removesNodes: boolean = false; + protected includeComments = false; constructor() { diff --git a/src/index.ts b/src/index.ts index 3c82b7a..a11dec8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,5 +4,50 @@ export { ExclusiveCanonicalizationWithComments, } from "./exclusive-canonicalization"; export { SignedXml } from "./signed-xml"; -export * from "./types"; -export * from "./utils"; + +// These lists replace `export * from` and are exhaustive on purpose: they +// reproduce the surface the wildcards already published, so that narrowing it +// becomes a deliberate, separately reviewable break rather than a side effect. +export { createOptionalCallbackFunction } from "./types"; +export type { + CanonicalizationAlgorithmType, + CanonicalizationOrTransformAlgorithmType, + CanonicalizationOrTransformationAlgorithm, + CanonicalizationOrTransformationAlgorithmProcessOptions, + ComputeSignatureOptions, + ComputeSignatureOptionsLocation, + ErrorFirstCallback, + GetKeyInfoContentArgs, + HashAlgorithm, + HashAlgorithmType, + NamespacePrefix, + ObjectAttributes, + Reference, + RenderedNamespace, + SignatureAlgorithm, + SignatureAlgorithmType, + SignedXmlOptions, + TransformAlgorithm, +} from "./types"; + +export { + BASE64_REGEX, + EXTRACT_X509_CERTS, + PEM_FORMAT_REGEX, + derToPem, + encodeSpecialCharactersInAttribute, + encodeSpecialCharactersInText, + findAncestorNs, + findAncestorNsForNode, + findAttr, + findChildren, + // Deprecated alias of `findChildren`, still published because `export *` did. + // Removal tracked in https://github.com/node-saml/xml-crypto/issues/550 + // eslint-disable-next-line deprecation/deprecation + findChilds, + isArrayHasLength, + isDescendantOf, + normalizePem, + pemToDer, + validateDigestValue, +} from "./utils"; diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 843a53b..04165f5 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -433,13 +433,12 @@ export class SignedXml { return this.getCanonXml([this.canonicalizationAlgorithm], signedInfo[0], c14nOptions); } - private getCanonReferenceXml(doc: Document, ref: Reference, node: Node) { - /** - * Search for ancestor namespaces before canonicalization. - */ - if (Array.isArray(ref.transforms)) { - ref.ancestorNamespaces = utils.findAncestorNs(doc, ref.xpath, this.namespaceResolver); - } + private getCanonReferenceXml(ref: Reference, node: Node) { + // Derive the scope from the node being digested: findAncestorNs re-runs + // ref.xpath and keeps only the first match, which digests every reference + // addAllReferences() created from that xpath in the first match's scope. + isDomNode.assertIsElementNode(node); + ref.ancestorNamespaces = utils.findAncestorNsForNode(node); const c14nOptions = { inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList, @@ -478,7 +477,17 @@ export class SignedXml { if (name != null) { const algo = this.CanonicalizationAlgorithms[name]; if (algo) { - return new algo(); + const instance = new algo(); + // Registering is reachable from JavaScript, where the required type buys + // nothing. Checking here covers every algorithm we instantiate, rather than + // only the ones a particular caller happens to look at. + if (typeof instance.removesNodes !== "boolean") { + throw new Error( + `canonicalization algorithm '${name}' must declare a boolean 'removesNodes'`, + ); + } + + return instance; } } @@ -516,7 +525,7 @@ export class SignedXml { } } - const canonXml = this.getCanonReferenceXml(doc, ref, elem); + const canonXml = this.getCanonReferenceXml(ref, elem); const hash = this.findHashAlgorithm(ref.digestAlgorithm); const digest = hash.getHash(canonXml); @@ -576,7 +585,7 @@ export class SignedXml { return false; } - const canonXml = this.getCanonReferenceXml(doc, ref, elem); + const canonXml = this.getCanonReferenceXml(ref, elem); const hash = this.findHashAlgorithm(ref.digestAlgorithm); const digest = hash.getHash(canonXml); @@ -825,13 +834,9 @@ export class SignedXml { throw new Error("digestAlgorithm is required"); } - if (!utils.isArrayHasLength(transforms)) { - throw new Error("transforms must contain at least one transform algorithm"); - } - this.references.push({ xpath, - transforms, + transforms: transforms ?? [], digestAlgorithm, uri, digestValue, @@ -1141,6 +1146,22 @@ export class SignedXml { ); } + // The Signature is already in the document, and its SignatureValue is still + // empty, so digesting content that encloses it can never match on verification. + // https://www.w3.org/TR/xmldsig-core1/#sec-EnvelopedSignature + if ( + utils.isDescendantOf(signatureElem, node) && + (ref.transforms ?? []) + .map((transform) => this.findCanonicalizationAlgorithm(transform)) + .every((algorithm) => !algorithm.removesNodes) + ) { + throw new Error( + `The reference ${ref.xpath} encloses the signature, so it requires the ` + + "http://www.w3.org/2000/09/xmldsig#enveloped-signature transform. " + + "Without it the signature cannot be verified.", + ); + } + // Compute the target URI (ID already ensured earlier, extract it) let targetUri: string; if (ref.isEmptyUri) { @@ -1166,36 +1187,40 @@ export class SignedXml { referenceElem.setAttribute("Type", ref.type); } - const transformsElem = signatureDoc.createElementNS( - signatureNamespace, - `${currentPrefix}Transforms`, - ); - - for (const trans of ref.transforms || []) { - const transform = this.findCanonicalizationAlgorithm(trans); - const transformElem = signatureDoc.createElementNS( + if (utils.isArrayHasLength(ref.transforms)) { + const transformsElem = signatureDoc.createElementNS( signatureNamespace, - `${currentPrefix}Transform`, + `${currentPrefix}Transforms`, ); - transformElem.setAttribute("Algorithm", transform.getAlgorithmName()); - if (utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList)) { - const inclusiveNamespacesElem = signatureDoc.createElementNS( - transform.getAlgorithmName(), - "InclusiveNamespaces", + for (const trans of ref.transforms) { + const transform = this.findCanonicalizationAlgorithm(trans); + const transformElem = signatureDoc.createElementNS( + signatureNamespace, + `${currentPrefix}Transform`, ); - inclusiveNamespacesElem.setAttribute( - "PrefixList", - ref.inclusiveNamespacesPrefixList.join(" "), - ); - transformElem.appendChild(inclusiveNamespacesElem); + transformElem.setAttribute("Algorithm", transform.getAlgorithmName()); + + if (utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList)) { + const inclusiveNamespacesElem = signatureDoc.createElementNS( + transform.getAlgorithmName(), + "InclusiveNamespaces", + ); + inclusiveNamespacesElem.setAttribute( + "PrefixList", + ref.inclusiveNamespacesPrefixList.join(" "), + ); + transformElem.appendChild(inclusiveNamespacesElem); + } + + transformsElem.appendChild(transformElem); } - transformsElem.appendChild(transformElem); + referenceElem.appendChild(transformsElem); } // Get the canonicalized XML - const canonXml = this.getCanonReferenceXml(doc, ref, node); + const canonXml = this.getCanonReferenceXml(ref, node); // Get the digest algorithm and compute the digest value const digestAlgorithm = this.findHashAlgorithm(ref.digestAlgorithm); @@ -1212,7 +1237,6 @@ export class SignedXml { ); digestValueElem.textContent = digestAlgorithm.getHash(canonXml); - referenceElem.appendChild(transformsElem); referenceElem.appendChild(digestMethodElem); referenceElem.appendChild(digestValueElem); @@ -1313,6 +1337,21 @@ export class SignedXml { //if only y is the node to sign then a string would be without the definition of the p namespace. probably xmldom toString() should have added it. }); + if (typeof transformedXml === "string") { + return transformedXml; + } + + // A same-document reference dereferences to a node-set, which must be + // canonicalized to an octet stream before digesting. `loadReference` appends + // the same C14N on the verification side, so both sides digest equal bytes. + // https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel + if (!utils.isArrayHasLength(transforms)) { + const c14n = this.findCanonicalizationAlgorithm( + "http://www.w3.org/TR/2001/REC-xml-c14n-20010315", + ); + return String(c14n.process(transformedXml, options)); + } + return transformedXml.toString(); } diff --git a/src/types.ts b/src/types.ts index 08c4300..cb6662e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -126,6 +126,8 @@ export interface Reference { xpath?: string; // An array of transforms to be applied to the data before signing. + // An empty array emits no Transforms element; the referenced node is then + // digested directly, after C14N, per the XMLDSig processing model. transforms: ReadonlyArray; // The algorithm used to calculate the digest value of the data. @@ -167,6 +169,29 @@ export interface CanonicalizationOrTransformationAlgorithm { ): Node | string; getAlgorithmName(): CanonicalizationOrTransformAlgorithmType; + + /** + * Whether this algorithm can drop nodes from the node-set it is given, rather + * than only re-serializing it. Canonicalization algorithms declare `false`; + * the enveloped-signature transform declares `true`. + * + * Required, and enforced on both sides: every algorithm is checked as it is + * instantiated, so a missing or non-boolean value throws from `computeSignature()` + * and from `checkSignature()` alike. That covers algorithms registered from + * JavaScript, which this type cannot reach. Registrations that are never used are + * not checked. + * + * `computeSignature()` also reads the value to decide whether a reference that + * encloses the signature can ever verify: if every transform on that reference + * declares `false`, the `Signature` provably survives into the digest, so the + * reference is rejected rather than signed into something unverifiable. + * + * Declare `false` if the algorithm returns the node-set it was given, `true` if it + * filters nodes at all. Declaring `true` when it actually preserves everything + * skips the check above and can sign a reference nobody can verify; declaring + * `false` when it actually filters can reject a reference that would have worked. + */ + removesNodes: boolean; } /** Implement this to create a new HashAlgorithm */ diff --git a/src/utils.ts b/src/utils.ts index dc2dbf4..3c1656b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -240,6 +240,35 @@ function isElementSubset(docSubset: Node[]): docSubset is Element[] { return docSubset.every((node) => isDomNode.isElementNode(node)); } +/** + * Extract ancestor namespaces for an element that is already resolved. + * Prefer this over `findAncestorNs` when the target element is known, since + * `findAncestorNs` re-executes an XPath and uses only the first match. + * + * @param element - The element whose ancestor namespace declarations to collect + * @returns i.e. [{prefix: "saml", namespaceURI: "urn:oasis:names:tc:SAML:2.0:assertion"}] + */ +export function findAncestorNsForNode(element: Element): NamespacePrefix[] { + const ancestorNs = collectAncestorNamespaces(element); + const ancestorNsWithoutDuplicate: NamespacePrefix[] = []; + for (const ns of ancestorNs) { + const isDuplicate = ancestorNsWithoutDuplicate.some((seen) => seen.prefix === ns.prefix); + if (!isDuplicate) { + ancestorNsWithoutDuplicate.push(ns); + } + } + + const returningNs: NamespacePrefix[] = []; + const subsetNsPrefixes = findSubsetNSPrefixes(element); + for (const ns of ancestorNsWithoutDuplicate) { + if (!subsetNsPrefixes.has(ns.prefix)) { + returningNs.push(ns); + } + } + + return returningNs; +} + /** * Extract ancestor namespaces in order to import it to root of document subset * which is being canonicalized for non-exclusive c14n. @@ -268,24 +297,7 @@ export function findAncestorNs( throw new Error("Document subset must be list of elements"); } - const ancestorNs = collectAncestorNamespaces(docSubset[0]); - const ancestorNsWithoutDuplicate: NamespacePrefix[] = []; - for (const ns of ancestorNs) { - const isDuplicate = ancestorNsWithoutDuplicate.some((seen) => seen.prefix === ns.prefix); - if (!isDuplicate) { - ancestorNsWithoutDuplicate.push(ns); - } - } - - const returningNs: NamespacePrefix[] = []; - const subsetNsPrefixes = findSubsetNSPrefixes(docSubset[0]); - for (const ancestorNs of ancestorNsWithoutDuplicate) { - if (!subsetNsPrefixes.has(ancestorNs.prefix)) { - returningNs.push(ancestorNs); - } - } - - return returningNs; + return findAncestorNsForNode(docSubset[0]); } export function validateDigestValue(digest, expectedDigest) { diff --git a/test/c14n-non-exclusive-unit-tests.spec.ts b/test/c14n-non-exclusive-unit-tests.spec.ts index a5d308b..a086868 100644 --- a/test/c14n-non-exclusive-unit-tests.spec.ts +++ b/test/c14n-non-exclusive-unit-tests.spec.ts @@ -155,6 +155,23 @@ describe("C14N non-exclusive canonicalization tests", function () { test_findAncestorNs(xml, xpath, expected); }); + for (const { label, subsetXpath } of [ + { label: "attributes", subsetXpath: "//@attr" }, + { label: "text nodes", subsetXpath: "//*[local-name()='child']/text()" }, + ]) { + it(`findAncestorNs: Should reject a document subset of ${label}`, function () { + // Only elements carry namespace declarations. Without this check the + // non-element reaches findSubsetNSPrefixes, whose `.attributes` is null + // there, and the caller gets a TypeError instead of a usable message. + const xml = "text"; + const doc = new xmldom.DOMParser().parseFromString(xml); + + expect(() => utils.findAncestorNs(doc, subsetXpath)).to.throw( + "Document subset must be list of elements", + ); + }); + } + // Tests for c14nCanonicalization it("C14n: Correctly picks up root ancestor namespace", function () { const xml = ""; diff --git a/test/signature-object-tests.spec.ts b/test/signature-object-tests.spec.ts index 75126c2..7866c19 100644 --- a/test/signature-object-tests.spec.ts +++ b/test/signature-object-tests.spec.ts @@ -140,7 +140,10 @@ describe("ds:Object support in XML signatures", function () { sig.addReference({ xpath: "/*", digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1", - transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], + transforms: [ + "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + "http://www.w3.org/2001/10/xml-exc-c14n#", + ], }); // When we add a prefix to the signature, there is no default namespace @@ -167,7 +170,10 @@ describe("ds:Object support in XML signatures", function () { sigWithNull.addReference({ xpath: "/*", digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1", - transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], + transforms: [ + "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + "http://www.w3.org/2001/10/xml-exc-c14n#", + ], }); sigWithNull.computeSignature(xml); @@ -190,7 +196,10 @@ describe("ds:Object support in XML signatures", function () { sigWithEmpty.addReference({ xpath: "/*", digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1", - transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], + transforms: [ + "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + "http://www.w3.org/2001/10/xml-exc-c14n#", + ], }); sigWithEmpty.computeSignature(xml); @@ -628,7 +637,10 @@ describe("Signature self-reference prevention", function () { sig.addReference({ xpath: "/*", digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1", - transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], + transforms: [ + "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + "http://www.w3.org/2001/10/xml-exc-c14n#", + ], }); sig.addReference({ diff --git a/test/signature-unit-tests.spec.ts b/test/signature-unit-tests.spec.ts index 2ce3ced..48e6a08 100644 --- a/test/signature-unit-tests.spec.ts +++ b/test/signature-unit-tests.spec.ts @@ -1,6 +1,10 @@ import * as xpath from "xpath"; import * as xmldom from "@xmldom/xmldom"; -import { SignedXml, createOptionalCallbackFunction } from "../src/index"; +import { + SignedXml, + createOptionalCallbackFunction, + type CanonicalizationOrTransformationAlgorithm, +} from "../src/index"; import * as fs from "fs"; import * as crypto from "crypto"; import { expect } from "chai"; @@ -167,7 +171,10 @@ describe("Signature unit tests", function () { sig.addReference({ xpath: "//*[local-name(.)='x']", digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1", - transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], + transforms: [ + "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + "http://www.w3.org/2001/10/xml-exc-c14n#", + ], }); sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#"; sig.signatureAlgorithm = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; @@ -410,6 +417,8 @@ describe("Signature unit tests", function () { class DummyTransformation { includeComments = false; + // Returns a fixed string rather than the node-set it was given. + removesNodes = true; process = function () { return "< x/>"; }; @@ -421,6 +430,8 @@ describe("Signature unit tests", function () { class DummyCanonicalization { includeComments = false; + // Returns a fixed string rather than the node-set it was given. + removesNodes = true; process = function () { return "< x/>"; }; @@ -578,6 +589,8 @@ describe("Signature unit tests", function () { class DummyTransformation { includeComments = false; + // Returns a fixed string rather than the node-set it was given. + removesNodes = true; process = function () { return "< x/>"; }; @@ -589,6 +602,8 @@ describe("Signature unit tests", function () { class DummyCanonicalization { includeComments = false; + // Returns a fixed string rather than the node-set it was given. + removesNodes = true; process = function () { return "< x/>"; }; @@ -923,8 +938,7 @@ describe("Signature unit tests", function () { ref.uri, `wrong uri for index ${i}. expected: ${expectedUri} actual: ${ref.uri}`, ).to.equal(expectedUri); - expect(ref.transforms.length).to.equal(1); - expect(ref.transforms[0]).to.equal("http://www.w3.org/2001/10/xml-exc-c14n#"); + expect(ref.transforms).to.deep.equal(["http://www.w3.org/2001/10/xml-exc-c14n#"]); expect(ref.digestValue).to.equal(digests[i]); expect(ref.digestAlgorithm).to.equal("http://www.w3.org/2000/09/xmldsig#sha1"); } @@ -1089,6 +1103,294 @@ describe("Signature unit tests", function () { expect(URI.value, `uri should be empty but instead was ${URI.value}`).to.equal(""); }); + for (const { label, transforms } of [ + { label: "omitted transforms property", transforms: undefined }, + { label: "empty transforms array", transforms: [] as string[] }, + ]) { + it(`omits Transforms element when no transforms are specified (${label})`, function () { + const xml = ""; + const sig = new SignedXml(); + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.addReference({ + xpath: "//*[local-name(.)='x']", + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + uri: "#ref1", + digestValue: "", + inclusiveNamespacesPrefixList: [], + isEmptyUri: false, + ...(transforms !== undefined ? { transforms } : {}), + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + const doc = new xmldom.DOMParser().parseFromString(signedXml); + const transformNodes = xpath.select("//*[local-name(.)='Transforms']", doc); + expect( + transformNodes, + "Transforms element should be absent when no transforms specified", + ).to.have.length(0); + }); + } + + it("signs and verifies correctly with no transforms (round-trip)", function () { + const xml = ""; + const sig = new SignedXml(); + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.publicCert = fs.readFileSync("./test/static/client_public.pem"); + sig.addReference({ + xpath: "//*[local-name(.)='x']", + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + uri: "#ref1", + digestValue: "", + inclusiveNamespacesPrefixList: [], + isEmptyUri: false, + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + + const doc = new xmldom.DOMParser().parseFromString(signedXml); + const sigNode = xpath.select1( + "//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']", + doc, + ); + isDomNode.assertIsNodeLike(sigNode); + + const verifySig = new SignedXml(); + verifySig.publicCert = fs.readFileSync("./test/static/client_public.pem"); + verifySig.loadSignature(sigNode); + const result = verifySig.checkSignature(signedXml); + expect(result, "expected signature to verify successfully").to.be.true; + }); + + it("derives ancestor namespaces per referenced node when one xpath matches several", function () { + // Transforms are specified explicitly so this exercises only the ancestor + // namespace derivation: findAncestorNs(doc, ref.xpath) used the first XPath + // match (item1's scope) for every reference, so item2 was digested with the + // wrong ancestor namespaces and verification failed. + const xml = + "" + + "
one
" + + "
two
" + + "
"; + const sig = new SignedXml(); + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.publicCert = fs.readFileSync("./test/static/client_public.pem"); + // One addReference() call, so addAllReferences() matches both + // elements from the same ref.xpath. Two separate calls, each with its own + // single-match xpath, would pass even without the fix. + sig.addReference({ + xpath: "//*[local-name(.)='item']", + transforms: ["http://www.w3.org/TR/2001/REC-xml-c14n-20010315"], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + + const doc = new xmldom.DOMParser().parseFromString(signedXml); + const sigNode = xpath.select1( + "//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']", + doc, + ); + isDomNode.assertIsNodeLike(sigNode); + + const verifySig = new SignedXml(); + verifySig.publicCert = fs.readFileSync("./test/static/client_public.pem"); + verifySig.loadSignature(sigNode); + const result = verifySig.checkSignature(signedXml); + expect(result, "expected signature to verify successfully").to.be.true; + }); + + it("refuses to digest a non-element node passed to validateElementAgainstReferences", function () { + // `idAttributes` is public and mutable, so a JavaScript caller can empty it + // and reach the digest path with a node that never sees `getAttribute()`. + // Only elements carry namespace declarations, so this has to be refused + // outright rather than canonicalized into a digest nobody can interpret. + const xml = "hello"; + const sig = new SignedXml(); + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.addReference({ + xpath: "//*[local-name(.)='x']", + transforms: ["http://www.w3.org/TR/2001/REC-xml-c14n-20010315"], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + uri: "#ref1", + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + + const doc = new xmldom.DOMParser().parseFromString(signedXml); + const sigNode = xpath.select1("//*[local-name(.)='Signature']", doc); + isDomNode.assertIsNodeLike(sigNode); + + const verifySig = new SignedXml(); + verifySig.publicCert = fs.readFileSync("./test/static/client_public.pem"); + verifySig.loadSignature(sigNode); + verifySig.idAttributes = []; + + const textNode = xpath.select1("//*[local-name(.)='x']/text()", doc); + expect( + () => verifySig.validateElementAgainstReferences(textNode as unknown as Element, doc), + "a non-element must never be reported as covered by a validated reference", + ).to.throw(); + }); + + for (const { label, transforms } of [ + { label: "no transforms", transforms: undefined }, + { label: "canonicalization alone", transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"] }, + ]) { + it(`refuses to sign a reference that encloses the signature, given ${label}`, function () { + // The signature is appended into , so this reference covers it, and + // nothing in the chain can take it back out. Signing would emit a digest + // over an unfinished Signature that can never be reproduced on verification. + const xml = "hello"; + const sig = new SignedXml(); + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.addReference({ + xpath: "/*", + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + ...(transforms !== undefined ? { transforms } : {}), + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + + expect(() => sig.computeSignature(xml)).to.throw(/enveloped-signature transform/); + }); + } + + for (const { label, declared } of [ + { label: "omits removesNodes", declared: undefined }, + { label: "declares a non-boolean removesNodes", declared: "false" }, + ]) { + it(`refuses to sign with a registered algorithm that ${label}`, function () { + // Registering an algorithm is reachable from JavaScript, where the required + // type is no help. An undeclared value must not quietly skip the check on a + // reference that encloses the signature. + class UndeclaredAlgorithm { + process() { + return ""; + } + getAlgorithmName() { + return "http://Undeclared"; + } + } + if (declared !== undefined) { + Object.assign(UndeclaredAlgorithm.prototype, { removesNodes: declared }); + } + + const sig = new SignedXml(); + sig.CanonicalizationAlgorithms["http://Undeclared"] = + UndeclaredAlgorithm as unknown as new () => CanonicalizationOrTransformationAlgorithm; + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.addReference({ + xpath: "/*", + transforms: ["http://Undeclared"], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + + expect(() => sig.computeSignature("hi")).to.throw( + /must declare a boolean 'removesNodes'/, + ); + }); + } + + it("validates a transform declaration wherever it sits in the chain", function () { + // The enclosing-reference check stops at the first transform that removes + // nodes, so an undeclared algorithm sitting after enveloped-signature must + // still be rejected rather than skipped. + class UndeclaredAlgorithm { + process(node: Node) { + return node; + } + getAlgorithmName() { + return "http://Undeclared"; + } + } + + const sig = new SignedXml(); + sig.CanonicalizationAlgorithms["http://Undeclared"] = + UndeclaredAlgorithm as unknown as new () => CanonicalizationOrTransformationAlgorithm; + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.addReference({ + xpath: "/*", + transforms: ["http://www.w3.org/2000/09/xmldsig#enveloped-signature", "http://Undeclared"], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + + expect(() => sig.computeSignature("hi")).to.throw( + /must declare a boolean 'removesNodes'/, + ); + }); + + it("refuses to verify with a registered algorithm that omits removesNodes", function () { + // The declaration is part of the contract on both sides, so a verifier whose + // own registry is misconfigured must refuse rather than canonicalize with an + // algorithm that never said what it does. + const custom = "http://Custom"; + class DeclaredTransform { + removesNodes = false; + process(node: Node) { + return node.toString(); + } + getAlgorithmName() { + return custom; + } + } + class UndeclaredTransform { + process(node: Node) { + return node.toString(); + } + getAlgorithmName() { + return custom; + } + } + const register = (target: SignedXml, algorithm: unknown) => { + target.CanonicalizationAlgorithms[custom] = + algorithm as new () => CanonicalizationOrTransformationAlgorithm; + }; + + const sig = new SignedXml(); + register(sig, DeclaredTransform); + sig.privateKey = fs.readFileSync("./test/static/client.pem"); + sig.addReference({ + xpath: "//*[local-name(.)='x']", + transforms: [custom], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"; + sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + sig.computeSignature("hi"); + const signedXml = sig.getSignedXml(); + + const doc = new xmldom.DOMParser().parseFromString(signedXml); + const sigNode = xpath.select1("//*[local-name(.)='Signature']", doc); + isDomNode.assertIsNodeLike(sigNode); + + // The document itself is sound: a correctly declared verifier accepts it. + const declaredVerifier = new SignedXml(); + register(declaredVerifier, DeclaredTransform); + declaredVerifier.publicCert = fs.readFileSync("./test/static/client_public.pem"); + declaredVerifier.loadSignature(sigNode); + expect(declaredVerifier.checkSignature(signedXml), "document should be valid").to.be.true; + + const undeclaredVerifier = new SignedXml(); + register(undeclaredVerifier, UndeclaredTransform); + undeclaredVerifier.publicCert = fs.readFileSync("./test/static/client_public.pem"); + undeclaredVerifier.loadSignature(sigNode); + expect(() => undeclaredVerifier.checkSignature(signedXml)).to.throw( + /must declare a boolean 'removesNodes'/, + ); + }); + it("signer appends signature to a non-existing reference node", function () { const xml = "xml-cryptogithub"; const sig = new SignedXml();