From c7d031735369081e9397e2a84c0f59175a700075 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Thu, 10 Sep 2026 21:42:22 -0500 Subject: [PATCH 1/7] fix: preserve input references during signature creation Retain input XPath targets and compute their digests before inserting the new signature. This restores broad and ID-dependent selectors and detached signing through getSignatureXml() and getOriginalXmlWithIds(). Resolve references without input matches after insertion so Object and KeyInfo signing remains available, and retain the self-reference guards. Collect ancestor namespaces from each selected element to avoid repeating XPath expressions after ID assignment changes their results. Add regression tests for synchronous and callback signing, tampered data, Object reference ordering, inherited namespaces, and reused signers. Document how input and generated signature references are selected. --- README.md | 6 + src/signed-xml.ts | 106 ++++++++------- src/utils.ts | 40 +++--- test/signing-compatibility-tests.spec.ts | 156 +++++++++++++++++++++++ 4 files changed, 245 insertions(+), 63 deletions(-) create mode 100644 test/signing-compatibility-tests.spec.ts diff --git a/README.md b/README.md index 619bb348..9dc42fe6 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,12 @@ To sign xml documents: - `getSignatureXml()` - returns just the signature part, **must be called only after `computeSignature`** - `getOriginalXmlWithIds()` - **[deprecated]** returns the original xml with Id attributes added on relevant elements, **must be called only after `computeSignature`**. Use the `location` option of `computeSignature()` to place the signature, then `getSignedXml()`. See [how to specify the location of the signature](#how-to-specify-the-location-of-the-signature). +Reference XPath expressions first select elements from the input document. Once a reference +matches, its targets stay the same when IDs and the signature are added. References without +input matches are evaluated after signature insertion, allowing them to select generated +`Object` or `KeyInfo` elements. Use separate `addReference()` calls for input elements and +generated signature content. + To verify xml documents: - `loadSignature(signatureXml)` - loads the signature where: diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 843a53bf..9de5e20c 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -27,6 +27,8 @@ import * as hashAlgorithms from "./hash-algorithms"; import * as signatureAlgorithms from "./signature-algorithms"; import * as utils from "./utils"; +type SigningReferenceTarget = { node: Element; digestValue?: string }; + const warnOriginalXmlWithIds = deprecate( () => {}, "`getOriginalXmlWithIds()` is deprecated and will be removed in a future version. Use the `location` option of `computeSignature()` to place the signature, then `getSignedXml()`.", @@ -961,21 +963,18 @@ export class SignedXml { } } - // Add IDs for all non-self references upfront + const referenceTargets = new Map(); for (const ref of this.getReferences()) { - if (ref.isEmptyUri) { - continue; - } // No specific nodes to ID for empty URI - - const nodes = xpath.selectWithResolver( - ref.xpath ?? "", - doc, - this.namespaceResolver, - ) as Element[]; - for (const node of nodes) { + const nodes = xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver); + isDomNode.assertIsArrayOfNodes(nodes); + const targets = nodes.map((node) => { isDomNode.assertIsElementNode(node); - this.ensureHasId(node); - } + if (!ref.isEmptyUri) { + this.ensureHasId(node); + } + return { node }; + }); + referenceTargets.set(ref, targets); } // Capture original with IDs (no sig yet) @@ -1034,30 +1033,36 @@ export class SignedXml { } } - if (location.action === "append") { - referenceNode.appendChild(signatureElem); - } else if (location.action === "prepend") { - referenceNode.insertBefore(signatureElem, referenceNode.firstChild); - } else if (location.action === "before") { - if (referenceNode.parentNode == null) { - throw new Error( - "`location.reference` refers to the root node (by default), so we can't insert `before`", - ); - } - referenceNode.parentNode.insertBefore(signatureElem, referenceNode); - } else if (location.action === "after") { - if (referenceNode.parentNode == null) { - throw new Error( - "`location.reference` refers to the root node (by default), so we can't insert `after`", - ); - } - referenceNode.parentNode.insertBefore(signatureElem, referenceNode.nextSibling); - } - const previousSignatureNode = this.signatureNode; this.signatureNode = signatureElem; try { - this.addAllReferences(doc, signatureElem, prefix); + for (const [ref, targets] of referenceTargets) { + for (const target of targets) { + target.digestValue = this.calculateReferenceDigest(ref, target.node); + } + } + + if (location.action === "append") { + referenceNode.appendChild(signatureElem); + } else if (location.action === "prepend") { + referenceNode.insertBefore(signatureElem, referenceNode.firstChild); + } else if (location.action === "before") { + if (referenceNode.parentNode == null) { + throw new Error( + "`location.reference` refers to the root node (by default), so we can't insert `before`", + ); + } + referenceNode.parentNode.insertBefore(signatureElem, referenceNode); + } else if (location.action === "after") { + if (referenceNode.parentNode == null) { + throw new Error( + "`location.reference` refers to the root node (by default), so we can't insert `after`", + ); + } + referenceNode.parentNode.insertBefore(signatureElem, referenceNode.nextSibling); + } + + this.addAllReferences(doc, signatureElem, referenceTargets, prefix); } catch (error) { this.signatureNode = previousSignatureNode; throw error; @@ -1097,10 +1102,21 @@ export class SignedXml { } } - /** - * Adds all references to the SignedInfo after the signature placeholder is inserted. - */ - private addAllReferences(doc: Document, signatureElem: Element, prefix?: string): void { + private calculateReferenceDigest(ref: Reference, node: Element): string { + ref.ancestorNamespaces = utils.findAncestorNsForElement(node); + const canonXml = this.getCanonXml(ref.transforms, node, { + inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList, + ancestorNamespaces: ref.ancestorNamespaces, + }); + return this.findHashAlgorithm(ref.digestAlgorithm).getHash(canonXml); + } + + private addAllReferences( + doc: Document, + signatureElem: Element, + referenceTargets: Map, + prefix?: string, + ): void { if (!utils.isArrayHasLength(this.references)) { return; } @@ -1118,7 +1134,10 @@ export class SignedXml { // Process each reference for (const ref of this.getReferences()) { - const nodes = xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver); + const targets = referenceTargets.get(ref); + const nodes = targets?.length + ? targets.map((target) => target.node) + : xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver); if (!utils.isArrayHasLength(nodes)) { throw new Error( @@ -1127,7 +1146,7 @@ export class SignedXml { } // Process the reference - for (const node of nodes) { + for (const [index, node] of nodes.entries()) { isDomNode.assertIsElementNode(node); // Must not be a reference to Signature, SignedInfo, or a child of SignedInfo @@ -1194,10 +1213,6 @@ export class SignedXml { transformsElem.appendChild(transformElem); } - // Get the canonicalized XML - const canonXml = this.getCanonReferenceXml(doc, ref, node); - - // Get the digest algorithm and compute the digest value const digestAlgorithm = this.findHashAlgorithm(ref.digestAlgorithm); const digestMethodElem = signatureDoc.createElementNS( @@ -1210,7 +1225,8 @@ export class SignedXml { signatureNamespace, `${currentPrefix}DigestValue`, ); - digestValueElem.textContent = digestAlgorithm.getHash(canonXml); + digestValueElem.textContent = + targets?.[index]?.digestValue ?? this.calculateReferenceDigest(ref, node); referenceElem.appendChild(transformsElem); referenceElem.appendChild(digestMethodElem); diff --git a/src/utils.ts b/src/utils.ts index 76d6864d..c21f2024 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -243,6 +243,27 @@ function isElementSubset(docSubset: Node[]): docSubset is Element[] { return docSubset.every((node) => isDomNode.isElementNode(node)); } +export function findAncestorNsForElement(node: Element): NamespacePrefix[] { + const ancestorNs = collectAncestorNamespaces(node); + 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(node); + for (const ancestorNs of ancestorNsWithoutDuplicate) { + if (!subsetNsPrefixes.has(ancestorNs.prefix)) { + returningNs.push(ancestorNs); + } + } + + return returningNs; +} + /** * Extract ancestor namespaces in order to import it to root of document subset * which is being canonicalized for non-exclusive c14n. @@ -271,24 +292,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 findAncestorNsForElement(docSubset[0]); } export function validateDigestValue(digest, expectedDigest) { diff --git a/test/signing-compatibility-tests.spec.ts b/test/signing-compatibility-tests.spec.ts new file mode 100644 index 00000000..992f371c --- /dev/null +++ b/test/signing-compatibility-tests.spec.ts @@ -0,0 +1,156 @@ +import { expect } from "chai"; +import * as fs from "fs"; +import * as xmldom from "@xmldom/xmldom"; +import { SignedXml } from "../src/index"; + +describe("Signing compatibility", function () { + const privateKey = fs.readFileSync("./test/static/client.pem"); + const publicCert = fs.readFileSync("./test/static/client_public.pem"); + const canonicalization = "http://www.w3.org/2001/10/xml-exc-c14n#"; + const enveloped = "http://www.w3.org/2000/09/xmldsig#enveloped-signature"; + + for (const useCallback of [false, true]) { + describe(useCallback ? "with a callback" : "without a callback", function () { + for (const { name, reference, detached } of [ + { name: "signs all input elements", reference: "//*", detached: false }, + { + name: "signs an element selected by the absence of an ID", + reference: "/root[not(@Id)]", + detached: false, + }, + { name: "creates a verifiable detached signature", reference: "/*", detached: true }, + ]) { + it(name, async function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + signer.addReference({ + xpath: reference, + transforms: detached ? [canonicalization] : [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + + const xml = "trusted"; + if (useCallback) { + await new Promise((resolve, reject) => { + signer.computeSignature(xml, (err) => (err ? reject(err) : resolve())); + }); + } else { + signer.computeSignature(xml); + } + + const signedXml = detached + ? // eslint-disable-next-line deprecation/deprecation + signer.getOriginalXmlWithIds() + : signer.getSignedXml(); + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + + expect(verifier.checkSignature(signedXml)).to.be.true; + expect(verifier.getSignedReferences()).to.deep.equal(['trusted']); + expect(verifier.checkSignature(signedXml.replace("trusted", "tampered"))).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + }); + } + }); + } + + for (const objectFirst of [false, true]) { + it(`preserves input targets alongside an Object reference placed ${objectFirst ? "first" : "last"}`, function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + objects: [{ content: "object-data" }], + }); + const inputReference = { + xpath: "//*", + transforms: [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }; + const objectReference = { + xpath: "//*[local-name(.)='Object']", + transforms: [canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }; + for (const reference of objectFirst + ? [objectReference, inputReference] + : [inputReference, objectReference]) { + signer.addReference(reference); + } + + signer.computeSignature("trusted"); + + const signedXml = signer.getSignedXml(); + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signedXml)).to.be.true; + const signedElements = verifier + .getSignedReferences() + .map((xml) => new xmldom.DOMParser().parseFromString(xml).documentElement.localName); + expect(signedElements).to.deep.equal( + objectFirst ? ["Object", "root", "value"] : ["root", "value", "Object"], + ); + for (const content of ["trusted", "object-data"]) { + expect(verifier.checkSignature(signedXml.replace(content, "tampered"))).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + } + }); + } + + it("retains ancestor namespaces when adding an ID changes the reference XPath's result", function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + signer.addReference({ + xpath: "/root/item[not(@Id)]", + transforms: ["http://www.w3.org/TR/2001/REC-xml-c14n-20010315"], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + + signer.computeSignature('trusted'); + + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; + expect(verifier.getSignedReferences()).to.deep.equal([ + 'trusted', + ]); + }); + + it("preserves an existing signature when reusing the signer to sign its output", function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + signer.addReference({ + xpath: "/*", + transforms: [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + signer.computeSignature("trusted"); + const originalSignature = signer.getSignatureXml(); + + signer.computeSignature(signer.getSignedXml()); + + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; + expect( + verifier.checkSignature( + signer + .getSignedXml() + .replace( + originalSignature, + originalSignature.replace("SignatureValue>", "SignatureValue>tampered"), + ), + ), + ).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + }); +}); From 6f38d9c56322017ed1aed6f579a87566a2f3f1d6 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Thu, 10 Sep 2026 22:24:29 -0500 Subject: [PATCH 2/7] test: move signing compatibility tests into the integration spec The reference-selection and detached-signature cases exercise signing and verification end to end, so they belong with the other integration tests rather than in a spec of their own. Co-Authored-By: Claude Opus 5 --- test/signature-integration-tests.spec.ts | 152 ++++++++++++++++++++++ test/signing-compatibility-tests.spec.ts | 156 ----------------------- 2 files changed, 152 insertions(+), 156 deletions(-) delete mode 100644 test/signing-compatibility-tests.spec.ts diff --git a/test/signature-integration-tests.spec.ts b/test/signature-integration-tests.spec.ts index 578e2bf5..05633d37 100644 --- a/test/signature-integration-tests.spec.ts +++ b/test/signature-integration-tests.spec.ts @@ -379,4 +379,156 @@ describe("Signature integration tests", function () { isDomNode.assertIsElementNode(y); expect(y.namespaceURI ?? "", " must stay in no namespace").to.equal(""); }); + + describe("reference selection and detached signatures", function () { + const privateKey = fs.readFileSync("./test/static/client.pem"); + const publicCert = fs.readFileSync("./test/static/client_public.pem"); + const canonicalization = "http://www.w3.org/2001/10/xml-exc-c14n#"; + const enveloped = "http://www.w3.org/2000/09/xmldsig#enveloped-signature"; + + for (const useCallback of [false, true]) { + describe(useCallback ? "with a callback" : "without a callback", function () { + for (const { name, reference, detached } of [ + { name: "signs all input elements", reference: "//*", detached: false }, + { + name: "signs an element selected by the absence of an ID", + reference: "/root[not(@Id)]", + detached: false, + }, + { name: "creates a verifiable detached signature", reference: "/*", detached: true }, + ]) { + it(name, async function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + signer.addReference({ + xpath: reference, + transforms: detached ? [canonicalization] : [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + + const xml = "trusted"; + if (useCallback) { + await new Promise((resolve, reject) => { + signer.computeSignature(xml, (err) => (err ? reject(err) : resolve())); + }); + } else { + signer.computeSignature(xml); + } + + const signedXml = detached + ? // eslint-disable-next-line deprecation/deprecation + signer.getOriginalXmlWithIds() + : signer.getSignedXml(); + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + + expect(verifier.checkSignature(signedXml)).to.be.true; + expect(verifier.getSignedReferences()).to.deep.equal(['trusted']); + expect(verifier.checkSignature(signedXml.replace("trusted", "tampered"))).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + }); + } + }); + } + + for (const objectFirst of [false, true]) { + it(`preserves input targets alongside an Object reference placed ${objectFirst ? "first" : "last"}`, function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + objects: [{ content: "object-data" }], + }); + const inputReference = { + xpath: "//*", + transforms: [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }; + const objectReference = { + xpath: "//*[local-name(.)='Object']", + transforms: [canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }; + for (const reference of objectFirst + ? [objectReference, inputReference] + : [inputReference, objectReference]) { + signer.addReference(reference); + } + + signer.computeSignature("trusted"); + + const signedXml = signer.getSignedXml(); + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signedXml)).to.be.true; + const signedElements = verifier + .getSignedReferences() + .map((xml) => new xmldom.DOMParser().parseFromString(xml).documentElement.localName); + expect(signedElements).to.deep.equal( + objectFirst ? ["Object", "root", "value"] : ["root", "value", "Object"], + ); + for (const content of ["trusted", "object-data"]) { + expect(verifier.checkSignature(signedXml.replace(content, "tampered"))).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + } + }); + } + + it("retains ancestor namespaces when adding an ID changes the reference XPath's result", function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + signer.addReference({ + xpath: "/root/item[not(@Id)]", + transforms: ["http://www.w3.org/TR/2001/REC-xml-c14n-20010315"], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + + signer.computeSignature('trusted'); + + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; + expect(verifier.getSignedReferences()).to.deep.equal([ + 'trusted', + ]); + }); + + it("preserves an existing signature when reusing the signer to sign its output", function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + signer.addReference({ + xpath: "/*", + transforms: [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + signer.computeSignature("trusted"); + const originalSignature = signer.getSignatureXml(); + + signer.computeSignature(signer.getSignedXml()); + + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; + expect( + verifier.checkSignature( + signer + .getSignedXml() + .replace( + originalSignature, + originalSignature.replace("SignatureValue>", "SignatureValue>tampered"), + ), + ), + ).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + }); + }); }); diff --git a/test/signing-compatibility-tests.spec.ts b/test/signing-compatibility-tests.spec.ts deleted file mode 100644 index 992f371c..00000000 --- a/test/signing-compatibility-tests.spec.ts +++ /dev/null @@ -1,156 +0,0 @@ -import { expect } from "chai"; -import * as fs from "fs"; -import * as xmldom from "@xmldom/xmldom"; -import { SignedXml } from "../src/index"; - -describe("Signing compatibility", function () { - const privateKey = fs.readFileSync("./test/static/client.pem"); - const publicCert = fs.readFileSync("./test/static/client_public.pem"); - const canonicalization = "http://www.w3.org/2001/10/xml-exc-c14n#"; - const enveloped = "http://www.w3.org/2000/09/xmldsig#enveloped-signature"; - - for (const useCallback of [false, true]) { - describe(useCallback ? "with a callback" : "without a callback", function () { - for (const { name, reference, detached } of [ - { name: "signs all input elements", reference: "//*", detached: false }, - { - name: "signs an element selected by the absence of an ID", - reference: "/root[not(@Id)]", - detached: false, - }, - { name: "creates a verifiable detached signature", reference: "/*", detached: true }, - ]) { - it(name, async function () { - const signer = new SignedXml({ - privateKey, - canonicalizationAlgorithm: canonicalization, - signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", - }); - signer.addReference({ - xpath: reference, - transforms: detached ? [canonicalization] : [enveloped, canonicalization], - digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", - }); - - const xml = "trusted"; - if (useCallback) { - await new Promise((resolve, reject) => { - signer.computeSignature(xml, (err) => (err ? reject(err) : resolve())); - }); - } else { - signer.computeSignature(xml); - } - - const signedXml = detached - ? // eslint-disable-next-line deprecation/deprecation - signer.getOriginalXmlWithIds() - : signer.getSignedXml(); - const verifier = new SignedXml({ publicCert }); - verifier.loadSignature(signer.getSignatureXml()); - - expect(verifier.checkSignature(signedXml)).to.be.true; - expect(verifier.getSignedReferences()).to.deep.equal(['trusted']); - expect(verifier.checkSignature(signedXml.replace("trusted", "tampered"))).to.be.false; - expect(verifier.getSignedReferences()).to.be.empty; - }); - } - }); - } - - for (const objectFirst of [false, true]) { - it(`preserves input targets alongside an Object reference placed ${objectFirst ? "first" : "last"}`, function () { - const signer = new SignedXml({ - privateKey, - canonicalizationAlgorithm: canonicalization, - signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", - objects: [{ content: "object-data" }], - }); - const inputReference = { - xpath: "//*", - transforms: [enveloped, canonicalization], - digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", - }; - const objectReference = { - xpath: "//*[local-name(.)='Object']", - transforms: [canonicalization], - digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", - }; - for (const reference of objectFirst - ? [objectReference, inputReference] - : [inputReference, objectReference]) { - signer.addReference(reference); - } - - signer.computeSignature("trusted"); - - const signedXml = signer.getSignedXml(); - const verifier = new SignedXml({ publicCert }); - verifier.loadSignature(signer.getSignatureXml()); - expect(verifier.checkSignature(signedXml)).to.be.true; - const signedElements = verifier - .getSignedReferences() - .map((xml) => new xmldom.DOMParser().parseFromString(xml).documentElement.localName); - expect(signedElements).to.deep.equal( - objectFirst ? ["Object", "root", "value"] : ["root", "value", "Object"], - ); - for (const content of ["trusted", "object-data"]) { - expect(verifier.checkSignature(signedXml.replace(content, "tampered"))).to.be.false; - expect(verifier.getSignedReferences()).to.be.empty; - } - }); - } - - it("retains ancestor namespaces when adding an ID changes the reference XPath's result", function () { - const signer = new SignedXml({ - privateKey, - canonicalizationAlgorithm: canonicalization, - signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", - }); - signer.addReference({ - xpath: "/root/item[not(@Id)]", - transforms: ["http://www.w3.org/TR/2001/REC-xml-c14n-20010315"], - digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", - }); - - signer.computeSignature('trusted'); - - const verifier = new SignedXml({ publicCert }); - verifier.loadSignature(signer.getSignatureXml()); - expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; - expect(verifier.getSignedReferences()).to.deep.equal([ - 'trusted', - ]); - }); - - it("preserves an existing signature when reusing the signer to sign its output", function () { - const signer = new SignedXml({ - privateKey, - canonicalizationAlgorithm: canonicalization, - signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", - }); - signer.addReference({ - xpath: "/*", - transforms: [enveloped, canonicalization], - digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", - }); - signer.computeSignature("trusted"); - const originalSignature = signer.getSignatureXml(); - - signer.computeSignature(signer.getSignedXml()); - - const verifier = new SignedXml({ publicCert }); - verifier.loadSignature(signer.getSignatureXml()); - expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; - expect( - verifier.checkSignature( - signer - .getSignedXml() - .replace( - originalSignature, - originalSignature.replace("SignatureValue>", "SignatureValue>tampered"), - ), - ), - ).to.be.false; - expect(verifier.getSignedReferences()).to.be.empty; - }); -}); From ae7233a4e81e50b39b216d3b8b21e532c18def89 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Fri, 11 Sep 2026 15:08:04 -0500 Subject: [PATCH 3/7] fix: resolve every reference before adding IDs Each reference's XPath was evaluated and its targets given IDs before the next reference was evaluated, so a reference could change what a later one selected. With `/*` followed by `/root[not(@Id)]`, the first reference added an ID to the root and the second then matched nothing; in the reverse order both matched. Evaluate every reference against the unmodified input first, then add IDs to all of the targets. Co-Authored-By: Claude Opus 5 --- src/signed-xml.ts | 14 ++++++++--- test/signature-integration-tests.spec.ts | 30 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 9de5e20c..10fd8e76 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -969,13 +969,11 @@ export class SignedXml { isDomNode.assertIsArrayOfNodes(nodes); const targets = nodes.map((node) => { isDomNode.assertIsElementNode(node); - if (!ref.isEmptyUri) { - this.ensureHasId(node); - } return { node }; }); referenceTargets.set(ref, targets); } + this.ensureTargetsHaveIds(referenceTargets); // Capture original with IDs (no sig yet) this.originalXmlWithIds = doc.toString(); @@ -1102,6 +1100,16 @@ export class SignedXml { } } + private ensureTargetsHaveIds(referenceTargets: Map): void { + for (const [ref, targets] of referenceTargets) { + if (!ref.isEmptyUri) { + for (const { node } of targets) { + this.ensureHasId(node); + } + } + } + } + private calculateReferenceDigest(ref: Reference, node: Element): string { ref.ancestorNamespaces = utils.findAncestorNsForElement(node); const canonXml = this.getCanonXml(ref.transforms, node, { diff --git a/test/signature-integration-tests.spec.ts b/test/signature-integration-tests.spec.ts index 05633d37..7ef7fed8 100644 --- a/test/signature-integration-tests.spec.ts +++ b/test/signature-integration-tests.spec.ts @@ -477,6 +477,36 @@ describe("Signature integration tests", function () { }); } + for (const references of [ + ["/*", "/root[not(@Id)]"], + ["/root[not(@Id)]", "/*"], + ]) { + it(`resolves every reference against the input document with ${references[0]} first`, function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + for (const xpath of references) { + signer.addReference({ + xpath, + transforms: [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + } + + signer.computeSignature("trusted"); + + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; + expect(verifier.getSignedReferences()).to.deep.equal([ + 'trusted', + 'trusted', + ]); + }); + } + it("retains ancestor namespaces when adding an ID changes the reference XPath's result", function () { const signer = new SignedXml({ privateKey, From de3c63ff7144bff661783345d50e7c7d2ac2c27a Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Thu, 10 Sep 2026 22:29:08 -0500 Subject: [PATCH 4/7] fix: limit unmatched references to the new signature's content A reference that matched nothing in the input was re-evaluated against the whole document once the signature had been inserted. The insertion shifts positions, so an XPath such as `/root/*[2]` with a `prepend` location could then select an input element. The signer added that element's ID after the enclosing references had been digested and emitted a signature that failed verification, where 6.x rejected the reference as not found. Adding IDs has the same effect: `//*[@Id]` on `` signed the root once another reference had given it an ID. Keep only matches inside the new Signature for such references, so they can still select generated Object and KeyInfo content, and throw the 6.x error otherwise. Co-Authored-By: Claude Opus 5 --- README.md | 8 ++-- src/signed-xml.ts | 36 +++++++++++------- test/signature-integration-tests.spec.ts | 48 ++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 9dc42fe6..287c9b2d 100644 --- a/README.md +++ b/README.md @@ -304,10 +304,10 @@ To sign xml documents: - `getOriginalXmlWithIds()` - **[deprecated]** returns the original xml with Id attributes added on relevant elements, **must be called only after `computeSignature`**. Use the `location` option of `computeSignature()` to place the signature, then `getSignedXml()`. See [how to specify the location of the signature](#how-to-specify-the-location-of-the-signature). Reference XPath expressions first select elements from the input document. Once a reference -matches, its targets stay the same when IDs and the signature are added. References without -input matches are evaluated after signature insertion, allowing them to select generated -`Object` or `KeyInfo` elements. Use separate `addReference()` calls for input elements and -generated signature content. +matches, its targets stay the same when IDs and the signature are added. A reference without +input matches is evaluated after signature insertion and selects only elements inside the new +signature, such as generated `Object` or `KeyInfo` elements. Use separate `addReference()` calls +for input elements and generated signature content. To verify xml documents: diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 10fd8e76..c65ce9d9 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -1119,6 +1119,21 @@ export class SignedXml { return this.findHashAlgorithm(ref.digestAlgorithm).getHash(canonXml); } + private findSignatureContentTargets( + ref: Reference, + doc: Document, + signatureElem: Element, + ): SigningReferenceTarget[] { + const nodes = xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver); + isDomNode.assertIsArrayOfNodes(nodes); + return nodes + .filter((node) => node === signatureElem || utils.isDescendantOf(node, signatureElem)) + .map((node) => { + isDomNode.assertIsElementNode(node); + return { node }; + }); + } + private addAllReferences( doc: Document, signatureElem: Element, @@ -1140,23 +1155,19 @@ export class SignedXml { // but we will extract it here for clarity (and also make it support detached signatures in the future) const signatureDoc = signatureElem.ownerDocument; - // Process each reference - for (const ref of this.getReferences()) { - const targets = referenceTargets.get(ref); - const nodes = targets?.length - ? targets.map((target) => target.node) - : xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver); + for (const [ref, inputTargets] of referenceTargets) { + const targets = + inputTargets.length > 0 + ? inputTargets + : this.findSignatureContentTargets(ref, doc, signatureElem); - if (!utils.isArrayHasLength(nodes)) { + if (!utils.isArrayHasLength(targets)) { throw new Error( `the following xpath cannot be signed because it was not found: ${ref.xpath}`, ); } - // Process the reference - for (const [index, node] of nodes.entries()) { - isDomNode.assertIsElementNode(node); - + for (const { node, digestValue } of targets) { // Must not be a reference to Signature, SignedInfo, or a child of SignedInfo if ( node === signatureElem || @@ -1233,8 +1244,7 @@ export class SignedXml { signatureNamespace, `${currentPrefix}DigestValue`, ); - digestValueElem.textContent = - targets?.[index]?.digestValue ?? this.calculateReferenceDigest(ref, node); + digestValueElem.textContent = digestValue ?? this.calculateReferenceDigest(ref, node); referenceElem.appendChild(transformsElem); referenceElem.appendChild(digestMethodElem); diff --git a/test/signature-integration-tests.spec.ts b/test/signature-integration-tests.spec.ts index 7ef7fed8..c7410025 100644 --- a/test/signature-integration-tests.spec.ts +++ b/test/signature-integration-tests.spec.ts @@ -507,6 +507,30 @@ describe("Signature integration tests", function () { }); } + for (const references of [ + ["//*[@Id]", "/*"], + ["/*", "//*[@Id]"], + ]) { + it(`rejects a reference that matches an input element only after IDs are added, with ${references[0]} first`, function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + for (const xpath of references) { + signer.addReference({ + xpath, + transforms: [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + } + + expect(() => signer.computeSignature("trusted")).to.throw( + "the following xpath cannot be signed because it was not found: //*[@Id]", + ); + }); + } + it("retains ancestor namespaces when adding an ID changes the reference XPath's result", function () { const signer = new SignedXml({ privateKey, @@ -560,5 +584,29 @@ describe("Signature integration tests", function () { ).to.be.false; expect(verifier.getSignedReferences()).to.be.empty; }); + + it("rejects a reference that matches an input element only after the signature is inserted", function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + signer.addReference({ + xpath: "/root", + transforms: [enveloped, canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + signer.addReference({ + xpath: "/root/*[2]", + transforms: [canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + + expect(() => + signer.computeSignature("trusted", { + location: { reference: "/root", action: "prepend" }, + }), + ).to.throw("the following xpath cannot be signed because it was not found: /root/*[2]"); + }); }); }); From 37f7e5c0ded3198c6478e566daf2555eb9946ebb Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Fri, 11 Sep 2026 15:14:12 -0500 Subject: [PATCH 5/7] fix: resolve generated-content references before adding their IDs A reference with no input matches was evaluated, given an ID and digested before the next such reference was evaluated. With a reference to an Object followed by one to an element inside it, the second reference's ID changed the Object after its digest had been taken, and the signature failed verification; in the reverse order it verified. Evaluate all of these references against the new signature before adding IDs to any of their targets, as is already done for input references. They can therefore no longer select Reference elements written for earlier references. An XPath such as `.//*[local-name(.)='Reference']/*` is now rejected as not found, as in 6.1.2, instead of by the SignedInfo guard, so the guard's test now references SignatureMethod. Co-Authored-By: Claude Opus 5 --- README.md | 10 +++---- src/signed-xml.ts | 13 ++++++--- test/signature-integration-tests.spec.ts | 37 ++++++++++++++++++++++++ test/signature-object-tests.spec.ts | 8 +---- 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 287c9b2d..92ade028 100644 --- a/README.md +++ b/README.md @@ -303,11 +303,11 @@ To sign xml documents: - `getSignatureXml()` - returns just the signature part, **must be called only after `computeSignature`** - `getOriginalXmlWithIds()` - **[deprecated]** returns the original xml with Id attributes added on relevant elements, **must be called only after `computeSignature`**. Use the `location` option of `computeSignature()` to place the signature, then `getSignedXml()`. See [how to specify the location of the signature](#how-to-specify-the-location-of-the-signature). -Reference XPath expressions first select elements from the input document. Once a reference -matches, its targets stay the same when IDs and the signature are added. A reference without -input matches is evaluated after signature insertion and selects only elements inside the new -signature, such as generated `Object` or `KeyInfo` elements. Use separate `addReference()` calls -for input elements and generated signature content. +Every reference XPath is evaluated against the input document before any IDs or the signature +are added, so the order of `addReference()` calls does not change what a reference selects. A +reference that matches nothing in the input is evaluated after the signature is inserted and +selects only elements inside the new signature, such as generated `Object` or `KeyInfo` +elements. Use separate `addReference()` calls for input elements and generated signature content. To verify xml documents: diff --git a/src/signed-xml.ts b/src/signed-xml.ts index c65ce9d9..256f4dd3 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -1155,11 +1155,16 @@ export class SignedXml { // but we will extract it here for clarity (and also make it support detached signatures in the future) const signatureDoc = signatureElem.ownerDocument; + const signatureContentTargets = new Map(); for (const [ref, inputTargets] of referenceTargets) { - const targets = - inputTargets.length > 0 - ? inputTargets - : this.findSignatureContentTargets(ref, doc, signatureElem); + if (inputTargets.length === 0) { + signatureContentTargets.set(ref, this.findSignatureContentTargets(ref, doc, signatureElem)); + } + } + this.ensureTargetsHaveIds(signatureContentTargets); + + for (const [ref, inputTargets] of referenceTargets) { + const targets = signatureContentTargets.get(ref) ?? inputTargets; if (!utils.isArrayHasLength(targets)) { throw new Error( diff --git a/test/signature-integration-tests.spec.ts b/test/signature-integration-tests.spec.ts index c7410025..defa4b76 100644 --- a/test/signature-integration-tests.spec.ts +++ b/test/signature-integration-tests.spec.ts @@ -477,6 +477,43 @@ describe("Signature integration tests", function () { }); } + for (const objectFirst of [false, true]) { + it(`signs an Object and an element inside it with the ${objectFirst ? "Object" : "element"} referenced first`, function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + objects: [{ content: "object-data" }], + }); + const objectReference = "//*[local-name(.)='Object']"; + const valueReference = "//*[local-name(.)='value']"; + for (const xpath of objectFirst + ? [objectReference, valueReference] + : [valueReference, objectReference]) { + signer.addReference({ + xpath, + transforms: [canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + } + + signer.computeSignature("trusted"); + + const signedXml = signer.getSignedXml(); + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signedXml)).to.be.true; + const signedElements = verifier + .getSignedReferences() + .map((xml) => new xmldom.DOMParser().parseFromString(xml).documentElement.localName); + expect(signedElements).to.deep.equal( + objectFirst ? ["Object", "value"] : ["value", "Object"], + ); + expect(verifier.checkSignature(signedXml.replace("object-data", "tampered"))).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + }); + } + for (const references of [ ["/*", "/root[not(@Id)]"], ["/root[not(@Id)]", "/*"], diff --git a/test/signature-object-tests.spec.ts b/test/signature-object-tests.spec.ts index 75126c23..f4227d5b 100644 --- a/test/signature-object-tests.spec.ts +++ b/test/signature-object-tests.spec.ts @@ -626,13 +626,7 @@ 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#"], - }); - - sig.addReference({ - xpath: ".//*[local-name(.)='Reference']/*", + xpath: ".//*[local-name(.)='SignatureMethod']", digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1", transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], }); From 4d6e72d4b0e385176f5ce6820020c233243da75e Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Fri, 11 Sep 2026 21:04:00 -0500 Subject: [PATCH 6/7] test: cover an element excluded by an ID added for another reference Resolving each reference after the previous ones had added their IDs could drop an element the reference's XPath selects in the input: `/root/b` followed by `/root/c | /root/a[not(../b/@Id)]` signed only `b` and `c`, and tampering with `a` went undetected. Co-Authored-By: Claude Opus 5 --- test/signature-integration-tests.spec.ts | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/signature-integration-tests.spec.ts b/test/signature-integration-tests.spec.ts index defa4b76..ed53505f 100644 --- a/test/signature-integration-tests.spec.ts +++ b/test/signature-integration-tests.spec.ts @@ -568,6 +568,35 @@ describe("Signature integration tests", function () { }); } + it("signs an element its reference selects in the input that an ID added for another reference excludes", function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + }); + for (const xpath of ["/root/b", "/root/c | /root/a[not(../b/@Id)]"]) { + signer.addReference({ + xpath, + transforms: [canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + } + + signer.computeSignature("ABC"); + + const signedXml = signer.getSignedXml(); + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signedXml)).to.be.true; + expect(verifier.getSignedReferences()).to.deep.equal([ + 'B', + 'A', + 'C', + ]); + expect(verifier.checkSignature(signedXml.replace(">A<", ">tampered<"))).to.be.false; + expect(verifier.getSignedReferences()).to.be.empty; + }); + it("retains ancestor namespaces when adding an ID changes the reference XPath's result", function () { const signer = new SignedXml({ privateKey, From 21da53f7c42964f1c175b45ba4abf3116b4b9f2d Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Fri, 11 Sep 2026 22:16:07 -0500 Subject: [PATCH 7/7] docs: warn that an input match shadows generated signature content A reference whose xpath matches an input element never falls back to the new signature, so `//*[local-name(.)='Object']` signs an input `Object` and leaves a configured one unsigned. Recommend selecting generated content by its configured Id, and pin the precedence in a test. Co-Authored-By: Claude Opus 5 --- README.md | 5 +++++ test/signature-integration-tests.spec.ts | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/README.md b/README.md index 92ade028..e16093da 100644 --- a/README.md +++ b/README.md @@ -309,6 +309,11 @@ reference that matches nothing in the input is evaluated after the signature is selects only elements inside the new signature, such as generated `Object` or `KeyInfo` elements. Use separate `addReference()` calls for input elements and generated signature content. +An input match takes precedence, so a reference whose XPath also matches an input element signs +that element and leaves the generated one unsigned. Select generated content by the `Id` you +configured for it, as in [how to add custom Objects to the signature](#how-to-add-custom-objects-to-the-signature), +rather than by element name. + To verify xml documents: - `loadSignature(signatureXml)` - loads the signature where: diff --git a/test/signature-integration-tests.spec.ts b/test/signature-integration-tests.spec.ts index ed53505f..e5e5c5a8 100644 --- a/test/signature-integration-tests.spec.ts +++ b/test/signature-integration-tests.spec.ts @@ -434,6 +434,27 @@ describe("Signature integration tests", function () { }); } + it("prefers an input match over generated signature content for the same xpath", function () { + const signer = new SignedXml({ + privateKey, + canonicalizationAlgorithm: canonicalization, + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + objects: [{ content: "generated", attributes: { Id: "generated" } }], + }); + signer.addReference({ + xpath: "//*[local-name(.)='Object']", + transforms: [canonicalization], + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + }); + + signer.computeSignature("input data"); + + const verifier = new SignedXml({ publicCert }); + verifier.loadSignature(signer.getSignatureXml()); + expect(verifier.checkSignature(signer.getSignedXml())).to.be.true; + expect(verifier.getSignedReferences()).to.deep.equal(['input data']); + }); + for (const objectFirst of [false, true]) { it(`preserves input targets alongside an Object reference placed ${objectFirst ? "first" : "last"}`, function () { const signer = new SignedXml({