Skip to content

Introduce openssl backend for v1 and v3 API separation #16918

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

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

bukka
Copy link
Member

@bukka bukka commented Nov 24, 2024

The main purpose of this is to better handle the API difference and add an inital work to separate PHP and OpenSSL logic. This is really just the first step and further changes are coming after that.

The motivation for this is #14734 which would basically make the current code messy and introduce many ifdefs. The thing is that the v1 API will need to be maintained for some time as 1.1.1 is still used as default in RHEL 8 which has EOF in 2029. In addition there is LibreSSL that might become even more incompatible so even once we no longer need to support 1.1.1, we might still want to support LibreSSL and it's not clear if it's going to ever get providers.

bukka added a commit to bukka/php-src that referenced this pull request Nov 24, 2024
The main purpose of this is to better handle the API difference and add
an inital work to separate PHP and OpenSSL logic. This is really just
the first step and further changes are coming after that.

Closes phpGH-16918
@bukka bukka force-pushed the openssl_backend_separation branch 2 times, most recently from 9071632 to 27aaedf Compare November 24, 2024 15:54
bukka added a commit to bukka/php-src that referenced this pull request Nov 24, 2024
The main purpose of this is to better handle the API difference and add
an inital work to separate PHP and OpenSSL logic. This is really just
the first step and further changes are coming after that.

Closes phpGH-16918
@cmb69
Copy link
Member

cmb69 commented Nov 25, 2024

1.1.1 is still used as default in RHEL 8 which has EOF in 2029

shakes head

But okay, I guess we will have to deal with this, one way or another. Separating the backends makes sense to me, especially since openssl.c is already too big for my taste, but I'm afraid that this will cause big merge-troubles for years, and we might even miss to apply relevant fixes to all back-ends (cf. dns.c vs. dns_win32.c). That might especially be worrysome if we don't have a CI job running against OpenSSL 1.1.

@bukka
Copy link
Member Author

bukka commented Nov 25, 2024

Yeah we should at least have nightly job to run OpenSSL 1.1.1 and maybe LibreSSL might be good idea too. It might be actually used to also test older versions of other libraries or alternative implementations (e.g. MariaDB for mysqli).

In terms of merge troubles, I can't think about any other way but it's probably better than automerge to just a single back-end because it might give at least a hint that there might not be just one backend to change. Also there's not such a huge number of updates going to openssl.c in terms of bug fixes so might not be such a huge issue.

@bukka
Copy link
Member Author

bukka commented Dec 9, 2024

Just a heads, that if there are no objections I will merge this later this week.

As the next step I'm looking to cleaning up the 1.0.2 specific bits that are no longer supported and we kept them just for LibreSSL. But looking at it, this applies only to very old LibreSSL versions anyway (before 2.7 from what I read). I'm thinking about setting minimum to 3.0.2 but I don't have a good visibility into where it's actually used. I know it's used in OpenBSD which seems to have last stable supported versions 7.5 which uses 3.9.0 so maybe we can go higher than 3.0.2 with minimal version. @devnexen What do you think?

@devnexen
Copy link
Member

devnexen commented Dec 9, 2024

OpenBSD uses it as its default TLS library. FreeBSD allows you to set the TLS of your choice, not sure how much users actually set to libressl, but that is a possibility. There is libressl for openbsd and libressl for "the rest" (using wrappers to map openbsd native calls) but they should be the same api and behavior-wise. Using 3.0.2 as minimum is very reasonable IMHO.

The main purpose of this is to better handle the API difference and add
an inital work to separate PHP and OpenSSL logic. This is really just
the first step and further changes are coming after that.

Closes phpGH-16918
@bukka bukka force-pushed the openssl_backend_separation branch from 27aaedf to 59b862a Compare February 6, 2025 12:57
@bukka bukka merged commit d662ab5 into php:master Feb 6, 2025
8 of 9 checks passed
@bukka
Copy link
Member Author

bukka commented Mar 12, 2025

So I have been doing some testing with LibreSSL. It took me a bit to realise that version from git does not exactly work on Linux (seems OpenBSD specific) but the tarballs work so I have been checking the versions. And currently the oldest version that compiles from 3.x is 3.5.0 due to missing EVP_CTRL_AEAD_GET_TAG in previous versions. So the minimum supported is 3.5.0 and allowed some clean up as in #18032 .

Btw there are bunch of tests failures because it stripped some features and we don't do any checks for that. So it's more a support just to make it compile atm. It might improve in the future but it's not a priority.

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.

3 participants