crypto: add KeyObject.prototype.toCryptoKey

[panva]

For universal libraries executed in Node.js it would be useful to have an efficient way of accepting a KeyObject instance and transforming it to CryptoKey so that the actual crypto implementation doesn't have to depend on importing node:crypto but instead just use global SubtleCrypto.

To that end the application can check for key's Symbol.toStringTag being KeyObject and then having it wrapped around with a CryptoKey structure.

let publicKey!: crypto.KeyObject
let privateKey!: crypto.KeyObject

publicKey.toCryptoKey('Ed25519', true, ['verify']) // CryptoKey
privateKey.toCryptoKey('Ed25519', true, ['sign']) // CryptoKey

This PR adds the prototype methods and then utilizes the WebCryptoAPI implementation internals to run all the same routines as if SubtleCrypto.importKey was called, ensuring the CryptoKey is in the same state and conformance as if the KeyObject instance was to be called export() on and then SubtleCrypto.importKey was called. This PR makes that trip much more efficient.

The signature of this method is the same as SubtleCrypto.importKey sans that it's not async and the format and keyData arguments are not needed. The processing of arguments and errors work the same as for SubtleCrypto.importKey in that they are processed using its webidl definition (incl. converters)

The counterpart of this is KeyObject.from which takes a CryptoKey and returns its KeyObject handle.

I've considered making this a KeyObject.export option but there's an interaction with generateKeyPair(Sync) which would make the implementation really quirky, but possible if that's what we'd like to see instead.

[nodejs-github-bot]

Review requested:

• @nodejs/crypto

[mcollina]

Past experiences tells me that adding properties to standard objects might worsen the compatibility with other runtimes. How is this different?

[panva]

Past experiences tells me that adding properties to standard objects might worsen the compatibility with other runtimes. How is this different?

It's different in that it's not adding anything to a standards object. The importKey function surface remains unchanged, it's not accepting any new formats and the new functionality is exposed entirely on the KeyObject prototype.

The only point of interaction with other runtimes is if they want to have compatibility with node's own KeyObject API

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62918/

[codecov]

Codecov Report

Attention: Patch coverage is 86.12440% with 29 lines in your changes missing coverage. Please review.

Project coverage is 88.42%. Comparing base (36ca010) to head (cc6d809).
Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/crypto/keys.js	84.27%	25 Missing ⚠️
lib/internal/crypto/mac.js	78.94%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55262      +/-   ##
==========================================
+ Coverage   88.40%   88.42%   +0.02%     
==========================================
  Files         652      652              
  Lines      186573   186744     +171     
  Branches    36045    36089      +44     
==========================================
+ Hits       164938   165127     +189     
+ Misses      14903    14897       -6     
+ Partials     6732     6720      -12     

Files with missing lines	Coverage Δ
lib/internal/crypto/aes.js	90.46% <100.00%> (+0.13%)	⬆️
lib/internal/crypto/cfrg.js	96.97% <100.00%> (+0.04%)	⬆️
lib/internal/crypto/ec.js	93.20% <100.00%> (+0.41%)	⬆️
lib/internal/crypto/rsa.js	90.58% <100.00%> (+0.13%)	⬆️
lib/internal/crypto/webcrypto.js	96.18% <100.00%> (+1.44%)	⬆️
lib/internal/crypto/mac.js	88.83% <78.94%> (-1.06%)	⬇️
lib/internal/crypto/keys.js	93.90% <84.27%> (-1.86%)	⬇️

... and 47 files with indirect coverage changes

[panva]

More test coverage is coming.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62920/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62922/

[panva]

@tniessen

This generally looks good to me.

👍

My only concern is that it makes it even easier to make a non-extractable CryptoKey extractable. Do you have an opinion on whether KeyObject.from() should work for non-extractable keys @panva @jasnell?

I don't think this ability makes it any easier. I don't think converting key types is an issue, but not respecting the key properties during this conversion is, which is why this PR honours all validations of SubtleCrypto.importKey.

I think we can track two separate issues that make use of CryptoKey in node:crypto APIs strange

• the one you pointed out - KeyObject.from doesn't respect the extractable property
• all other node:crypto functions that accept a CryptoKey do not respect any of its algorithm's properties

I'll open a dedicated issue to discuss these and land this PR, is that okay @tniessen?

[tniessen]

Yes, that's fine by me, thank you @panva!

[nodejs-github-bot]

Landed in 90e3e5e