Document that privateKey and publicCert use only one of several keys

[cjbarth]

Summary

toPem() keeps every message in a value, and the verification options in the README describe publicCert as "your certificate as a string, a string of multiple certs in PEM format, or a Buffer" (README). privateKey when signing, and publicCert when verifying, are then passed to Node's crypto, which uses one key from the value and ignores the rest. The README doesn't say this, and nothing reports what was ignored.

A privateKey holding two private keys signs with the first.

A verification publicCert holding two keys trusts only one of them:

publicCert	Accepts a signature by
public key A, then public key B	A only
certificate A, then certificate B	A only
certificate A, then public key B	B only
public key B, then certificate A	B only

So "a string of multiple certs" verifies against only its first certificate. When a value mixes the two forms, Node takes the public key whatever the order. Node 16 and Node 26 behave the same way.

This fails closed: a signature by an ignored key is rejected, not accepted. The cost is confusion. A caller who puts an old and a new certificate in one value for a key rollover will have the new one rejected on the day of the switch, and the README told them that would work.

Reproduction

Run from the repository root, with xml-crypto resolving to 6.3.0:

const crypto = require("crypto");
const xmldom = require("@xmldom/xmldom");
const xpath = require("xpath");
const { SignedXml } = require("xml-crypto");

function keyPair() {
  const { privateKey, publicKey } = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
  return {
    privateKey: privateKey.export({ type: "pkcs8", format: "pem" }),
    publicKey: publicKey.export({ type: "spki", format: "pem" }),
  };
}

function sign(privateKey) {
  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(.)='book']",
    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("<library><book/></library>");
  return sig.getSignedXml();
}

function verifies(xml, publicCert) {
  const doc = new xmldom.DOMParser().parseFromString(xml, "text/xml");
  const sig = new SignedXml({ publicCert });
  sig.loadSignature(xpath.select1("//*[local-name(.)='Signature']", doc));
  try {
    return sig.checkSignature(xml);
  } catch {
    return false;
  }
}

const a = keyPair();
const b = keyPair();
const signedByA = sign(a.privateKey);
const signedByB = sign(b.privateKey);

// Signing with two private keys uses the first.
console.log(verifies(sign(a.privateKey + b.privateKey), a.publicKey)); // true
console.log(verifies(sign(a.privateKey + b.privateKey), b.publicKey)); // false

// Verifying with two public keys trusts the first.
console.log(verifies(signedByA, a.publicKey + b.publicKey)); // true
console.log(verifies(signedByB, a.publicKey + b.publicKey)); // false

Scope for 6.4

Nothing here may fail a configuration that signs or verifies today.

• Fix the README. privateKey holds one private key, and a file that also holds its certificate is fine. Verification takes one key from publicCert, and from several certificates it takes the first, which is how a chain given leaf first works. To trust several independent keys, verify with each in turn, as node-saml does for its idpCert array. Replace "a string of multiple certs in PEM format" with that.
• Test what the README then promises: a value holding two certificates verifies a signature made with the first certificate's key, and does not verify one made with the second's. The second half is the important one. It stops a later change from "fixing" this by trying each key in turn, which would make a chain trust its issuer's key.

Warnings for the ambiguous cases were considered and rejected. Detecting them means re-parsing key material that Node already parses on every sign and verify call, to second-guess Node's own key parser, for a mistake that already fails closed. That is duplicate machinery with no security benefit, so this issue asks for no warning and no new error.

Not in this issue

• Rejecting ambiguous values. Reject a privateKey or publicCert that holds several keys with no single reading #608 proposed it for 7.0 and is closed as not planned, for the same reason the warnings were dropped.
• Anything that changes how several keys or certificates are represented or iterated. That belongs with the 7.0 key-type work in Decouple public key and data types from node:crypto (BinaryLike / KeyLike) #545, not in 6.x.

[cjbarth]

#611 fixes the README and adds the test. It leaves out the warnings.

privateKey and publicCert are typed crypto.KeyLike, so what counts as a valid key value is whatever Node's crypto reads. RFC 7468 section 2 says "Files MAY contain multiple textual encoding instances". Node reads such a value and fails closed: a signature made with an ignored key is rejected. Nothing here should throw, so a warning ahead of an error would only be code to maintain. #608 is closed as not planned for the same reason.