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

Sorting out our plans for 3.2 #138

Open
wbl opened this issue Sep 14, 2023 · 35 comments
Open

Sorting out our plans for 3.2 #138

wbl opened this issue Sep 14, 2023 · 35 comments

Comments

@wbl
Copy link
Collaborator

wbl commented Sep 14, 2023

Unfortunately many of the function names we've chosen to use are going to get repurposed by the OpenSSL quic support. That's very unfortunate because 3.2 supposedly solves many of the locking issues and boosts performance (there are other longstanding performance issues to solve).

I see some pretty unattractive options ahead:
1: Backport much of this work onto 3.1
2: Add patches removing upstream QUIC support first
3: Stay on 3.1 or 3.0 and ignore the other changes.

@wbl
Copy link
Collaborator Author

wbl commented Sep 18, 2023

@tmshort curious to know your thoughts on this problem

@nibanks
Copy link
Member

nibanks commented Sep 27, 2023

many of the function names we've chosen

Should we simply do a copy/replace of all names used for this project to add some special prefix to uniquely identify them, as well as prevent further conflicts in the future?

@wbl
Copy link
Collaborator Author

wbl commented Sep 27, 2023

I think we should do that for the internal ones, I don't think we'll need to change the public API, but I'll have to look again to see what we did squat on.

If we do change the public API that would be a real pain.

@wbl
Copy link
Collaborator Author

wbl commented Sep 27, 2023

SSL_is_quic is probably the worst one.

@nibanks
Copy link
Member

nibanks commented Sep 27, 2023

If we're going to have to make changes, let's rip off the band aid ASAP. We can change it to SSL_is_quictls.

@wbl
Copy link
Collaborator Author

wbl commented Sep 28, 2023

Or maybe prefix it also with the same one? OpenSSL doesn't seem receptive to our complaints (unsurprisingly).

This turns out to be less of a problem than the new SSL_CONNECTION abstraction. The hack job I'm doing now won't be anything I can bring back, but after I do the hack job I'll have a better idea of what to do here.

@wbl
Copy link
Collaborator Author

wbl commented Sep 28, 2023

OPENSSL_NO_QUIC is another conflict, but easier to change and if we do we can use the suggestion to combine the SSL_is_quic functions

@wbl
Copy link
Collaborator Author

wbl commented Oct 26, 2023

Note to projects/packagers that depend on this: this is going to be an issue with 3.3 and later. Weigh in please: we know you exist but we don't hear from you!

@tatsuhiro-t
Copy link

If we need to change API for 3.2 or later, then I can deal with it by ifdef assuming that the change only happens for 3.2 or later.

@talregev
Copy link

talregev commented Oct 30, 2023

Can you make quic tls lib on top of open ssl 3.2 and create the same functions but different name space. Also change the lib names that it will be alongside openssl?

@ranisalt
Copy link

ranisalt commented Nov 7, 2023

@wbl not a maintainer but Node.js uses this fork and I have a pull request nodejs/node#50353 that requires OpenSSL 3.2 and this is currently blocked because OpenSSL included in the repo cannot be updated.

@talregev
Copy link

talregev commented Nov 7, 2023

@wbl not a maintainer but Node.js uses this fork and I have a pull request nodejs/node#50353 that requires OpenSSL 3.2 and this is currently blocked because OpenSSL included in the repo cannot be updated.

openssl 3.2 not yet release officially. this issue is a thoughts what can done when the openssl 3.2 will be out.
They plan to update it, but there is different way to do it. Any preferable way for you?

@ranisalt
Copy link

ranisalt commented Nov 7, 2023

I know it's not released yet, I should have clarified that the linked PR will require OpenSSL 3.2 when it's out and I'm working with the beta version.

Unfortunately, I don't know how Node.js uses QUIC so I can't really provide an answer as to how the naming issues could be fixed. I believe that staying on a previous version is definitely not the solution, though. So 2 for me if the only options are 1 to 3 in OP.

@tmshort
Copy link
Member

tmshort commented Nov 15, 2023

Apologies for late response.
I did not have plans to release a 3.2 version of QuicTLS, however, I was going to look at an alternative approach.
See: #124

@tmshort
Copy link
Member

tmshort commented Nov 15, 2023

Meanwhile, I think the best approach is to stay at 3.1 and backport various performance fixes, until #124 can be evaluated.

@baentsch
Copy link

the best approach is to stay at 3.1 and backport various performance fixes

A comment from the "sidelines" (FWIW and FYI only): The above would mean we'll be stopped from adding full PQ crypto support to quictls: We had done that with an old code base but now changed the integration over to an OpenSSL provider. PQ KEMs added this way work since OpenSSL 3.0, so do with quictls. However, full PQ signature support only is available since OpenSSL 3.2 (it is an official release by now). From this perspective it would be really unfortunate if quictls cannot follow the README guidance:

We will endeavor to track OpenSSL releases within a day or so

@Firegarden
Copy link

Firegarden commented Jan 20, 2024

How is everyone doing? I am just a lowly end user wondering what it will take to compile nginx with openssl 3.2 with http3 support. As Johnny Cash would say "time keeps draggin' on" so just curious if we can get a rhythm because we got the open ssl 3.2 quic support blues...

@baentsch
Copy link

what it will take to compile nginx with openssl 3.2 with http3 support

This fork seems abandoned to me (last commit 6 months ago) -- for IMO understandable reasons. We de-listed our PQC quictls integration.

@tavrez
Copy link

tavrez commented Jan 30, 2024

OpenSSL 3.2.1 just released with a flag to disable quic(no-apps), will it helps?

