Generalized provisioning of crypto keys from OpenSSL engines and/or providers

[nwf-msr]

What is the problem this feature will solve?

The fix for #15113, #24234, added support for crypto key objects, but the issue notes that "Loading keys from engines is not supported yet.". Later, c2ce8d0 (see #28973) added support for loading engine keys specifically for TLS, which means that the requisite calls to OpenSSL had to be added but only for SecureContext objects. In the interim, this code has migrated over to crypto_context.cc, but it is still exclusively associated with SecureContext objects, meaning, I think, that it is still not possible to use the lower-level cryptographic primitives of the crypto API with engine keys.

This impacts node.js downstream libraries, by requiring that applications handle exposed private key material to, for example, construct JWTs. See Azure/azure-sdk-for-js#22011 and octokit/auth-app.js#336 .

What is the feature you are proposing to solve the problem?

I would like the API of #24234 to be extended to support loading key objects from engines. This should be, I think, a fairly straightforward refactoring of existing code.

What alternatives have you considered?

There appear to be no general-purpose, viable alternatives that do not require exposing keying material to the running node.js application. It could be possible to specifically modify https://github.com/auth0/node-jwa to reach around node.js to, say, contact an openssh agent for PKCS#11 services, but this is much less preferable to just allowing the built-in crypto layer, which jwa already uses, able to load engine keys.

[bnoordhuis]

Can you post an example of what you think the API should look like?

[nwf-msr]

Riffing off the existing APIs, specifically...

• crypto.createPrivateKey,
• tls.createSecureContext, and
• OpenSSL req's -keyform parameter and its implication for -inkey,

I think I want to extend crypto.createPrivateKey to accept an object key with

• its .format field being the string "engine",
• an optional .engine string field behaving like tls.createSecureContext's privateKeyEngine
• its .key then being a string identifier suitable for the engine's use (that is, acting like tls.createSecureContext's privateKeyIdentifier).

So, for example, using the OpenSC PKCS#11 engine, I could write something like

p11key = crypto.createPrivateKey(
  { format: "engine"
  , engine: "pkcs11"
  , key: "pkcs11:token=sometokenname;object=somekeyname;type=private"
  })

(For this engine, the suitable key identifiers are PKCS#11 URIs or its custom naming scheme.)

Using https://github.com/auth0/node-jwa as an example consumer of such an API, we see that it simply passes its privateKey arguments through to crypto.createSign().sign(), which is documented to behave as though non-KeyObject privateKey values are passed through crypto.createPrivateKey first.

This kind of pass-through extends quite high in application stacks and would address at least one of my two examples (octokit/auth-app.js#336) quite nicely: because private keys are simply passed down

• to jwa
• through jws and
• through jsonwebtoken and
• through universal-github-app-jwt and
• through octokit/auth-app
• from octokit/app,

the proposed API change would mean that changing only the very top of an OctoKit.js GitHub App (or even its json configuration file), from specifying a string privateKey to instead specifying a suitable object, would likely be enough to use keys in a PKCS#11 HSM.

(The other example, Azure/azure-sdk-for-js#22011, starts off well from jsonwebtoken and ascending through msal, but it runs into ad-hoc validation logic in https://github.com/Azure/azure-sdk-for-js/blob/191e4ce330acd60e014c13412204b9598870cfa1/sdk/identity/identity/src/msal/nodeFlows/msalClientCertificate.ts#L83-L90, and so the Azure SDK will probably need a new, fairly simple, kind of nodeFlow for this engine-based workflow.)

In any case, all of that argues, I think, for the suitability of this kind of extension to createPrivateKey and, similarly, createPublicKey. I'm not going to insist that create*Key pronounce the identifier as key as OpenSSL does, and the use of keyIdentifier a la createSecureContext also seems fine; the real key piece is format: "engine" and its semantics.

[bnoordhuis]

Seems reasonable. For feature parity there should probably be a similar "bridge" in crypto.createPublicKey() to ENGINE_load_public_key().

cc @nodejs/crypto - needs your input.

[bnoordhuis]

I'll take the lack of comments as quiet approval. :-)

@nwf-msr Want to open a pull request?

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[nwf-msr]

Please don't close this as "stale". It's still an outstanding issue.