crypto: remove experimental Curve448 WebCryptoAPI algorithms

[panva]

These experimental Issues and PRs related to experimental features. algoritms come from https://wicg.github.io/webcrypto-secure-curves/ and Node.js is the only server runtime to implement them. There's no indication that that would change in the future and browser support for them is very limited with no intents to ship or otherwise tracked standards positions. As a result only Curve25519 is being added to the actual WebCryptoAPI Editor's Draft and so we can clean them up from our implementation.

This is not technically semver-major PRs that contain breaking changes and should be released in the next major version. but we might as well treat it as such for v23.0 (or an early v23.x) to have a clear cut.

[nodejs-github-bot]

Review requested:

• @nodejs/crypto
• @nodejs/web-standards

[avivkeller]

This PR removes a feature, so I've added needs-citgm PRs that need a CITGM CI run. just in case it causes breakages, feel free to adjust

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.39%. Comparing base (89a2f56) to head (2731ae6).
Report is 216 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55209      +/-   ##
==========================================
- Coverage   88.39%   88.39%   -0.01%     
==========================================
  Files         652      652              
  Lines      186565   186472      -93     
  Branches    36046    36008      -38     
==========================================
- Hits       164916   164823      -93     
- Misses      14908    14912       +4     
+ Partials     6741     6737       -4     

Files with missing lines	Coverage Δ
lib/internal/crypto/cfrg.js	96.56% <ø> (-0.38%)	⬇️
lib/internal/crypto/diffiehellman.js	97.52% <100.00%> (-0.01%)	⬇️
lib/internal/crypto/util.js	92.97% <ø> (-0.14%)	⬇️
lib/internal/crypto/webcrypto.js	94.59% <ø> (-0.15%)	⬇️
lib/internal/crypto/webidl.js	98.27% <ø> (-0.03%)	⬇️

... and 32 files with indirect coverage changes

[Slayer95]

In the thread from the issue you linked, Flow reported half a year ago

Flow 6.17 supports Ed25519, X25519, Ed448 and X448.

Yet WICG's summarizing table (which happens to be by this same PR's author) hasn't been updated to reflect that, which is why this issue claims that there are no other implementors.

Also note that Curve25519's PR into W3C repo predates Flow's reported implementation. Also note that said PR has not been merged yet.

So, while these features are experimental, it doesn't look to me like evidence is solid for their removal in less than two years.

[panva]

Yet WICG's summarizing table (which happens to be by this same PR's author) hasn't been updated to reflect that, which is why this issue claims that there are no other implementors.

That's an omission on my part. Apologies and updated.

Also note that Curve25519's PR into W3C repo predates Flow's reported implementation. Also note that said PR has not been merged yet.

That doesn't seem to be relevant. Curve25519 algorithms are going to be merged and there are only a couple issues remaining which @twiss is driving home with the other vendors.

So, while these features are experimental, it doesn't look to me like evidence is solid for their removal in less than two years.

Neither one of the big 3 browser distributions people tend to focus on have an intent to ship nor even tracking a position for Curve448 implementation. Likewise, Node.js is the only non-browser runtime with support. The point of WebCryptoAPIs existance in Node.js is one of interoperability so I see no point in keeping these algorithms around if developers cannot rely upon them being available in other runtimes their code may be moved to.

[twiss]

Hi 👋 I know the point of tagging me wasn't to ask my opinion but I'll volunteer one anyway, if you don't mind :)

The reason I only included Curve25519 in w3c/webcrypto#362 was because I thought we could merge it sooner than Curve448 (for the reasons stated there), not because I wanted to give up on standardizing Curve448 altogether. https://github.com/WICG/webcrypto-secure-curves/ will still continue to exist after that PR is merged, and will still specify Curve448, so if that's good enough for Node.js to implement it (as an experimental algorithm) now, that should continue to be the case, IMHO.

I think it would be more ideal if the outcome of merging w3c/webcrypto#362 would be to mark Curve25519 as non-experimental, but keep Curve448 (as experimental), rather than removing Curve448.

---

Btw, note that Curve448 is already specified in TLS 1.3 as well, so you might want to keep an implementation of it anyway - even though it's not widely implemented yet there either, but hopefully that will change in the future as well.

For Firefox, their position on Curve448 in general is:

We'd review an implementation for this. We'd prefer a verified implementation from the HACL* project, which I believe is targeting this for use as a ciphersuite for TLS. Marking P5, would accept patches.

(...)

I think for now we don't have plans to support Curve448, but we might in future.

So they don't have active plans right now but are not opposed and would accept patches, and it seems the main blocker is support in the cryptography library they use.

And if Curve448 does eventually get implemented for TLS (as the spec prescribes), I think it would make sense to expose it in Web Crypto as well.

[panva]

Hi 👋 I know the point of tagging me wasn't to ask my opinion but I'll volunteer one anyway, if you don't mind :)

Of course not :)

I think it would be more ideal if the outcome of merging w3c/webcrypto#362 would be to mark Curve25519 as non-experimental, but keep Curve448 (as experimental), rather than removing Curve448.

As soon as the PR is merged and chrome unflags their implementation of it I'll mark the two Curve25519 algorithms stable in Node.js. I've seen standards be DOA because of chrome not shipping an experimental implementation of theirs on a whim before (S·T·T·L Token Binding)

I'd like to hear from @tniessen and @jasnell or other @nodejs/web-standards members before retracting this PR. What are your thoughts on shipping support for algorithms that cannot (and will not in the foreseeable future) run on other implementations of WebCryptoAPI?

[tniessen]

I don't have a strong opinion on this. I very much appreciate the standardization effort by @twiss and would like to see this implemented consistently across runtimes, but as long as Node.js supports X448/Ed448 through its own APIs, I suppose that's alright.

[panva]

As soon as the PR is merged and chrome unflags their implementation of it I'll mark the two Curve25519 algorithms stable in Node.js

See #56142