webcrypto: graduate from experimental, expose via crypto

[jasnell]

In the browser, the crypto global exposes subtle and
getRandomValues as properties. In the Node.js impl, those
require an additional indirection via crypto.webcrypto.*.
This commit exposes aliases to promote cross-environment
isomorphism.

Note: In the browser, the CryptoKey object is also exposed
as a global. I decided not to do that here because it requires
a larger change to the internal node_crypto binding because
of the need to register external references. That can be done
in a separate PR but it's a much lower priority.

This also graduates WebCrypto from experimental status.

Signed-off-by: James M Snell [email protected]

[targos]

crypto is not a global, as the added test shows.

[targos]

This also graduates WebCrypto from experimental status.

I'd be much more comfortable approving this part of the change if we had the WebCryptoAPI WPT in the tree.

[jasnell]

crypto is not a global, as the added test shows.

Sorry, I was speaking of it as a global in terms of the repl but wasn't clear. Because it is a global in the repl, making globalThis.crypto == crypto.webcrypto would be problematic since crypto would refer to two entirely different things depending on how you ran Node.js.

[panva]

I've done interoperability testing on everything defined by Web Cryptography API, but I wouldn't rush calling the Non-standard Node.js extensions as stable just yet, as evidenced by the fact that every time I attempted to use one of them I found a thing to fix. Is there a way for us to call the standard defined algorithms stable but keep the node specific extensions experimental?

We should also take this moment to consider whether we really want to keep those extensions around. This might be the last chance for us to ditch or trim those. Not saying we should, but this is the time to reflect.

This commit exposes aliases to promote cross-environment isomorphism.

I'm also not sure this PR achieves such a thing. For one it's not exposed via globalThis, so one has to write runtime specific code to import crypto anyway. [1], [2]

[targos]

Sorry, I was speaking of it as a global in terms of the repl but wasn't clear. Because it is a global in the repl, making globalThis.crypto == crypto.webcrypto would be problematic since crypto would refer to two entirely different things depending on how you ran Node.js.

I understand. We need to think about this, because one solution could be to make globalThis.crypto a reference to the entire crypto module (along with the change in this PR) (Not saying we should do that).
In the current state, it can also be problematic in the context of the REPL, because modules that check for the presence of globalThis.crypto.subtle will behave differently than in a usual Node.js context.

[jasnell]

@panva:

Is there a way for us to call the standard defined algorithms stable but keep the node specific extensions experimental?

Yes, absolutely.

We should also take this moment to consider whether we really want to keep those extensions around. This might be the last chance for us to ditch or trim those. Not saying we should, but this is the time to reflect.

I believe there's value in keeping them, but I'm a bit biased since I added them ;-) ... we can absolutely keep those extensions experimental for some time.

@targos:

We need to think about this, because one solution could be to make globalThis.crypto a reference to the entire crypto module (along with the change in this PR) (Not saying we should do that).

I could go either way with it. I'm fine with making it a global, but I'm also fine with keeping it behind the require('crypto') and import ... from 'crypto'

[tniessen]

could be to make globalThis.crypto a reference to the entire crypto module (along with the change in this PR) (Not saying we should do that)

This might lead to problems when Node.js is compiled without OpenSSL and disables lazy loading.

[panva]

#38029 has to land first

[panva]

I believe there's value in keeping them

I'm torn on this one, aside from the Curve25519/448 raw key import which we could just add to crypto.createPublicKey and crypto.createPrivateKey respectively I am lacking the signal that resulted in them being added.

Since these are not portable to other Web Cryptography API implementations if it came down to a decision I'd say remove the Node.js-specific extensions.

I could go either way with it. I'm fine with making it a global, but I'm also fine with keeping it behind the require('crypto') and import ... from 'crypto'

Adding it to global would conflict with crypto in REPL and adding to actual require(crypto) does not help with universal libs. So I'd stick with the current scheme.