Design the sync/async model for 7.0 (Web Crypto, remote keys, and retiring the callback overloads)

[cjbarth]

Summary

Decide how xml-crypto exposes synchronous and asynchronous operation in 7.0, and restructure the internals so both are supported without duplicating the signing and verification logic.

This is the design half of the Web Crypto work. #545 covers the type abstraction; this issue covers control flow. Neither should be settled in isolation, but they can be implemented separately.

Why this needs deciding now

Three separate things want an asynchronous path, and today none of them has a working one:

• Web Crypto ([ENHANCEMENT]: web-crypto support #477, Added web-crypto support (#1) #513) — crypto.subtle is promise-only. There is no synchronous subtle.sign(), so any non-Node runtime forces async.
• Remote keys — HSM / KMS / signing-server implementations of SignatureAlgorithm. This is the documented reason the callback overloads exist.
• checkSignature's async path is currently broken. checkSignature calls verifySignature in its three-argument synchronous form and never passes the callback down, so an async-only verifier returns undefined and a valid signature is reported as incorrect. Regressed in v6.1.0; still present. Fails closed, so it is a correctness bug rather than a hole, but the README documents a feature that does not work.

Meanwhile the synchronous path is the one nearly every consumer uses, and it must not get slower or become contagious.

What we measured

Promise overhead is not the constraint

RSA-2048 sign, sync           : 738.8 us
RSA-2048 sign, async-wrapped  : 753.3 us
bare await, no work           : 0.675 us
promise overhead as % of sign : 0.091%

An await is under a tenth of a percent of an RSA signature. The reason not to make everything async is caller contagion, not CPU: a consumer validating a SAML response inside a synchronous code path cannot call an async method without restructuring upward. That is an API cost and it is real, but it is not a performance cost.

The dual-mode pattern is expensive internally

#513 keeps one function that is sync or async depending on whether a callback was passed. To make that work, every method on the call path had to be doubled:

private validateReference(ref, doc): boolean;
private validateReference(ref, doc, callback): void;
private createReferences(doc, prefix): string;
private createReferences(doc, prefix, callback): void;
private createSignedInfo(doc, prefix): string;
private createSignedInfo(doc, prefix, callback): void;

each with // Sync mode / // Async mode branches, plus a hand-rolled sequential accumulator for the reference loop. signed-xml.ts grows by +340/−114, and most of that is bridging rather than function. It also replicates the seam that produced #527, five more times.

"Sync unless you provide a callback" is observable to the caller

One piece of caller code, two algorithms, dual-mode semantics:

[A] Node algorithm  (sync)
   callback ran. audit is: null          <- handler runs before the caller finishes setup
   caller finished setup.

[B] WebCrypto algo  (async)
   caller finished setup.
   callback ran. audit is: {"signedAt":"now"}

and, more seriously:

[A] Node algorithm  -> try/catch CAUGHT: caller's handler failed
[B] WebCrypto algo  -> try/catch did NOT catch it
   ...it surfaced later as uncaughtException: caller's handler failed

Switching signatureAlgorithm — a one-line configuration change — silently changes whether the caller's own try/catch works, and whether state assigned after the call is visible to the handler. For a callback that carries "is this signature valid", that is not academic.

What Node actually does

Node's error-first callbacks are always asynchronous, including on the error path and including calls it could answer immediately:

  caller finished setup.              <- always first
  readFile error   cb -> after is: set    (/definitely/not/here)
  dns.lookup cb       -> after is: set    (localhost, from /etc/hosts)
  readFile success cb -> after is: set

Node's answer to sync-vs-async is a pair of separately named functions — readFile / readFileSync, writeFile / writeFileSync, and roughly forty more. "Sync unless a callback is provided" is the thing that convention exists to avoid.

Proposed direction

1. Write the logic once: sync phases separated by async barriers

The asynchronous work is confined to three leaf operations — getHash, getSignature, verifySignature — across six call sites in signed-xml.ts. Everything around them (canonicalization, XPath, digest comparison, DOM assembly) is synchronous computation with nothing to await. So each flow is a sequence of synchronous phases with exactly two async barriers:

sign:   canonicalize refs -> [hash each] -> assemble + canonicalize SignedInfo -> [sign] -> insert & serialize
verify: canonicalize refs -> [hash each] -> compare digests, canonicalize SignedInfo -> [verify] -> set signedReferences

Write the phases once, synchronous, with no modes. Then two thin orchestrators:

computeSignature(xml: string): void {
  const refs   = this.canonicalizeReferences(doc);           // shared, sync
  const digest = refs.map((r) => hash.getHash(r.canon));     // sync leaf
  const info   = this.assembleSignedInfo(digest);            // shared, sync
  this.finalize(algo.getSignature(info, this.privateKey));   // sync leaf + shared
}

async computeSignatureAsync(xml: string): Promise<SignedXml> {
  const refs   = this.canonicalizeReferences(doc);                          // same fn
  const digest = await Promise.all(refs.map((r) => hash.getHashAsync(r.canon)));
  const info   = this.assembleSignedInfo(digest);                           // same fn
  this.finalize(await algo.getSignatureAsync(info, this.privateKey));       // same fn
  return this;
}

The mode lives at the top where it is visible, rather than threaded through five private methods.

2. Optional async twins on the algorithm interfaces

interface HashAlgorithm {
  getAlgorithmName(): HashAlgorithmType;
  getHash(xml: string): string;
  getHashAsync?(xml: string): Promise<string>;
}

Same shape for SignatureAlgorithm. Implementations pick a side: a Node-backed algorithm implements the synchronous method, a Web Crypto-backed one implements the asynchronous method. Nobody writes both, so this is not a doubled API for implementers.

3. Fail closed at the wrong entry point

Calling computeSignature() with an async-only algorithm configured throws immediately and says what to do:

WebCryptoSha256 is async-only; use computeSignatureAsync()

#513 gets this instinct right for hashes. With separate methods the type system carries most of it and the guard covers the JavaScript caller.

4. Retire the callback overloads

createOptionalCallbackFunction and the callback forms go. A caller who wants a callback can .then() or promisify. Removing them is a loud break — a failed compile or a TypeError — rather than the silent one that changing callback timing would produce.

If we would rather keep a callback form, it must be always deferred, matching Node. Dual-mode timing should not be designed in deliberately.

Open questions

• Do the async entry points get their own names (computeSignatureAsync) or a separate namespace / class?
• Should checkSignature's currently-broken async path be repaired in 6.x first, or left to be superseded by this work in 7.0?
• Do we need a bridging helper for implementers who genuinely want one algorithm class usable in both modes?
• Does anything other than the three crypto leaves need to become async? Canonicalization and XPath are believed to be entirely synchronous — worth confirming before committing to the phase structure.
• Error semantics: computeSignature(xml, cb) today throws synchronously for signatureAlgorithm is required and Private key is required, but calls back for location.action. In the promise API every failure becomes a rejection. Confirm that is what we want.

References

• Decouple public key and data types from node:crypto (BinaryLike / KeyLike) #545 — BinaryLike / KeyLike type abstraction (the other half of this work)
• [ENHANCEMENT]: web-crypto support #477 — Web Crypto support request
• Added web-crypto support (#1) #513 — draft Web Crypto PR; source of the dual-mode cost measurement
• Callback invoked twice on unhandled exception #527 — callback invoked twice; the bug this seam produced in 6.x