Support --with-openssl-argon2 on Windows

[cmb69]

We change the error for ZTS builds to a warning, to not break snapshot builds which automatically will try to enable OpenSSL password hashing.

We also change some messages to better fit building on Windows.

And of course, we cannot easily check whether OSSL_set_max_threads() is actually available; instead we're looking up the function declaration in its header file.

---

Note that we're still using OpenSSL 3.0 for the master branch, so even if attempted, enabling of OpenSSL argon2 password hashing is not supposed to work (besides that in regular CI we only test ZTS builds). I tested locally with OpenSSL 3.2.2, though, and everything appears to be fine.

While not directly related to this PR, we should consider to update our OpenSSL on Windows. PHP 8.4 is supposed to be supported until 31st Dec 2028, but OpenSSL 3.0 will only be supported until 7th September 2026. This might even be an issue for PHP 8.3. Unfortunately, even OpenSSL 3.3 support ends on 9th April 2026 (thus even earlier than 3.0), but we likely need to update to more recent OpenSSL minor versions. Maybe you can comment about the stability, @bukka.

cc @remicollet since you implemented the feature (thanks!)

PS: OpenSSL 3.2.2 builds for Windows (build against PHP 8.3, but work for PHP 8.4, too) are available at https://github.com/winlibs/winlib-builder/actions/runs/10668521738.

PPS: original feature via #13635.

[bukka]

This should not be enabled on ZTS because calling OSSL_set_max_threads sets global shared value so it might be problematic because other threads could change the value. From the look to the code it uses lock so the actual setting is safe but it's not ideal from the user PoV. It means that this might not be expected for users so we would basically introduce something with a bug.

The solution would be to use custom libctx but that's a bit more work to do: #14734 (we don't need to probably do everything in one go but it's probably too late for 8.4 even for initial setup - I hope to have it in 8.5 though).

[bukka]

In terms of OpenSSL update, it might be worth to wait till 3.4 that should be due in October. Think it should be still fine to bump it during RC considering that we would get a year longer support for that version.

[cmb69]

This should not be enabled on ZTS […]

It is not, see https://github.com/php/php-src/actions/runs/10670305069/job/29574047418?pr=15713#step:5:332.

The test failures are unrelated; I needed to rebase to fix these.

[cmb69]

In terms of OpenSSL update, it might be worth to wait till 3.4 that should be due in October. Think it should be still fine to bump it during RC considering that we would get a year longer support for that version.

I'm fine with having OpenSSL 3.4 for GA, but shouldn't we update to OpenSSL 3.3 as soon as possible?

[petk]

@cmb69 sorry for the off-topic question here. Should Windows also get the --with-openssl-legacy-provider configure option? That was added to PHP-8.4 in Autotools. It simply sets the LOAD_OPENSSL_LEGACY_PROVIDER preprocessor macro and it is available as of OpenSSL 3.0 (although the check is not needed because the version is also checked in the C code).

[cmb69]

Should Windows also get the --with-openssl-legacy-provider configure option?

Yes, I think so. I don't think, though, that we should enable that by default for snapshot builds (so it would need to be added to snapshot_build_exclusions in confutils.js). And it would need to be checked whether that works.

I wonder, though, why this would even need a compile time option; couldn't that be enabled via openssl.cnf [provider_sect]?

[bukka]

I don't think that option is really useful for Windows. It's more convenience for autotools builds where custom builds are more usual.

[bukka]

It is not, see https://github.com/php/php-src/actions/runs/10670305069/job/29574047418?pr=15713#step:5:332.

Ah ok I see, it's just ignored. I didn't check the PR before properly and thought that it just prints warning and enable it.

[petk]

I don't think that option is really useful for Windows. It's more convenience for autotools builds where custom builds are more usual.

But, it does work, right? Because then in CMake I would have to filter Windows target out specifically.

[remicollet]

@cmb69 thanks for taking care of Windows part of this feature

[petk]

--- a/UPGRADING.INTERNALS
+++ b/UPGRADING.INTERNALS
@@ -117,6 +117,8 @@ PHP 8.4 INTERNALS UPGRADE NOTES
    - The configure option --with-pspell has been removed.
+   - New configure option --with-openssl-argon2 to enable PASSWORD_ARGON2
+     from OpenSSL 3.2
    - Symbol SIZEOF_SHORT removed (size of 2 on 32-bit and 64-bit platforms).
@@ -135,8 +137,6 @@ PHP 8.4 INTERNALS UPGRADE NOTES
      --with-ftp-ssl and --with-mysqlnd-ssl.
    - New configure option --with-openssl-legacy-provider to enable OpenSSL
      legacy provider.
-   - New configure option --with-openssl-argon2 to enable PASSWORD_ARGON2
-     from OpenSSL 3.2
    - COOKIE_IO_FUNCTIONS_T symbol has been removed (use cookie_io_functions_t).

Perhaps the UPGRADING.INTERNALS update can also go in this PR. Thanks @cmb69

[cmb69]

I don't think that option is really useful for Windows. It's more convenience for autotools builds where custom builds are more usual.

I assume you are referring to --with-openssl-legacy-provider. In this case I agree that it's not particularly useful for Windows, but wouldn't hurt to add it (if disabled for snapshot builds).

But, it does work, right?

Yep, should work. Tested quickly with set CFLAGS=/DLOAD_OPENSSL_LEGACY_PROVIDER=1 and verified with openssl_get_cipher_methods().

Perhaps the UPGRADING.INTERNALS update can also go in this PR.

Isn't this info about new configuration options supposed to go into UPGRADING? After all, it's not really for developers, but rather for users, and I think the info is also relevant for the PHP manual, but the doc team is unlikely to look into UPGRADING.INTERNALS. Maybe @Girgias can clarify?

[petk]

Isn't this info about new configuration options supposed to go into UPGRADING?

New configure options are more relevant to those who build PHP from scratch than those who use certain functionality in their PHP code. If the reader is the same person, than both places are probably good to add. To adjust some build script, a single collection of the configure options is simpler to browse.

[cmb69]

@petk, I've written to the internals list for clarification.

[petk]

@petk, I've written to the internals list for clarification.

I've sent reply there also but I get constant issues with undelivered messages there. I'll recheck later if I have more time to adjust my mail server.

Edit: Now it seems to send it.

[cmb69]

Rebased and force-pushed to resolve merge conflicts.

[Girgias]

I don't think that option is really useful for Windows. It's more convenience for autotools builds where custom builds are more usual.

I assume you are referring to --with-openssl-legacy-provider. In this case I agree that it's not particularly useful for Windows, but wouldn't hurt to add it (if disabled for snapshot builds).

But, it does work, right?

Yep, should work. Tested quickly with set CFLAGS=/DLOAD_OPENSSL_LEGACY_PROVIDER=1 and verified with openssl_get_cipher_methods().

Perhaps the UPGRADING.INTERNALS update can also go in this PR.

Isn't this info about new configuration options supposed to go into UPGRADING? After all, it's not really for developers, but rather for users, and I think the info is also relevant for the PHP manual, but the doc team is unlikely to look into UPGRADING.INTERNALS. Maybe @Girgias can clarify?

New build configurations should also be in upgrading I think, as I never would check UPGRADING.INTERNALS when writing docs.

[petk]

Here two new functions were also added: openssl_password_hash() and openssl_password_verify(). These are more relevant in the UPGRADING file.

[cmb69]

Here two new functions were also added: openssl_password_hash() and openssl_password_verify(). These are more relevant in the UPGRADING file.

Right. However, these will not generally be added by this patch, but rather have been added by #13635.