Skip to content

Fix GH-20920: fix build with LibreSSL - #23689

Open
gvozdetsky wants to merge 2 commits into
php:PHP-8.5from
gvozdetsky:fix/gh-20920
Open

gvozdetsky wants to merge 2 commits into
php:PHP-8.5from
gvozdetsky:fix/gh-20920

Conversation

@gvozdetsky

Copy link
Copy Markdown

Fixes #20920.

Supersedes #20932 — thanks to @jordikroon for the original patch and for
letting me pick this up.

This addresses two of the three LibreSSL build failures reported in GH-20920,
one commit each.

1. ENGINE_cleanup() called without a prototype

The call used to be guarded by #ifndef OPENSSL_NO_ENGINE. The v1/v3 backend
split in d662ab5 moved it from ext/openssl/openssl.c into
ext/openssl/openssl_backend_v1.c and kept only the surrounding
#ifdef LIBRESSL_VERSION_NUMBER. LibreSSL defines OPENSSL_NO_ENGINE, so
<openssl/engine.h> is not included and the call is compiled without a
prototype — a warning with older compilers, an error with GCC 14 and later,
which default to C23. That is what @bkuhls hit with gcc15.

This restores the guard, so the call matches the condition of its include,
as suggested by @botovq.

2. PKCS7_NO_DUAL_CONTENT is not always available

The constant was only added in LibreSSL 4.3.0. Following @sthen's suggestion,
it is guarded on the macro itself rather than on LIBRESSL_VERSION_NUMBER, so
that LibreSSL 4.3.0 and later — which added the macro in response to this very
report (openbsd/src@a7d6713) — keep exposing it.

Testing

Built with --enable-debug --enable-zts against LibreSSL 4.2.1 and 4.3.2
(both built from source) and against OpenSSL 3.0.13:

  • LibreSSL 4.2.1 reproduces both errors on PHP-8.5 and compiles cleanly with
    this PR.
  • Under LibreSSL 4.3.2, the set of failing ext/openssl tests is identical
    with and without this PR (26 pre-existing LibreSSL incompatibilities:
    removed TLS 1.0/1.1, DH/DSA differences, openssl_error_string() formats,
    security_level). So this PR neither fixes nor breaks any of them.
  • Under LibreSSL 4.3.2 the constant is still registered
    (PKCS7_NO_DUAL_CONTENT === 65536), confirming the macro-based guard.
  • On OpenSSL 3.0.13: ext/openssl 214 passed, 0 failed, 15 skipped — no
    behaviour change, since OPENSSL_NO_ENGINE is not defined there and the
    constant exists.

This is not sufficient on its own

Linking still fails under LibreSSL with:

undefined reference to `sk_X509_new_reserve'
  ext/openssl/openssl_backend_common.c:714 (php_openssl_load_all_certs_from_file)

#21430 fixes that, and it is needed on PHP-8.5 as well, not only on master.
With #21430 applied on top of this PR, the build against LibreSSL 4.2.1
completes successfully.

@bkuhls, @bneumeier — could you confirm this matches what you see on your
LibreSSL setups?

The call was guarded by #ifndef OPENSSL_NO_ENGINE until the v1/v3 backend
split in d662ab5 moved it from ext/openssl/openssl.c into
ext/openssl/openssl_backend_v1.c, where only the surrounding
#ifdef LIBRESSL_VERSION_NUMBER was kept.

LibreSSL defines OPENSSL_NO_ENGINE, so <openssl/engine.h> is not included
and ENGINE_cleanup() ends up being called without a prototype.  That is a
warning with older compilers and an error with GCC 14 and later, which
default to C23.  Restore the guard so that the call matches the condition
of its include.
PKCS7_NO_DUAL_CONTENT was only added in LibreSSL 4.3.0, so the generated
registration in openssl_arginfo.h fails to compile against older releases.
Guard the constant on the macro itself rather than on LIBRESSL_VERSION_NUMBER
so that LibreSSL 4.3.0 and later, which added the macro in response to this
report, keep exposing the constant.
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.

1 participant