diff --git a/README.md b/README.md index 619bb34..e16093d 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,17 @@ 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). +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. + +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/src/signed-xml.ts b/src/signed-xml.ts index 843a53b..256f4dd 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,22 +963,17 @@ 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); - } + return { node }; + }); + referenceTargets.set(ref, targets); } + this.ensureTargetsHaveIds(referenceTargets); // Capture original with IDs (no sig yet) this.originalXmlWithIds = doc.toString(); @@ -1034,30 +1031,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 +1100,46 @@ 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 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, { + inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList, + ancestorNamespaces: ref.ancestorNamespaces, + }); + 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, + referenceTargets: Map, + prefix?: string, + ): void { if (!utils.isArrayHasLength(this.references)) { return; } @@ -1116,20 +1155,24 @@ 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 nodes = xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver); + const signatureContentTargets = new Map(); + for (const [ref, inputTargets] of referenceTargets) { + 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(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 node of nodes) { - 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 || @@ -1194,10 +1237,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 +1249,7 @@ export class SignedXml { signatureNamespace, `${currentPrefix}DigestValue`, ); - digestValueElem.textContent = digestAlgorithm.getHash(canonXml); + digestValueElem.textContent = digestValue ?? this.calculateReferenceDigest(ref, node); referenceElem.appendChild(transformsElem); referenceElem.appendChild(digestMethodElem); diff --git a/src/utils.ts b/src/utils.ts index 76d6864..c21f202 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/signature-integration-tests.spec.ts b/test/signature-integration-tests.spec.ts index 578e2bf..e5e5c5a 100644 --- a/test/signature-integration-tests.spec.ts +++ b/test/signature-integration-tests.spec.ts @@ -379,4 +379,321 @@ 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; + }); + } + }); + } + + 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({ + 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; + } + }); + } + + 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)]", "/*"], + ]) { + 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', + ]); + }); + } + + 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("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, + 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; + }); + + 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]"); + }); + }); }); diff --git a/test/signature-object-tests.spec.ts b/test/signature-object-tests.spec.ts index 75126c2..f4227d5 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#"], });