keyobject.asymmetricKeyType crashes on C++ assertion

[panva]

A followup discussion to #26319

We're still crashing here on any unhandled key. e.g:

-----BEGIN PRIVATE KEY-----
MC4CAQAwBQYDK2VuBCIEILD/13Y5R/tmcCjZVSooIcpfGvZxf+qt6dMu5FYaOC1a
-----END PRIVATE KEY-----

This makes the API rather unstable since end-users can force key material that will crash the process. Developers have no way to detect this other than parsing the key material before sending it to create(private|public)Key, kinda killing the KeyObject point.

The sane thing here would be return "unsupported"/"unknown" or to not instantiate the KeyObject at all and throw in JS instead.

Returning unsupported is enough for libraries using KeyObjects to check if they want to accept the key.

/cc @tniessen @sam-github @bnoordhuis what do you think?

[tniessen]

You are right... Maybe we shouldn't allow such keys to be created in the first place? There are some OIDs we should support and I intend to work on that soon (currently working on .fields), but I don't know whether ECDH keys are useful in this context.

[tniessen]

Or, as you said, we could allow to construct them as long as OpenSSL supports it and return unsupported... This did not sound like a good idea to me at first, but now that I'm thinking about it, it might just work. Initially, I thought it would be easy to enumerate all key types supported by OpenSSL, but in the end, we might keep running into such problems.

[panva]

but I don't know whether ECDH keys are useful in this context

they are, see:

• https://tools.ietf.org/html/rfc8037#section-3.2
• crypto: add support for x25119 and x448 key pair generation #26774

[panva]

Not instantiating i don't think i can quite pull off myself, returning unsupported i can open a fix for.

[sam-github]

or throwing an error?

[panva]

Throwing an error on a getter of an existing KeyObject instance sounds rather awkward to me.

[sam-github]

I didn't read carefully enough, ignore that.

'unknown' as a default sounds reasonable.

Is there any reason we shouldn't also add all the currently defined values in evp.h to the switch?

[tniessen]

@sam-github That sounds reasonable. (A couple of days ago, I desperately tried to get node to accept an EVP_PKEY_RSA2 key without success...)