Document that signing's publicCert needs a certificate, and accept one given as bare Base64

[cjbarth]

Summary

When signing, publicCert has one job: supplying the certificate that goes in KeyInfo. Since #597, the default SignedXml.getKeyInfoContent returns null when pemCertificates() finds no certificate in publicCert (src/signed-xml.ts#L220-L243), and computeSignature() then leaves KeyInfo out. Leaving it out was right. The alternative was the invalid <X509Data/> from #596. But whatever publicCert holds, signing succeeds, so a caller who set it has no way to learn that it did nothing.

On 6.3.0:

publicCert	Result
a PEM certificate	KeyInfo included
a PEM public key	no KeyInfo, no error
a PEM private key	no KeyInfo, no error
the Base64 of a certificate, without boundaries	no KeyInfo, no error
"not a certificate"	no KeyInfo, no error
a KeyObject	no KeyInfo, no error

The README invites the second row. The signing options describe publicCert as "a Buffer or pem encoded String containing your public key" (README), and only a note further down says that KeyInfo needs an X.509 certificate.

The fourth row is inconsistent with the rest of the library. toPem(value, "CERTIFICATE") reads a bare Base64 certificate, and getCertFromKeyInfo() reads an X509Certificate element's content that way, so the same certificate is understood when verifying but ignored when signing.

node-saml hit this in node-saml/node-saml#409, where a public key given as its publicCert option produced an AuthnRequest without KeyInfo. It now checks pemCertificates() itself before assigning publicCert (985a4b0).

Reproduction

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

const crypto = require("crypto");
const fs = require("fs");
const { SignedXml } = require("xml-crypto");

const privateKey = fs.readFileSync("test/static/client.pem", "latin1");
const certificate = fs.readFileSync("test/static/client_public.pem", "latin1");

function keyInfo(publicCert) {
  const sig = new SignedXml({
    privateKey,
    publicCert,
    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/2001/10/xml-exc-c14n#"],
  });
  sig.computeSignature("<library><book/></library>");
  return sig.getSignatureXml().includes("<KeyInfo>") ? "KeyInfo" : "no KeyInfo";
}

const publicKey = crypto.createPublicKey(privateKey);
console.log(keyInfo(certificate)); // KeyInfo
console.log(keyInfo(publicKey.export({ type: "spki", format: "pem" }))); // no KeyInfo
console.log(keyInfo(privateKey)); // no KeyInfo
console.log(keyInfo(certificate.trim().split("\n").slice(1, -1).join(""))); // no KeyInfo
console.log(keyInfo("not a certificate")); // no KeyInfo
console.log(keyInfo(publicKey)); // no KeyInfo

Scope for 6.4

Nothing here may fail a configuration that signs today.

• Fix the signing options in the README: publicCert is a certificate or a chain of them, and a value without a certificate produces no KeyInfo.
• Read a bare Base64 publicCert as a certificate when it is one, as toPem(value, "CERTIFICATE") does. A value that isn't a certificate keeps today's behavior.
• Add a regression test for each row of the table, asserting the result the README then documents.

Warning: decided against

A warning when no certificate is found was left to review, and review declined it. KeyInfo is optional (XMLDSig 4.5), so signing without one is valid output, and a warning for something that shouldn't throw is code to maintain for no gain. #497 shows what a warning at a working application costs. #610 records the decision.

Not in this issue

Making signing throw in these cases is a breaking change, so it is tracked for 7.0 in #598, together with the keyInfoAttributes case. #598 also covers whether a key object given as the signing publicCert should be an error.