Edit: I've checked the commit and it's nothing but disabling quicserver app, no encapsulation with ifdef or something to hide the whole quic part.

@wbl
Copy link
Collaborator Author

wbl commented Jan 30, 2024

It depends on how far it goes. The restructuring that creates a lot of the issues would not be affected by the flag. I've not had time to approach 3.2 the right way: it will be quite a bit of work, but we are currently planning to do it. Note that we are keeping up with the 3.0 release series.

@tmshort
Copy link
Member

tmshort commented Jan 30, 2024

@baentsch we follow upstream OpenSSL fairly closely (or at least, try to), they just released.

@baentsch
Copy link

@baentsch we follow upstream OpenSSL fairly closely (or at least, try to), they just released.

ACK -- "upstream OpenSSL" is somehow a continuum, though... fwiw, 3.2 was released in November.

So would it be fair to amend your statement with the version moniker "3.0.x" as otherwise the statement seems to contradict the comment above

not had time to approach 3.2

?

The issue with 3.0.x is that it doesn't include a few logic updates to fully enable the OpenSSL provider concept that are resolved (only) in 3.2.x and which thus fully enable PQC. So any upgrade to 3.2.x would help the PQC integration quite a bit, but I do understand there's quite some issues...

@Firegarden
Copy link

After thorough discussion and consideration of the challenges posed by OpenSSL 3.2's release, the following steps outline the best path forward for the quictls/openssl project:

  1. Namespace Changes and Public API: Begin with renaming internal functions using a unique prefix to prevent conflicts with OpenSSL 3.2. Carefully evaluate and minimally adjust the public API to ensure backward compatibility and minimize disruptions for users, with specific attention to functions like SSL_is_quic which may be renamed to SSL_is_quictls.

  2. Versioning and Compatibility: Focus on maintaining compatibility with OpenSSL up to 3.1 while integrating essential performance improvements from later versions where feasible. This interim strategy ensures stability as we develop a more comprehensive approach to embrace OpenSSL 3.2.

  3. Long-Term Strategy and Community Engagement: Initiate groundwork for full OpenSSL 3.2 compatibility, recognizing the significant undertaking to adapt to structural changes. Concurrently, engage with the community and stakeholders to understand impacts, gather consensus, and ensure that the quictls project continues to meet the needs of its users while aligning with the broader OpenSSL ecosystem's evolution.

This plan balances immediate needs with strategic long-term goals, ensuring the quictls project remains a valuable and cohesive

@Firegarden
Copy link

My last comment was generated by open ai. Does anyone developer who is coding on quictls need access to gpt 4 turbo? I am no C programmer but looking at the concerns it seems like the only challenges are planning the namespaces and compatibility - ai can help!

Also you have some very logical suggestions such as SSL_is_quic, require renaming (e.g., to SSL_is_quictls) to circumvent direct conflicts with OpenSSL 3.2.

Personally I use this library on my nginx build and you could say it's extremely valuable in terms of giving the common man access to compete with the best. Build the latest nginx with http3 and quictls and you are on the leading edge. Lets keep riding the wave!

@tmshort
Copy link
Member

tmshort commented Mar 19, 2024

We are aware of aware of the general conflicts for QuicTLS and 3.2; we only have limited capacity to support all the branches that OpenSSL supports. We need a migration strategy that would allow transition from 3.0/3.1 to 3.2.

@wbl
Copy link
Collaborator Author

wbl commented Apr 23, 2024

I'm going to start attacking the rebase this week.

@ranisalt
Copy link

How about going straight to 3.3? It came so fast 😆

@wbl
Copy link
Collaborator Author

wbl commented Apr 30, 2024

I've currently got a branch that seems to pass tests on top of 3.3 when our feature is enable and the openssl one is not. It's not impossible that I've screwed something up still, so some more testing is required, and currently it collapses a bunch of history we'd prefer to preserve for ease of fixing up the initial rebase so there is still some work to do.

@nibanks
Copy link
Member

nibanks commented Apr 30, 2024

If you have a branch here I can point MsQuic to, I can try to run our tests too.

@tmshort
Copy link
Member

tmshort commented May 1, 2024

I haven't had the time to do this (but I had started), thanks @wbl.

@bneradt
Copy link

bneradt commented May 15, 2024

Just to clarify: I notice there is a openssl-3.3.0+quic branch. If our organization can move to that and skip 3.2.0, is that 3.3.0 branch feature complete with the quic API?

@wbl
Copy link
Collaborator Author

wbl commented May 15, 2024

We have a PR open to add QUIC to that branch. That PR has not been merged yet #159 and needs a little work to see what's wrong with the fuzzer build.

@bneradt
Copy link

bneradt commented Jun 7, 2024

We have a PR open to add QUIC to that branch. That PR has not been merged yet #159 and needs a little work to see what's wrong with the fuzzer build.

Hi. Just checking in for an update with this. Has the fuzzer issues been worked through?

Thank you for the work on this!

@bneradt
Copy link

bneradt commented Jul 8, 2024

We have a PR open to add QUIC to that branch. That PR has not been merged yet #159 and needs a little work to see what's wrong with the fuzzer build.

Hi. Just checking in for an update with this. Has the fuzzer issues been worked through?

Thank you for the work on this!

The PR #159, from just looking at the comments, looks close to being merged. Does that sound right, @wbl ? Thank you again for working on this.

@wbl
Copy link
Collaborator Author

wbl commented Jul 9, 2024

I've got to respond to those comments, and I think one or two people might be interested in reviewing and taking a look to make sure I didn't screw up too badly, but I'd say close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants