Skip to content

Reference the OpenSSL ECH feature branch rather than the defo-project fork #17251

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

Closed
wants to merge 1 commit into from

Conversation

sftcd
Copy link
Contributor

@sftcd sftcd commented May 5, 2025

This PR just changes the recommended OpenSSL git repository to use when building the experimental ECH support. We used to reference the defo-project fork, but (together with OpenSSL maintainers) we have now upstreamed enough ECH code into the OpenSSL project;s ECH feature branch that that can now be used for ECH-enabled curl builds.

The OpenSSL project's feature branch seems more "official" so is probably a better thing to reference for curl documentation. Note though that ECH is not yet part of an OpenSSL release, so it's still not very official, but we are working on that.

The only change here is to the docs/ECH.md build instructions.

I'm not sure if this PR is suited for merging during the feature freeze or not. Either's fine. If there's a preference to not merge this and keep referencing the defo-project fork until ECH is part of an OpenSSL release, that's also fine.

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine to merge even in feature freeze. In particular since this feature still is marked experimental.

@sftcd
Copy link
Contributor Author

sftcd commented May 5, 2025

Oh, maybe hold merging this a bit, I forgot that this version of OpenSSL now includes emitting two key shares in the ClientHello (one x25519 as before and one PQ-hybrid) so I should run the curl tests properly to make sure that doesn't break something. Doing that now... (or in the morning here)

@sftcd
Copy link
Contributor Author

sftcd commented May 5, 2025

Actually it seems ok:

....
test 4000...OK (1668 out of 1669, remaining: 00:00, took 1.501s, duration: 24:47)
test 4001...OK (1669 out of 1669, remaining: 00:00, took 1.463s, duration: 24:48)
TESTDONE: 1810 tests were considered during 1489 seconds.
TESTDONE: 1613 tests out of 1613 reported OK: 100%
make[1]: Leaving directory '/home/stephen/code/cpr/curl/tests'

@bagder bagder closed this in de881a9 May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants