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: Resolve build failure due to using OpenSSL@3 instead of OpenSSL@1.1 on macOS for PHP < 8.1 #1359

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

driskell
Copy link
Contributor

@driskell driskell commented Dec 7, 2023

When building PHP < 8.1 on macOS and you have openssl@3 installed via Homebrew, you can get an error about SSL padding because it tries to link to openssl@3 and it's not compatible as it removes old SSL constants. You need openssl@1.1. I didn't want to uninstall openssl@3 though (it breaks things and starts removing other recipes!).

This detects the scenario of openssl@3 which we know simply won't work, and checks for openssl@1.1.

Similarly to the other PR I did I wasn't sure how to approach this as there's no real environment checking. Perhaps if there's environment checking for openssl@1.1 before continuing then this check here wouldn't be needed and you can just change the brew prefix to @1.1.

Issue with doing that now is that if you change it to @1.1 - it won't fail - because the PkgConfig kicks in and locates openssl@3 from Homebrew again 😆 - so you have to ensure that openssl@1.1 is installed. Hopefully this helps users anyway.

@peter279k
Copy link
Member

@driskell, thanks for your PR.

If possible, could you provide your macOS environemt that you run this change? (e.g. Mac Mini M1)

And I can reproduce the issue and verify the PR is correct easily :).

@driskell
Copy link
Contributor Author

driskell commented Dec 7, 2023

@driskell, thanks for your PR.

If possible, could you provide your macOS environemt that you run this change? (e.g. Mac Mini M1)

And I can reproduce the issue and verify the PR is correct easily :).

Apple MacBook Pro M3

@driskell
Copy link
Contributor Author

@peter279k Is there something I can help do that will help get this merged?

@peter279k peter279k requested a review from c9s March 18, 2024 16:37
@peter279k
Copy link
Member

peter279k commented Mar 18, 2024

Thanks for PR. This PR needs the macOS machine to be verified.

Do you try to build the phpbrew with this PR to verify that?

@driskell
Copy link
Contributor Author

Thanks for PR. This PR needs the macOS machine to be verified.

Do you try to build the phpbrew with this PR to verify that?

I’m sorry I think I’m not understanding. I don’t know what you mean by the PR needing the macOS machine to be verified?

This PR is tested and in use by my colleagues for all builds on our macOS Sonoma 14.x machines all running Apple Silicon and reproduces as explained

@driskell
Copy link
Contributor Author

@driskell, thanks for your PR.
If possible, could you provide your macOS environemt that you run this change? (e.g. Mac Mini M1)
And I can reproduce the issue and verify the PR is correct easily :).

Apple MacBook Pro M3

I gave machine details here as requested some time ago - if you needed OS version too as noted it’s macOS 14 😊

Anything else let me know 👍

@peter279k peter279k self-assigned this Mar 19, 2024
@peter279k peter279k added macOS OpenSSL Issues related to compiling PHP with OpenSSL labels Mar 19, 2024
@driskell driskell force-pushed the fix-openssl-padding-php-lt-81 branch from 82145e5 to 806bb00 Compare March 20, 2024 10:29
@driskell
Copy link
Contributor Author

driskell commented Mar 20, 2024

I've updated this PR with the commit it was fixed in PHP so it's documented.

Reproduction is essentially:

  • On macOS Sonoma 14.4
  • Install openssl@3 with Homebrew
  • Try to install using phpbrew any PHP version before 8.1.0 - for example 8.0.30 - and it will fail without this PR with undeclared symbol on RSA_SSLV23_PADDING

@peter279k
Copy link
Member

Using the latest PHPBrew version 2.2 and compiling the PHP 8.0.30 version log is as follows:

$ phpbrew --debug install 8.0.30 +mbstring +json +filter +openssl +zip +intl +xml +ctype +pcre
make >> '/Users/administrator/.phpbrew/build/php-8.0.30/build.log' 2>&1
Error: Make failed:
The last 5 lines in the log file:
/opt/homebrew/Cellar/openssl@3/3.2.1/include/openssl/rsa.h:300:29: note: passing argument to parameter 'rsa' here

                       RSA *rsa, int padding);

                            ^

