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

Fix: don't hardcode alpn protocol byte size (OpenSSL) #14769

Merged

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Jul 1, 2024

For some reason OpenSSL used to negotiate the protocol by itself, without invoking the select callback, or maybe didn't respect the total bytesize when processing the alpn string.

That changed in the 3.0.14 and other bugfix releases of OpenSSL, which exposed the bug.

EDIT: the best proof is the fixed OpenSSL 3.0 test suite on CI ✔️

For some reason OpenSSL used to negotiate the protocol by itself,
without invoking the select callback, or maybe didn't respect the
total bytesize when processing the alpn string.

That changed in the 3.0.14 and other bugfix releases of OpenSSL, which
exposed the bug.
Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

Wow, that was fast! 🚀

@beta-ziliani beta-ziliani added this to the 1.13.0 milestone Jul 1, 2024
@ysbaddaden ysbaddaden merged commit 5b50088 into crystal-lang:master Jul 1, 2024
61 checks passed
@ysbaddaden ysbaddaden deleted the fix/openssl-alpn-negotiation branch July 1, 2024 15:31
@ysbaddaden
Copy link
Contributor Author

@beta-ziliani I'm just sweeping after my bugs:
20d59b6#diff-9ac7faab010739a786c76dbf215016472ad708410b39271b65437c616314a7d0R123

It only took 8 years to notice.

straight-shoota pushed a commit that referenced this pull request Jul 2, 2024
For some reason OpenSSL used to negotiate the protocol by itself,
without invoking the select callback, or maybe it didn't respect the
total bytesize when processing the alpn string.

That changed in the 3.0.14 and other bugfix releases of OpenSSL, which
exposed the bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants