fix: omit KeyInfo when there is no content for it

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 `<KeyInfo />` 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 `<KeyInfo />` element, it won't be included in the
-generated XML.
+it returns no content, the `<KeyInfo />` element is not included in the generated XML, even when
+`keyInfoAttributes` are set.
 
 To specify custom attributes on `<KeyInfo />`, add the properties to the `.keyInfoAttributes` property.
 

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 "<foo:X509Data></foo:X509Data>"
+   * the resultant XML string will be "<foo:X509Data><foo:X509Certificate>...</foo:X509Certificate></foo:X509Data>"
    *
    * @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")}</${prefix}X509Certificate>`,
-        )
-        .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")}</${prefix}X509Certificate>`,
+      )
+      .join("");
+
     return `<${prefix}X509Data>${x509Certs}</${prefix}X509Data>`;
   }
 
@@ -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}</${currentPrefix}KeyInfo>`;
-    }
-
-    return "";
+    return `<${currentPrefix}KeyInfo${keyInfoAttrs}>${keyInfoContent}</${currentPrefix}KeyInfo>`;
   }
 
   /**

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<typeof SignedXml>[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("<root><x /></root>");
+      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 = "<root><x /></root>";
     const sig = new SignedXml();