chore: merge master into 6.x

README.md

@@ -152,7 +152,7 @@ signature algorithms enabled at same time.
 When signing a xml document you can pass the following options to the `SignedXml` constructor to customize the signature process:
 
 - `privateKey` - **[required]** a `Buffer` or pem encoded `String` containing your private key
-- `publicCert` - **[optional]** a `Buffer` or pem encoded `String` containing your public key
+- `publicCert` - **[optional]** the X.509 certificate to publish in `<KeyInfo>`, or a chain of them, as PEM or as one certificate's base64 without the PEM boundaries, in a `String` or `Buffer`. A value that holds no certificate, such as a public key, produces no `<KeyInfo>`.
 - `signatureAlgorithm` - **[required]** one of the supported [signature algorithms](#signature-algorithms). Ex: `sign.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"`
 - `canonicalizationAlgorithm` - **[required]** one of the supported [canonicalization algorithms](#canonicalization-and-transformation-algorithms). Ex: `sign.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#WithComments"`
 
@@ -226,7 +226,7 @@ To customize this see [customizing algorithms](#customizing-algorithms) for an e
 
 When verifying a xml document you can pass the following options to the `SignedXml` constructor to customize the verify process:
 
-- `publicCert` - **[optional]** your certificate as a string, a string of multiple certs in PEM format, or a Buffer
+- `publicCert` - **[optional]** the certificate or public key to verify with, as a PEM `String` or `Buffer`. Verification uses [one key](#one-key-per-value) from it.
 - `privateKey` - **[optional]** your private key as a string or a Buffer - used for verifying symmetrical signatures (HMAC)
 
 The certificate that will be used to check the signature will first be determined by calling `this.getCertFromKeyInfo()`, which function you can customize as you see fit. If that returns `null`, then `publicCert` is used. If that is `null`, then `privateKey` is used (for symmetrical signing applications).
@@ -565,6 +565,18 @@ MIIBxDCCAW6gAwIBAgIQxUSX...
 -----END CERTIFICATE-----
 ```
 
+### One key per value
+
+`privateKey` when signing, and `publicCert` when verifying, are passed to Node's crypto, which uses
+one key from the value.
+
+- `privateKey` holds one private key. A file that also holds its certificate, or its chain, is
+  fine.
+- Verification takes one key from `publicCert`. From several certificates it takes the first, which
+  is how a chain given leaf first works, so the chain's issuers are not trusted to sign.
+- To trust several independent keys, verify with each in turn, as node-saml does for its `idpCert`
+  array.
+
 ### What the parser accepts
 
 `toPem()`, `pemToDer()` and `pemCertificates()` read

src/signed-xml.ts

@@ -38,6 +38,20 @@ function findSignatureElements(node: Node): Element[] {
   return signatures.filter(isDomNode.isElementNode);
 }
 
+function certificatesToPublish(publicCert: string): string[] {
+  const certificates = utils.pemCertificates(publicCert);
+  if (certificates.length > 0) {
+    return certificates;
+  }
+
+  try {
+    return [utils.bareCertificate(publicCert)];
+  } catch {
+    // Not a certificate in either form, so there is none to publish, and KeyInfo is optional.
+    return [];
+  }
+}
+
 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()`. For a detached signature, put an ID attribute the signer recognizes on each referenced element (`wsu:Id` for WS-Security), sign that document, and send it alongside `getSignatureXml()`.",
@@ -229,7 +243,7 @@ export class SignedXml {
     }
 
     // A KeyObject holds a key and never a certificate, so there is no X509Data to build from it.
-    const certificates = typeof publicCert === "string" ? utils.pemCertificates(publicCert) : [];
+    const certificates = typeof publicCert === "string" ? certificatesToPublish(publicCert) : [];
 
     // X509Data requires at least one child: https://www.w3.org/TR/xmldsig-core1/#sec-X509Data
     if (certificates.length === 0) {

src/utils.ts

@@ -408,6 +408,19 @@ function pemText(value: string | Buffer): string {
     : value.toString("base64");
 }
 
+// Refuses PEM rather than passing it through as toPem() does, so no key can come out as a certificate.
+export function bareCertificate(value: string): string {
+  const text = normalizePemInput(value);
+  const data = text.replace(/\n/g, "");
+
+  if (!BASE64_TEXT_REGEX.test(text) || !isBase64Data(data)) {
+    throw new Error("Invalid PEM format.");
+  }
+  assertX509Certificate(data);
+
+  return canonicalBase64(data);
+}
+
 /**
  * Returns a value as canonical PEM: one message per certificate or key, wrapped at 64 characters.
  * The value may be a PEM message, several of them, base64 data with the label supplied by the

test/signature-unit-tests.spec.ts

@@ -1,6 +1,6 @@
 import * as xpath from "xpath";
 import * as xmldom from "@xmldom/xmldom";
-import { SignedXml, createOptionalCallbackFunction } from "../src/index";
+import { SignedXml, createOptionalCallbackFunction, pemCertificates, toPem } from "../src/index";
 import * as fs from "fs";
 import * as crypto from "crypto";
 import { expect } from "chai";
@@ -1442,6 +1442,27 @@ describe("Signature unit tests", function () {
     it("when publicCert is a KeyObject, which holds a key and never a certificate", function () {
       expect(selectKeyInfo({ publicCert: crypto.createPublicKey(privateKey) })).to.be.empty;
     });
+
+    it("when publicCert is a private key", function () {
+      expect(selectKeyInfo({ publicCert: privateKey })).to.be.empty;
+    });
+
+    it("when publicCert is the base64 of a private key, without boundaries", function () {
+      const der = crypto.createPrivateKey(privateKey).export({ type: "pkcs8", format: "der" });
+
+      expect(selectKeyInfo({ publicCert: der.toString("base64") })).to.be.empty;
+    });
+
+    it("when publicCert is the base64 of a certificate with more data after it", function () {
+      const der = fs.readFileSync("./test/static/client_public.der");
+      const publicCert = Buffer.concat([der, Buffer.from("more")]).toString("base64");
+
+      expect(selectKeyInfo({ publicCert })).to.be.empty;
+    });
+
+    it("when publicCert is not a certificate in any form", function () {
+      expect(selectKeyInfo({ publicCert: "not a certificate" })).to.be.empty;
+    });
   });
 
   describe("getCertFromKeyInfo", function () {
@@ -1504,7 +1525,7 @@ describe("Signature unit tests", function () {
     });
   });
 
-  function signWithPublicCert(publicCert: string) {
+  function signWithPublicCert(publicCert: string | Buffer) {
     const sig = new SignedXml({
       privateKey: fs.readFileSync("./test/static/client.pem"),
       publicCert,
@@ -1525,7 +1546,7 @@ describe("Signature unit tests", function () {
   }
 
   // The text of each X509Certificate that signing with this publicCert puts into KeyInfo.
-  function publishedCertificates(publicCert: string): string[] {
+  function publishedCertificates(publicCert: string | Buffer): string[] {
     const doc = new xmldom.DOMParser().parseFromString(signWithPublicCert(publicCert)());
     const certificates = xpath.select("//*[local-name(.)='X509Certificate']", doc);
     isDomNode.assertIsArrayOfNodes(certificates);
@@ -1564,6 +1585,15 @@ describe("Signature unit tests", function () {
     expect(signWithPublicCert(publicCert)).to.throw("Invalid PEM format.");
   });
 
+  it("publishes a publicCert given as the base64 of a certificate, without boundaries", function () {
+    const lines = fs.readFileSync("./test/static/client_public.pem", "latin1").trim().split("\n");
+    const data = lines.slice(1, -1);
+
+    expect(publishedCertificates(data.join(""))).to.deep.equal([data.join("")]);
+    expect(publishedCertificates(data.join("\n"))).to.deep.equal([data.join("")]);
+    expect(publishedCertificates(Buffer.from(data.join("\n")))).to.deep.equal([data.join("")]);
+  });
+
   it("signs a BER certificate into KeyInfo with its octets as given", function () {
     // RFC 7468 section 5.1 allows BER, and XML Signature 1.1 says an implementation SHOULD NOT
     // re-encode a certificate: https://www.w3.org/TR/xmldsig-core1/#sec-X509Data
@@ -1725,4 +1755,43 @@ describe("Signature unit tests", function () {
       "#unique-id",
     );
   });
+
+  it("verifies with the first of two certificates, and not the second, which in a chain is the issuer's", function () {
+    const bundle = fs.readFileSync("./test/static/client_bundle.pem", "latin1");
+    const first = fs.readFileSync("./test/static/client_public.pem", "latin1");
+    const second = toPem(pemCertificates(bundle)[0], "CERTIFICATE");
+
+    function checkSignedBy(privateKey: string, publicCert: string) {
+      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",
+      });
+      sig.addReference({
+        xpath: "//*[local-name(.)='x']",
+        digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256",
+        transforms: [
+          "http://www.w3.org/2000/09/xmldsig#enveloped-signature",
+          "http://www.w3.org/2001/10/xml-exc-c14n#",
+        ],
+      });
+      sig.computeSignature("<root><x /></root>");
+      const xml = sig.getSignedXml();
+
+      const verifier = new SignedXml({ publicCert });
+      const signature = xpath.select1(
+        "//*[local-name(.)='Signature']",
+        new xmldom.DOMParser().parseFromString(xml),
+      );
+      isDomNode.assertIsNodeLike(signature);
+      verifier.loadSignature(signature);
+
+      return verifier.checkSignature(xml);
+    }
+
+    expect(checkSignedBy(fs.readFileSync("./test/static/client.pem", "latin1"), first + second)).to
+      .be.true;
+    expect(checkSignedBy(bundle, second)).to.be.true;
+    expect(() => checkSignedBy(bundle, first + second)).to.throw("invalid signature");
+  });
 });

test/utils-tests.spec.ts

@@ -86,6 +86,8 @@ describe("Utils tests", function () {
         "a line break between quanta": "QUJD\nREVG",
         "a line break anywhere in the data": "QU\nJDRE\nVG",
         "blanks in the data": " QU JD\tREVG ",
+        // RFC 7468 Figure 1 'base64finl': https://www.rfc-editor.org/rfc/rfc7468#section-3
+        "a pad split across a line ending": "QUJDCg=\n=",
       };
 
       const rejected = {
@@ -101,6 +103,8 @@ describe("Utils tests", function () {
         "a character outside the base64 alphabet": "QU-JD",
         "nothing at all": "",
         "only blanks": "    ",
+        "only padding": "==",
+        "a padded line with more data after it": "QUJDCg==\nQUJD",
       };
 
       Object.entries(accepted).forEach(([description, data]) => {
@@ -200,6 +204,31 @@ describe("Utils tests", function () {
         );
       });
 
+      for (const [name, prefix] of [
+        ["a UTF-8 BOM", Buffer.from([0xef, 0xbb, 0xbf])],
+        ["blank lines", Buffer.from("\r\n  \n")],
+      ] as const) {
+        it(`a Buffer holding a PEM file that opens with ${name}, rather than DER`, function () {
+          const file = Buffer.concat([prefix, fs.readFileSync("./test/static/client_public.pem")]);
+
+          expect(utils.toPem(file)).to.equal(normalizedPem);
+        });
+      }
+
+      for (const [name, value, label] of [
+        ["blank lines around a message", `\n\n${normalizedPem}\n\n`, undefined],
+        ["a line ending after bare base64", `${body.join("\n")}\n`, "CERTIFICATE"],
+        [
+          "blanks and CRLF around bare base64",
+          `\r\n  \r\n${body.join("\r\n")}\r\n\t\r\n`,
+          "CERTIFICATE",
+        ],
+      ] as const) {
+        it(name, function () {
+          expect(utils.toPem(value, label)).to.equal(normalizedPem);
+        });
+      }
+
       it("several certificates in one value", function () {
         const bundle = fs.readFileSync("./test/static/client_bundle.pem", "latin1");
 
@@ -382,6 +411,49 @@ describe("Utils tests", function () {
         );
       });
     });
+
+    describe("rejects a value it cannot account for whole", function () {
+      const normalizedPem = fs.readFileSync("./test/static/client_public.pem", "latin1");
+      const lines = normalizedPem.trim().split("\n");
+      const header = lines[0];
+      const footer = lines[lines.length - 1];
+      const body = lines.slice(1, -1);
+      const last = body.length - 1;
+
+      for (const [place, value] of [
+        ["the header's line", [`${header}${body[0]}`, ...body.slice(1), footer].join("\n")],
+        [
+          "the footer's line",
+          [header, ...body.slice(0, last), `${body[last]}${footer}`].join("\n"),
+        ],
+        ["both boundaries' line", `${header}${body.join("")}${footer}`],
+      ] as const) {
+        it(`data on ${place}`, function () {
+          expect(() => utils.toPem(value)).to.throw("Invalid PEM format.");
+        });
+      }
+
+      it("a message that is opened and never closed", function () {
+        // node-saml's ReDoS regression; the rejection is asserted, not the time it takes.
+        const unclosed = `-----BEGIN CERTIFICATE-----\r\n${"AAAA\r\n".repeat(26)}!`;
+
+        expect(() => utils.toPem(unclosed)).to.throw("Invalid PEM format.");
+      });
+
+      it("a header followed directly by its footer", function () {
+        expect(() => utils.toPem(`${header}\n${footer}\n`)).to.throw("Invalid PEM format.");
+      });
+
+      for (const [place, value] of [
+        ["before the message", `subject=/CN=client\n${normalizedPem}`],
+        ["after the message", `${normalizedPem}Issued for testing.\n`],
+        ["between two messages", `${normalizedPem}and its issuer:\n${normalizedPem}`],
+      ] as const) {
+        it(`explanatory text ${place}`, function () {
+          expect(() => utils.toPem(value)).to.throw("Invalid PEM format.");
+        });
+      }
+    });
   });
 
   describe("pemToDer", function () {