crypto: deprecate KeyObject.from but support CryptoKeys in create*Key

[tniessen]

This renames KeyObject.from to KeyObject.fromCryptoKey. Currently, it only accepts CryptoKey instances as inputs, and I don't see that changing any time soon.

This is mainly for API safety. KeyObject.from is a very generic name for something that has a very specific purpose. Also, the name says absolutely nothing about the type of the parameter:

function something(someKeyThing) {
  // ...
  KeyObject.from(someKeyThing); // What is the input here?!
  // ...
}

The function is currently undocumented, I'll update this if/when #37198 lands.

I don't think this function is being tested at all, but I might have missed it. I'd love to hear other opinions if someone thinks we should keep the function as it is (modulo being documented).

This deprecates KeyObject.from and adds support for CryptoKey instances to createPrivateKey etc.

It's also questionable that non-extractable instances of CryptoKey can apparently be converted to unprotected KeyObjects , which then allow extracting the key material. Not sure if that's a feature or a bug.

[tniessen]

cc @nodejs/crypto for opinions and input. For context: #37198 (comment)

[panva]

I think we can close #37198 and, if docs are added here for the renamed method, accept this change.

I'm indifferent to whether non-extractable can be passed or not. It one really wants the keyobject they can get the kKeyObkect handle symbol anyway.

[tniessen]

I think we can close #37198 and, if docs are added here for the renamed method, accept this change.

@panva I think #37198 should still land, at the very least for the other changes that are unrelated to this PR :)

[jasnell]

I'm -1 on this. The naming was intentionally generic. While this only works with CryptoKey now, there's no reason it cannot support other inputs in the future...

If we don't want KeyObject.from(), if anything, let's make the crypto.create(Private|Public|Secret)Key() variants accept a CryptoKey and make it unwrap the KeyObject from the CryptoKey instead of creating a new instance.

[tniessen]

there's no reason it cannot support other inputs in the future...

@jasnell Well, that's really the question that it boils down to. What other inputs could we safely handle here that aren't already covered by other APIs? I don't think JWK is a safe candidate here because it's not a uniquely identifiable data type, and because the user would have no control over the resulting key type (public versus private).

[panva]

FYI crypto.create(Private|Public|Secret)Key() for JWK is coming along nicely.

[tniessen]

Adding to #37240 (comment)...

If we don't want KeyObject.from(), if anything, let's make the crypto.create(Private|Public|Secret)Key() variants accept a CryptoKey and make it unwrap the KeyObject from the CryptoKey instead of creating a new instance.

@jasnell I can change this PR to match that if you and/or other people from @nodejs/crypto prefer this. Maybe I'm wrong and KeyObject.from does make sense for generic inputs, but I really don't know what inputs that could be.

[panva]

So far crypto.create(Private|Public|Secret)Key() has been used for key material - pem, der, coming jwk. CryptoKey is already a representantion of a parsed key material. One exception to that is passing private keyobject to createPublicKey.

[jasnell]

Let's just add CryptoKey as an option for the create variants, then we can deprecate KeyObject.from

[tniessen]

Let's just add CryptoKey as an option for the create variants, then we can deprecate KeyObject.from

On it, but not ready yet.