From 7633e4d813af51d8569d90c23b7d10700205c4f5 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Mon, 14 Sep 2026 19:35:13 -0500 Subject: [PATCH] fix: omit KeyInfo when there is no content for it XMLDSig requires KeyInfo and X509Data to each have at least one child element. The default getKeyInfoContent returned an empty X509Data when publicCert held no certificate, and setting keyInfoAttributes without any content wrote the text "null" into KeyInfo. Closes #596 Co-Authored-By: Claude Opus 5 --- README.md | 4 ++-- src/signed-xml.ts | 37 +++++++++++++++++-------------- test/signature-unit-tests.spec.ts | 32 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 2d33b85c..c7e6f77c 100644 --- a/README.md +++ b/README.md @@ -384,8 +384,8 @@ Now define the extension point you want to implement. You can choose one or more To determine the inclusion and contents of a `` element, the function `this.getKeyInfoContent()` is called. There is a default implementation of this. If you wish to change this implementation, provide your own function assigned to the property `this.getKeyInfoContent`. If -there are no attributes and no contents to the `` element, it won't be included in the -generated XML. +it returns no content, the `` element is not included in the generated XML, even when +`keyInfoAttributes` are set. To specify custom attributes on ``, add the properties to the `.keyInfoAttributes` property. diff --git a/src/signed-xml.ts b/src/signed-xml.ts index e1116e85..97bbfd4a 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -213,7 +213,7 @@ export class SignedXml { * Builds the contents of a KeyInfo element as an XML string. * * For example, if the value of the prefix argument is 'foo', then - * the resultant XML string will be "" + * the resultant XML string will be "..." * * @return an XML string representation of the contents of a KeyInfo element, or `null` if no `KeyInfo` element should be included */ @@ -224,7 +224,6 @@ export class SignedXml { prefix = prefix ? `${prefix}:` : ""; - let x509Certs = ""; if (Buffer.isBuffer(publicCert)) { publicCert = publicCert.toString("latin1"); } @@ -234,17 +233,20 @@ export class SignedXml { publicCertMatches = publicCert.match(utils.EXTRACT_X509_CERTS) || []; } - if (publicCertMatches.length > 0) { - x509Certs = publicCertMatches - .map( - (c) => - `<${prefix}X509Certificate>${utils - .pemToDer(c) - .toString("base64")}`, - ) - .join(""); + // X509Data requires at least one child: https://www.w3.org/TR/xmldsig-core1/#sec-X509Data + if (publicCertMatches.length === 0) { + return null; } + const x509Certs = publicCertMatches + .map( + (c) => + `<${prefix}X509Certificate>${utils + .pemToDer(c) + .toString("base64")}`, + ) + .join(""); + return `<${prefix}X509Data>${x509Certs}`; } @@ -1286,6 +1288,12 @@ export class SignedXml { } private getKeyInfo(prefix) { + const keyInfoContent = this.getKeyInfoContent({ publicCert: this.publicCert, prefix }); + // KeyInfo requires at least one child: https://www.w3.org/TR/xmldsig-core1/#sec-KeyInfo + if (!keyInfoContent) { + return ""; + } + const currentPrefix = prefix ? `${prefix}:` : ""; let keyInfoAttrs = ""; @@ -1295,12 +1303,7 @@ export class SignedXml { }); } - const keyInfoContent = this.getKeyInfoContent({ publicCert: this.publicCert, prefix }); - if (keyInfoAttrs || keyInfoContent) { - return `<${currentPrefix}KeyInfo${keyInfoAttrs}>${keyInfoContent}`; - } - - return ""; + return `<${currentPrefix}KeyInfo${keyInfoAttrs}>${keyInfoContent}`; } /** diff --git a/test/signature-unit-tests.spec.ts b/test/signature-unit-tests.spec.ts index 3f9439c2..269f97e5 100644 --- a/test/signature-unit-tests.spec.ts +++ b/test/signature-unit-tests.spec.ts @@ -1372,6 +1372,38 @@ describe("Signature unit tests", function () { ); }); + describe("omits KeyInfo when there is no content for it", function () { + const privateKey = fs.readFileSync("./test/static/client.pem"); + + function selectKeyInfo(options: ConstructorParameters[0]) { + const sig = new SignedXml({ + privateKey, + canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#", + signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + ...options, + }); + sig.addReference({ + xpath: "//*[local-name(.)='x']", + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], + }); + sig.computeSignature(""); + const doc = new xmldom.DOMParser().parseFromString(sig.getSignedXml()); + + return xpath.select("//*[local-name(.)='KeyInfo']", doc); + } + + it("when publicCert contains no certificate", function () { + const publicCert = crypto.createPublicKey(privateKey).export({ type: "spki", format: "pem" }); + + expect(selectKeyInfo({ publicCert })).to.be.empty; + }); + + it("when keyInfoAttributes are set without a publicCert", function () { + expect(selectKeyInfo({ keyInfoAttributes: { Id: "key" } })).to.be.empty; + }); + }); + it("adds id and type attributes to Reference elements when provided", function () { const xml = ""; const sig = new SignedXml();