108 warnings and 1 error generated.

make: *** [ext/openssl/openssl.lo] Error 1

When Installing the openssl@1.1 package, using this patch to build the PHPBrew. And compile PHP8.0.30 version log is as follows:

$ phpbrew --version
phpbrew - 2.2.0-pr-1359
cliframework core: 2.5.4
$ brew install openssl@1.1
$ brew --prefix openssl@1.1
/opt/homebrew/opt/openssl@1.1
$ phpbrew --debug install 8.0.30 +mbstring +json +filter +openssl +zip +intl +xml +ctype +pcre

./configure '--cache-file=/Users/administrator/.phpbrew/cache/config.cache' '--prefix=/Users/administrator/.phpbrew/php/php-8.0.30' '--disable-all' '--enable-phar' '--enable-session' '--enable-short-tags' '--enable-tokenizer' '--enable-mbstring' '--enable-json' '--enable-filter' '--with-openssl' '--with-zip' '--enable-intl' '--enable-dom' '--with-libxml' '--enable-simplexml' '--enable-xml' '--enable-xmlreader' '--enable-xmlwriter' '--with-xsl' '--enable-ctype' '--enable-opcache' '--enable-cli' '--with-config-file-path=/Users/administrator/.phpbrew/php/php-8.0.30/etc/cli' '--with-config-file-scan-dir=/Users/administrator/.phpbrew/php/php-8.0.30/var/db/cli' 'PKG_CONFIG_PATH=/opt/homebrew/opt/oniguruma/lib/pkgconfig:/opt/homebrew/opt/openssl@1.1/lib/pkgconfig:/opt/homebrew/opt/icu4c/lib/pkgconfig' >> '/Users/administrator/.phpbrew/build/php-8.0.30/build.log' 2>&1

As we can see, it will find the OpenSSL@1.1 package automatically when compiling PHP version is older than 8.1.

And we don't specify the OpenSSL 1.1 manually when compiling these PHP versions:

phpbrew --debug install 8.0. +mbstring +json +filter +openssl='/opt/homebrew/opt/openssl@1.1' +zip +intl +xml +ctype +pcre

We remove the openssl@1.1 package and compile the PHP 8.0.30 again. It will throw the following exception:

$ brew uninstall openssl@1.1
$ rm -rf /opt/homebrew/etc/openssl@1.1
$ phpbrew --debug install 8.0.30 +mbstring +json +filter +openssl +zip +intl +xml +ctype +pcre
===> phpbrew will now build 8.0.30
---> Parsing variants from command arguments '+mbstring +json +filter +openssl +zip +intl +xml +ctype +pcre'
===> Loading and resolving variants...
Homebrew prefix "/opt/homebrew/opt/openssl@1.1" does not exist.
Exception: PHP < 8.1 requires openssl@1.1.
Thrown from /Users/administrator/phpbrew/src/PhpBrew/VariantBuilder.php at line 548:

  545            if ($prefix !== null && $build->compareVersion('8.1') < 0 && $prefix === (new BrewPrefixFinder('openssl@3'))->findPrefix()) {
  546                $prefix = (new BrewPrefixFinder('openssl@1.1'))->findPrefix();
  547                if ($prefix === null) {
> 548                    throw new Exception('PHP < 8.1 requires openssl@1.1.');
  549                }
  550            }
  551
  552            return $parameters->withOptionOrPkgConfigPath($build, '--with-openssl', $prefix);

Trace:

@driskell, please help me to review and check that. Thanks.

@peter279k
Copy link
Member

@c9s, could you help me to review this PR? Thanks.

@peter279k peter279k requested review from c9s and removed request for c9s March 27, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macOS OpenSSL Issues related to compiling PHP with OpenSSL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants