Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenSSL v3.2 in Node.js v18 and/or v20 #51152

Closed
Ethan-Arrowood opened this issue Dec 13, 2023 · 18 comments
Closed

OpenSSL v3.2 in Node.js v18 and/or v20 #51152

Ethan-Arrowood opened this issue Dec 13, 2023 · 18 comments
Labels
openssl Issues and PRs related to the OpenSSL dependency. question Issues that look for answers.

Comments

@Ethan-Arrowood
Copy link
Contributor

Hi folks, recently, OpenSSL v3.2 was released: https://www.openssl.org/blog/blog/2023/11/23/OpenSSL32/

I didn't see any existing open issues or PRs discussing this, but I am well aware of this discussion regarding OpenSSL v3.1: #47177

These versions of OpenSSL contain critical performance improvements and I'd appreciate a clearer understanding of when we could expect an upgrade? What is the EOL date math that would enable us to start upgrading?

OpenSSL v3.2 will be supported until 2025-11-23

Node.js v18 is EOL on 2025-04-30 and Node.js v20 is EOL on 2026-04-30.

Does this mean we could upgrade to OpenSSL v3.2 in Node.js v18 ?

If so, can I follow the steps at:

And start the upgrade process? Or would this fall to the responsibilities of the @nodejs/releasers group?

@Ethan-Arrowood Ethan-Arrowood added openssl Issues and PRs related to the OpenSSL dependency. question Issues that look for answers. labels Dec 13, 2023
@richardlau
Copy link
Member

Or would this fall to the responsibilities of the @nodejs/releasers group?

Yes, this falls under the responsibilities of the Release WG.

There's a big complication with OpenSSL in Node.js -- we're actually using a fork from https://github.com/quictls/openssl. We switched to that fork for quic support (which is still being incrementally worked on). AIUI the fork has some issues integrating with OpenSSL 3.2: quictls/quictls#14.

@Ethan-Arrowood
Copy link
Contributor Author

Does OpenSSL v3.2 provide the necessary quic support now? The first line of their blog post states they've added more quic support.

There are major performance improvements made in these new minor versions of OpenSSL. Would be extremely beneficial to get them into Node

@richardlau
Copy link
Member

Does OpenSSL v3.2 provide the necessary quic support now? The first line of their blog post states they've added more quic support.

You'd have to ask @jasnell.

@ranisalt
Copy link

I'm directly interested in this because of #50353. I believe OpenSSL 3.2 is not enough because it only supports client-side QUIC, not server-side (it's planned for 3.3) and the quictls fork hit a wall due to conflicts.

@Ethan-Arrowood
Copy link
Contributor Author

Alright, that's unfortunate news. I wonder if it would be possible for the fork we use to back port some of the performance changes. That is what I'm primarily after here.

@panva
Copy link
Member

panva commented Dec 14, 2023

Personally I am not interested in the fork related features and would prefer to get performance fixes and new features earlier. Is any of the existing quic work actually exposed? I'm asking to understand why the switch to a fork happened before the underlying work was in a usable state. It seems OpenSSL 3.x is picking up steam and the use of a fork which is now blocked on conflicts hampers our ability to ship improvements inherited or dependant on OpenSSL actual.

Just like with 3.1 it's important to enable 3.2 in CI, update whatever needs updating (similar to #47859) so that we can start using 3.2 with custom builds / linked openssl.


FWIW 3.2 has several updates that I'd like to expose sooner rather than later

  • performance fixes and improvements 🤞
  • RFC6979 Deterministic ECDSA signatures
  • Ed25519ph & Ed448ph
  • Ability for setting the Ed448 context
  • Argon2

@targos
Copy link
Member

targos commented Dec 14, 2023

The main blocker for OpenSSL 3.1 and 3.2 is not the fork. It's the fact that they aren't LTS releases.

@mhdawson
Copy link
Member

mhdawson commented Dec 18, 2023

I was just going to say what @targos mentioned. See this article (updated Nov 2023) for an overview of the release strategy - https://www.openssl.org/policies/releasestrat.html

3.0 is supported until 2026-09-7 and is the only LTS that I see in the list. We don't want to use a version and then have to go backwards if the next LTS is not at that level. For example if we choose 3.2 version but the next LTS is 3.1 then we'd be in trouble.

@sapireli
Copy link

@mhdawson @targos honestly 3.0 performance is so bad, i think going with the standard release instead of LTS makes so much sense. Even if 3.2 is deprecated in 2025 its worth the pain to skip this faulty LTS.

@jasnell
Copy link
Member

jasnell commented Dec 19, 2023

As far as I know, 3.2 does not yet have a complete quic impl and appears to have intentionally used API names used by the quic-tls fork. Moving to 3.2 would likely mean I would just abandon all work on quic in node. Not going to block that decision tho.

@sapireli
Copy link

sapireli commented Dec 19, 2023

@jasnell, I appreciate the significant effort you've put into this project. It's evident that they specifically adopted the fork API names to both encourage users to revert to the original version and hinder backports... According to their man-page, the client implementation appears to be complete, with further developments on the way I guess for the server.

I actually think what you suggested is inevitable, at some point in time it will no longer make any sense to backport security fixes, speed enhancements, etc. The only question is whether that should happen now or later. I vote now.

@jasnell
Copy link
Member

jasnell commented Dec 20, 2023

Like I said, I'm not going to block. The quic-tls impl already had client and server support and there was no reason whatsoever for openssl not to just accept that work beyond pure spite and stubbornness. Stepping back to 3.2 with client side only would mean rewriting a non-trivial part of the impl and and taking a step backwards in features.

If the decision is made to go with openssl 3.2, I'll open a PR that pulls all of the quic stuff back out and I'll let someone else in the future pick it back up if they want.

@jasnell
Copy link
Member

jasnell commented Dec 20, 2023

@panva:

Is any of the existing quic work actually exposed? I'm asking to understand why the switch to a fork happened before the underlying work was in a usable state. It seems OpenSSL 3.x is picking up steam and the use of a fork which is now blocked on conflicts hampers our ability to ship improvements inherited or dependant on OpenSSL actual.

It's important to understand the history here a bit.

First, the switch to the fork was to enable the work on quic to actually proceed. QUIC integrates TLS 1.3 into the protocol in a unique way that was previously unsupported by openssl and openssl was refusing at the time to move ahead with changes necessary to support it. In the meantime, BoringSSL worked up a set of APIs that worked. Akamai, however, couldn't make use of BoringSSL due to LTS constraints so they ported the BoringSSL APIs to OpenSSL and opened a PR that sat open for a very long time. Initially, I had started my work on quic based on that PR branch. After a lot of back and forth and handwaving, OpenSSL decided it wasn't going to land the port of the BoringSSL APIs for whatever reason and decided that instead of taking the already fully functional client and server implementation that had been contributed by Akamai, they were going to do their own entirely different implementation spread out over several years in pieces. Why they made that decision is still a mystery to me.

Following that decision, Microsoft and Akamai decided to create the quic+tls fork with the commitment that they would keep it up to date with baseline openssl 3 as much as possible. After discussing the issue with the TSC, it was decided to go ahead and switch to the fork in order to allow the work on quic to proceed. I may be forgetting details but I believe we had full consensus from the TSC to do so.

Use of this fork has been unproblematic until 3.2 arrived. The fork has been tracking the core project as closely as possible, particularly with security fixes. The issue we find ourselves in now is once again because of the decision OpenSSL has made to use the same API names as the BoringSSL APIs used by the quic+tls fork and their decision to Do It Differently and without server side support coming until 3.3.

But, it should be obvious that my time and availability to complete the work on quic is massively reduced as of late and I simply don't have the time or inclination to go through another significant rewrite of a non-trivial portion of the implementation because of some nonsensical decision OpenSSL made so I'm not going to fight it.

That said, the folks behind the quic+tls fork are considering an alternative approach targeting OpenSSL 3.3. See quictls/openssl#124 for details. For now, however, it appears their plan (quictls/quictls#14) is to stick to 3.1 for now and backport performance fixes until their alternative approach can be fully determined. Even this, however, would suggest that we'll end up needing to rewrite the crypto/tls bits of the quic implementation no matter what the end decision is (either rewriting entirely to the "official" openssl APIs whatever they eventually end up being or rewriting to the alternative approach that quic+tls folks are considering).

THAT SAID (lol... forgive me, I'm operating a bit stream of consciousness right now)... our current implementation relies HEAVILY on ngtcp2 (specifically ngtcp2/ngtcp2_crypto.h and ngtcp2/ngtcp2_crypto_openssl.h) which abstracts the details here. We could preserve the existing work if ngtcp2 is updated to abstract away the details, but only if the server-side support is added also. That hasn't happened yet but Tatsuhiro-san has indicated he is willing to do so quictls/quictls#14.

@sapireli :

Even if 3.2 is deprecated in 2025 its worth the pain to skip this faulty LTS.

Unfortunately we can't really make the decision based on performance. The version of OpenSSL that we ship in an LTS version of Node.js needs to have at least as long of an LTS cycle as that Node.js version itself (or close enough) that we can honor our commitment to shipping security fixes when needed. In this case, performance does not trump TLS and security.

@panva
Copy link
Member

panva commented Dec 20, 2023

@jasnell

It's important to understand the history here a bit.

Exactly why I asked. Thank you.

I will reiterate that I agree with 3.0, being the only schedule-valid LTS, being the one bundled. But we should look into 3.2 compatibility soon. I've dynamically linked 3.2 and ran the suite, seems the task will not be as simple as for 3.1

@meyfa
Copy link
Contributor

meyfa commented Dec 20, 2023

Does this mean we could upgrade to OpenSSL v3.2 in Node.js v18 ?

I think it makes no sense to have a more recent OpenSSL version in v18 than in v20 or v21. Firstly, changes are usually applied to the current Node.js version for some time before backporting, which this proposal would deviate from. Secondly, someone using Node.js 18 with OpenSSL 3.2 would be downgraded to a lower OpenSSL version when they update Node.js, which would be quite a surprise for them, I think.

I also think it would be unfortunate to lose all of @jasnell's work on QUIC. Yes, crypto performance is important, but IMHO, having QUIC supported in Node.js would be great as well. It's hardly clear that we should prefer one over the other.

@gurgunday
Copy link
Contributor

That said, the folks behind the quic+tls fork are considering an alternative approach targeting OpenSSL 3.3. See quictls/openssl#124 for details. For now, however, it appears their plan (quictls/quictls#14) is to stick to 3.1 for now and backport performance fixes until their alternative approach can be fully determined. Even this, however, would suggest that we'll end up needing to rewrite the crypto/tls bits of the quic implementation no matter what the end decision is (either rewriting entirely to the "official" openssl APIs whatever they eventually end up being or rewriting to the alternative approach that quic+tls folks are considering).

If there's going to be a big rewrite in any case, the Node.js project can at least make sure it happens once only, which to me suggests that it should be done to move back to OpenSSL

@jasnell
Copy link
Member

jasnell commented Dec 30, 2023

... the Node.js project can at least make sure it happens once only, which to me suggests that it should be done to move back to OpenSSL

The Node.js project does not control any of the pieces that would need to be rewritten here. It would need to be done by the quictls and ngtcp2 projects.

@Ethan-Arrowood
Copy link
Contributor Author

Closing as I believe we have sufficient answer here 😄 Thank you folks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openssl Issues and PRs related to the OpenSSL dependency. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

10 participants