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

Add OpenSSL 3 support #31

Merged
merged 5 commits into from
Jun 7, 2024
Merged

Add OpenSSL 3 support #31

merged 5 commits into from
Jun 7, 2024

Conversation

bartbes
Copy link
Member

@bartbes bartbes commented Apr 13, 2024

Fixes #21.
Based on the first commit of #30.

The only actual breakage was the rename of SSL_get_peer_certificate to SSL_get1_peer_certificate.

I had spotted a new HTTP client in OpenSSL3, and tried to get that to work.
Unfortunately, it's too limited for our purposes: it only supports GET and POST, and has no way to retrieve the status code or response headers.

@bartbes
Copy link
Member Author

bartbes commented Apr 13, 2024

Now if only I'd based it on the actual branch, I would've had the fix for that compile error too. I'm marking the PR as draft for now, let me know if you want it rebased on main, or on #30 (or main after that's merged).

@bartbes bartbes marked this pull request as draft April 13, 2024 12:57
@MikuAuahDark
Copy link
Contributor

I'll wait until #30 is safe to merge before merging this one. Thanks.

@MikuAuahDark
Copy link
Contributor

Feel free to rebase then open the PR.

bartbes added 5 commits April 21, 2024 13:48
I checked, but as far back as the docs for 1.1, the docs say you should call free.
As indicated by SSL_CTRL_OPTIONS no longer being defined, we should avoid it as much as possible.
So now the source just contains an in-order list, which is a lot more legible.
@bartbes bartbes marked this pull request as ready for review April 21, 2024 11:53
@bartbes
Copy link
Member Author

bartbes commented Apr 21, 2024

I did notice the CI job only builds and tests with openssl 1.1, because it uses ubuntu 20.04.

@slime73
Copy link
Member

slime73 commented May 5, 2024

I'm a little unsure about bumping CI to 22.04 because love 11's appimage is compiled with 20.04. Anyone have opinions?

@slime73
Copy link
Member

slime73 commented May 5, 2024

Maybe we could run two linux jobs (20.04 versus 22.04) and label their artifacts appropriately. That might not be useful for very long though.

@MikuAuahDark
Copy link
Contributor

Since we're only runtime loading OpenSSL3, I wonder if it's good idea to clone OpenSSL source code locally then use the include for it? This would require more modification to lua-https CMake and we'll eventually need Ubuntu 22.04 for testing it anyway.

@MikuAuahDark
Copy link
Contributor

I'll merge this for now. Figuring out where to source the OpenSSL header can be done later (or by simply updating lua-https runners).

@MikuAuahDark MikuAuahDark merged commit d0de6e2 into love2d:main Jun 7, 2024
6 checks passed
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

Successfully merging this pull request may close these issues.

Support OpenSSL 3
3 participants