feat: check private-key data against its label in toPem() and pemToDer()

README.md

@@ -654,6 +654,16 @@ Rejected, with an error rather than a certificate:
   certificate cut short, or one with more bytes after it. Node's
   [`X509Certificate`](https://nodejs.org/api/crypto.html#class-x509certificate) decides whether
   the data is a certificate, so it is judged as X.509 and not by its base64 alone.
+- a `PRIVATE KEY` or `RSA PRIVATE KEY` whose data Node cannot read as a private key under that
+  label: PKCS #1 data labelled `PRIVATE KEY`, a public key, or base64 of something else. Node's
+  [`crypto.createPrivateKey()`](https://nodejs.org/api/crypto.html#cryptocreateprivatekeykey)
+  decides, so the answer follows the Node version: Node 16 refuses PKCS #8 data labelled
+  `RSA PRIVATE KEY`, and later versions read it. Base64 does not say whether a key is PKCS #8,
+  which is `PRIVATE KEY`, or PKCS #1, which is `RSA PRIVATE KEY`, so a caller that takes either as
+  bare base64 tries `PRIVATE KEY` first and `RSA PRIVATE KEY` second. In that order each key comes
+  out under its own label on every supported Node version. Tried the other way round, PKCS #8 data
+  comes out labelled `RSA PRIVATE KEY` on a Node version that reads it so, and Node 16 then refuses
+  that PEM.
 - a header with no data under it, and a blank line in the middle of a message's data.
 - a boundary sharing its line with other text.
   [Figure 1](https://www.rfc-editor.org/rfc/rfc7468#section-3) gives an encapsulation boundary a

src/utils.ts

@@ -1,4 +1,4 @@
-import { X509Certificate } from "crypto";
+import { X509Certificate, createPrivateKey } from "crypto";
 import * as xpath from "xpath";
 import type { NamespacePrefix, PemLabel } from "./types";
 import * as isDomNode from "@xmldom/is-dom-node";
@@ -348,12 +348,35 @@ export function normalizePem(pem: string): string {
 // same bytes whatever line width, line ending or blanks it arrived with. Only the data is handed
 // to `normalizePem`, which wraps at 64 characters whatever it is given: a boundary broken across
 // two lines would be a message this parser could no longer read back.
-function formatPemMessage(label: string, data: string): string {
+function writePemMessage(label: string, data: string): string {
+  return `-----BEGIN ${label}-----\n${normalizePem(canonicalBase64(data))}-----END ${label}-----\n`;
+}
+
+const PRIVATE_KEY_LABELS = ["PRIVATE KEY", "RSA PRIVATE KEY"];
+
+// Base64 does not say whether it holds PKCS #8 or PKCS #1, and the label does, so Node is asked to
+// read the data under the label it carries: https://www.rfc-editor.org/rfc/rfc7468#section-10
+function assertPrivateKey(label: string, data: string): void {
+  try {
+    createPrivateKey(writePemMessage(label, data));
+  } catch {
+    throw new Error("Invalid PEM format.");
+  }
+}
+
+function assertEncapsulatedData(label: string, data: string): void {
   if (label === "CERTIFICATE") {
     assertX509Certificate(data);
   }
+  if (PRIVATE_KEY_LABELS.includes(label)) {
+    assertPrivateKey(label, data);
+  }
+}
 
-  return `-----BEGIN ${label}-----\n${normalizePem(canonicalBase64(data))}-----END ${label}-----\n`;
+function formatPemMessage(label: string, data: string): string {
+  assertEncapsulatedData(label, data);
+
+  return writePemMessage(label, data);
 }
 
 /**
@@ -392,7 +415,8 @@ export function pemCertificates(pem: string): string[] {
  * Returns the decoded bytes of the one PEM message a value holds, whatever its label.
  *
  * @param pem The PEM message to decode
- * @throws Error if the value is not a single well-formed PEM message
+ * @throws Error if the value is not a single well-formed PEM message, or if its data is not what
+ *   a `CERTIFICATE`, `PRIVATE KEY` or `RSA PRIVATE KEY` label says it is
  */
 export function pemToDer(pem: string): Buffer {
   const text = normalizePemInput(pem);
@@ -409,9 +433,7 @@ export function pemToDer(pem: string): Buffer {
   }
 
   const [{ label, data }] = messages;
-  if (label === "CERTIFICATE") {
-    assertX509Certificate(data);
-  }
+  assertEncapsulatedData(label, data);
 
   return Buffer.from(data, "base64");
 }
@@ -451,8 +473,9 @@ export function bareCertificate(value: string): string {
  *
  * @param value The certificate or key to return as PEM
  * @param pemLabel The label to give base64 data, which needs one; ignored when the value is PEM
- * @throws Error if the value is neither a well-formed PEM nor base64, or if it is base64 and no
- *   label, or an unusable one, was given
+ * @throws Error if the value is neither a well-formed PEM nor base64, if it is base64 and no
+ *   label, or an unusable one, was given, or if the data is not what a `CERTIFICATE`,
+ *   `PRIVATE KEY` or `RSA PRIVATE KEY` label says it is
  */
 export function toPem(value: string | Buffer, pemLabel?: PemLabel): string {
   const text = normalizePemInput(pemText(value));

test/utils-tests.spec.ts

@@ -581,6 +581,50 @@ describe("Utils tests", function () {
     });
   });
 
+  describe("reads a PRIVATE KEY or RSA PRIVATE KEY message's data as a private key", function () {
+    const key = crypto.createPrivateKey(fs.readFileSync("./test/static/client.pem"));
+    const pkcs8 = key.export({ type: "pkcs8", format: "der" });
+    const pkcs1 = key.export({ type: "pkcs1", format: "der" });
+    const publicKey = crypto.createPublicKey(key).export({ type: "spki", format: "der" });
+    const noKey = Buffer.from("base64, and no private key");
+    const wrap = (label: string, der: Buffer) =>
+      `-----BEGIN ${label}-----\n${der.toString("base64")}\n-----END ${label}-----\n`;
+
+    for (const [structure, der, label] of [
+      ["PKCS #8", pkcs8, "PRIVATE KEY"],
+      ["PKCS #1", pkcs1, "RSA PRIVATE KEY"],
+    ] as const) {
+      it(`accepts ${structure} data labelled ${label}, and keeps its octets as given`, function () {
+        expect(utils.pemToDer(utils.toPem(wrap(label, der)))).to.deep.equal(der);
+        expect(utils.pemToDer(utils.toPem(der.toString("base64"), label))).to.deep.equal(der);
+        expect(utils.pemToDer(utils.toPem(der, label))).to.deep.equal(der);
+      });
+    }
+
+    for (const [problem, der, label] of [
+      ["PKCS #1 data labelled PRIVATE KEY", pkcs1, "PRIVATE KEY"],
+      ["a public key labelled PRIVATE KEY", publicKey, "PRIVATE KEY"],
+      ["base64 that is no key, labelled PRIVATE KEY", noKey, "PRIVATE KEY"],
+      ["base64 that is no key, labelled RSA PRIVATE KEY", noKey, "RSA PRIVATE KEY"],
+    ] as const) {
+      it(`refuses ${problem}, in each function that reads one`, function () {
+        expect(() => utils.toPem(wrap(label, der))).to.throw("Invalid PEM format.");
+        expect(() => utils.toPem(der.toString("base64"), label)).to.throw("Invalid PEM format.");
+        expect(() => utils.toPem(der, label)).to.throw("Invalid PEM format.");
+        expect(() => utils.pemToDer(wrap(label, der))).to.throw("Invalid PEM format.");
+      });
+    }
+
+    it("refuses a mislabelled key in a value that also holds its certificate", function () {
+      const certificate = fs.readFileSync("./test/static/client_public.pem", "latin1");
+
+      expect(() => utils.toPem(wrap("PRIVATE KEY", pkcs1) + certificate)).to.throw(
+        "Invalid PEM format.",
+      );
+      expect(utils.pemCertificates(wrap("PRIVATE KEY", pkcs1) + certificate)).to.have.lengthOf(1);
+    });
+  });
+
   describe("pemCertificates", function () {
     const bundle = fs.readFileSync("./test/static/client_bundle.pem", "latin1");