fix(Crypto,NetSSL): OpenSSL fixes, version macro cleanup, dead code removal by matejk · Pull Request #5246 · pocoproject/poco

matejk · 2026-03-15T14:31:17Z

Summary

fix(NetSSL): Report the most relevant OpenSSL error in SecureSocketImpl::handleError() by using ERR_peek_last_error() + ERR_clear_error() instead of ERR_get_error() which returns the oldest (often stale) error from the FIFO queue. Fixes InvalidCertificateHandler onInvalidCertificate wrong exception #4772.
fix(NetSSL): Restore bidirectional SSL_shutdown() for blocking sockets with a timeout-based retry loop polling for SSL_ERROR_WANT_READ/SSL_ERROR_WANT_WRITE, with non-retryable errors mapped through handleError(). Non-blocking sockets retain the single-call behavior. Fixes SecureSocketImpl::shutdown() regression since 1.14.0 #4883.
fix(Crypto): Add CONF_modules_unload(1) to OpenSSLInitializer::uninitialize() for OpenSSL 3.x when the reference count reaches zero (previously empty). OSSL_PROVIDER_unload() for the default and legacy providers was implemented and tested (see commit history), but OpenSSL leaks internal OSSL_LIB_CTX child context memory on the unload path, causing LeakSanitizer failures in CI. Provider cleanup is therefore delegated to OpenSSL's OPENSSL_cleanup atexit handler. Partially addresses OpenSSLInitializer::uninitialize() Openssl 3.x #4451.
enh(Crypto): Introduce POCO_OPENSSL_VERSION_PREREQ(maj, min, pat) macro replacing the old 2-parameter OPENSSL_VERSION_PREREQ and all raw OPENSSL_VERSION_NUMBER comparisons. Includes documentation on LibreSSL versioning (sets OPENSSL_VERSION_NUMBER to 0x20000000L) and guidance to use #if !defined(LIBRESSL_VERSION_NUMBER) guards for API divergences.
chore(Crypto,NetSSL): Remove dead version-guarded code paths that are always true or always false given the minimum OpenSSL 1.1.1 requirement. Affected files: DigestEngine.cpp, Envelope.cpp, ECDSADigestEngine.cpp, EVPTest.cpp/.h, Session.cpp, Context.cpp, SSLManager.cpp/.h, X509Certificate.cpp.
test(Crypto): Add OpenSSLInitializerTest covering reference-counted init/uninit, RAII pattern, and provider availability for OpenSSL 3.x.
test(NetSSL): Add testShutdownBidirectional for blocking socket SSL shutdown verification. Add bounded timeouts to all connection wait loops to prevent CI hangs.

Test plan

Crypto tests pass (47 tests OK)
NetSSL tests pass (60 runs, only 2 pre-existing proxy-related failures)

Copilot

Pull request overview

This PR updates Poco’s OpenSSL integration across Crypto and NetSSL_OpenSSL to improve OpenSSL 3.x compatibility, simplify version-guarding, and address SSL shutdown/error-reporting regressions, with new tests added to validate behavior.

Changes:

Improve OpenSSL error reporting in SecureSocketImpl::handleError() and rework SecureSocketImpl::shutdown() to support bidirectional shutdown for blocking sockets.
Introduce POCO_OPENSSL_VERSION_PREREQ(maj, min, pat) and remove dead/obsolete OpenSSL version-guarded code paths (min OpenSSL 1.1.1).
Add new unit tests for OpenSSL initializer behavior and a NetSSL shutdown regression test.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`NetSSL_OpenSSL/testsuite/src/SecureStreamSocketTest.h`	Registers new shutdown regression test.
`NetSSL_OpenSSL/testsuite/src/SecureStreamSocketTest.cpp`	Adds `testShutdownBidirectional()` regression test for #4883.
`NetSSL_OpenSSL/src/SecureSocketImpl.cpp`	Implements blocking-socket bidirectional shutdown loop; improves OpenSSL error stack reporting.
`NetSSL_OpenSSL/src/Context.cpp`	Replaces raw OpenSSL version checks with `POCO_OPENSSL_VERSION_PREREQ` and removes dead guards.
`NetSSL_OpenSSL/src/SSLManager.cpp`	Removes obsolete OpenSSL version guards around OCSP headers/callback logic.
`NetSSL_OpenSSL/include/Poco/Net/SSLManager.h`	Removes obsolete OpenSSL < 1.0.1 FIPS include guard.
`NetSSL_OpenSSL/src/Session.cpp`	Removes dead fallback path for non-resumable sessions on older OpenSSL.
`NetSSL_OpenSSL/src/X509Certificate.cpp`	Removes obsolete pre-1.0.2 hostname verification path; uses `X509_check_host`/IP checks.
`Crypto/include/Poco/Crypto/Crypto.h`	Introduces `POCO_OPENSSL_VERSION_PREREQ(maj, min, pat)` and updates min-version check.
`Crypto/include/Poco/Crypto/OpenSSLInitializer.h`	Updates OpenSSL 3.x guards to use the new prereq macro.
`Crypto/src/OpenSSLInitializer.cpp`	Adds OpenSSL 3.x module unload call on refcount reaching zero (and documents provider-cleanup strategy).
`Crypto/testsuite/src/OpenSSLInitializerTest.h`	Adds new Crypto tests for OpenSSL initializer behavior (OpenSSL 3.x provider availability guarded).
`Crypto/testsuite/src/OpenSSLInitializerTest.cpp`	Implements new tests for reference-counted init/uninit and provider availability.
`Crypto/testsuite/src/CryptoTestSuite.cpp`	Registers `OpenSSLInitializerTest` in the Crypto test suite.
`Crypto/testsuite/src/EVPTest.h`	Updates OpenSSL version guards to `POCO_OPENSSL_VERSION_PREREQ`.
`Crypto/testsuite/src/EVPTest.cpp`	Updates OpenSSL version guards to `POCO_OPENSSL_VERSION_PREREQ`.
`Crypto/src/CipherFactory.cpp`	Switches OpenSSL 3.x guard to `POCO_OPENSSL_VERSION_PREREQ`.
`Crypto/src/DigestEngine.cpp`	Switches OpenSSL 3.x guard to `POCO_OPENSSL_VERSION_PREREQ` and removes dead pre-1.1.0 paths.
`Crypto/src/ECDSADigestEngine.cpp`	Removes dead pre-1.1.0 ECDSA compatibility code.
`Crypto/src/Envelope.cpp`	Removes dead pre-1.1.0 EVP cipher ctx init path.
`Crypto/src/EVPPKey.cpp`	Switches OpenSSL 3.x guard to `POCO_OPENSSL_VERSION_PREREQ`.
`Crypto/include/Poco/Crypto/EVPPKey.h`	Switches OpenSSL 3.x guard to `POCO_OPENSSL_VERSION_PREREQ`.
`Crypto/src/PKCS12Container.cpp`	Switches OpenSSL 3.x guard to `POCO_OPENSSL_VERSION_PREREQ`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T13:09:18Z

+#if POCO_OPENSSL_VERSION_PREREQ(3, 0, 0)
+		// Provider cleanup is deliberately left to OpenSSL's internal
+		// atexit handler (OPENSSL_cleanup). We must NOT:
+		//  - call OSSL_PROVIDER_unload(): leaks OSSL_LIB_CTX child
+		//    contexts that the unload path does not fully free
+		//  - null the static pointers: makes the provider allocations
+		//    unreachable, causing LeakSanitizer to report them as leaks
+		//    (LSAN runs before atexit handlers on Linux/GCC)
+		// The pointers remain valid and reachable until process exit,
+		// at which point OPENSSL_cleanup frees everything.
+		CONF_modules_unload(1);
+#endif


The PR description says OpenSSL 3.x provider cleanup is added via OSSL_PROVIDER_unload() for default/legacy providers, but uninitialize() explicitly does not unload providers and only calls CONF_modules_unload(1). Please align the implementation with the PR description (either implement the unload + pointer handling safely, or update the PR description/tests to reflect the intentional atexit-based cleanup behavior).

Copilot · 2026-04-01T13:09:19Z

+void OpenSSLInitializerTest::testMultipleInitialize()
+{
+	// Multiple initialize calls should not throw.
+	// The reference count keeps track internally.
+	OpenSSLInitializer::initialize();
+	OpenSSLInitializer::initialize();
+	OpenSSLInitializer::initialize();
+
+	// Matching uninitialize calls
+	OpenSSLInitializer::uninitialize();
+	OpenSSLInitializer::uninitialize();
+	OpenSSLInitializer::uninitialize();
+}


These tests never exercise the ref-count-to-zero path in OpenSSLInitializer::uninitialize() (where CONF_modules_unload(1) and any provider cleanup would run) because the Crypto testsuite driver initializes Crypto/OpenSSL globally for the full test run. As a result, _rc typically stays >= 1 and the new cleanup logic is effectively untested. Consider adding a test setup that brings the refcount to zero (and restores it), or running this suite without the global CryptoInitializer so the cleanup branch is covered.

…emoval - fix(NetSSL): use ERR_peek_last_error in handleError() to report the most recent (relevant) OpenSSL error instead of the oldest one from the FIFO error queue (fixes #4772) - fix(NetSSL): restore bidirectional SSL_shutdown() for blocking sockets with timeout-based retry loop; preserve single-call behavior for non-blocking sockets (fixes #4883) - fix(Crypto): add provider cleanup in OpenSSLInitializer::uninitialize() for OpenSSL 3.x — unload legacy and default providers and call CONF_modules_unload when ref count reaches zero (fixes #4451) - enh(Crypto): introduce POCO_OPENSSL_VERSION_PREREQ(maj, min, pat) macro with documentation on LibreSSL versioning implications; replace all raw OPENSSL_VERSION_NUMBER comparisons across Crypto and NetSSL_OpenSSL - chore(Crypto,NetSSL): remove dead version-guarded code paths that are always true/false given minimum OpenSSL 1.1.1 requirement (removes pre-1.1.1 fallbacks in DigestEngine, Envelope, ECDSADigestEngine, EVPTest, Session, Context, SSLManager, X509Certificate) - test(Crypto): add OpenSSLInitializerTest covering ref-counted init/uninit, RAII pattern, and provider lifecycle (load, unload, reload) for OpenSSL 3.x - test(NetSSL): add testShutdownBidirectional for blocking socket SSL shutdown

Avoid reloading OpenSSL providers after a full uninitialize cycle in the test. OpenSSL internally leaks OSSL_LIB_CTX state when providers are unloaded and reloaded, triggering LeakSanitizer false positives. Also add haveDefaultProvider() accessor to verify that both default and legacy provider pointers are properly nullified after cleanup.

Remove testProviderCleanup which caused LeakSanitizer failures in all ASAN CI jobs. The test drained the OpenSSL ref count to 0 (unloading providers) and then had to re-initialize to restore state, but OpenSSL internally leaks OSSL_LIB_CTX state when providers are unloaded and reloaded in the same process (~6517 bytes / 117 allocations). The provider cleanup code in uninitialize() is validated indirectly: if OSSL_PROVIDER_unload was missing, ASAN would detect the leak from the initial provider load (since those providers would never be freed). Also: - Remove haveDefaultProvider() which was only used by this test - Add comment in uninitialize() documenting the OpenSSL reload leak - Add comment in test file explaining why cleanup cannot be tested

Do not call OSSL_PROVIDER_unload() explicitly in uninitialize(). OpenSSL's internal atexit handler (OPENSSL_cleanup) properly frees all provider state at process exit. Calling OSSL_PROVIDER_unload() ourselves causes LeakSanitizer to report indirect leaks from OSSL_LIB_CTX child contexts that are not fully cleaned up by the unload path. Instead, just reset the managed pointers so haveLegacyProvider() reflects that the library is considered uninitialized from POCO's perspective.

Do not null the static provider pointers in uninitialize(). On Linux with GCC's ASAN, LeakSanitizer runs before atexit handlers, so nulling the pointers makes the provider allocations unreachable and triggers leak reports. Keeping the pointers valid lets LSAN see them as still-reachable while OPENSSL_cleanup frees everything at exit. Also do not call OSSL_PROVIDER_unload() — it leaks OSSL_LIB_CTX child contexts. Provider cleanup is fully delegated to OpenSSL's internal atexit handler.

- Handle non-retryable SSL errors in blocking shutdown loop by passing rc through handleError() after the loop (same as non-blocking branch) - Handle partial writes in testShutdownBidirectional sendBytes loop - Add 10s timeout to all connection wait loops in SecureStreamSocketTest to prevent CI hangs

…PR#5313 PR#5313 (OPENSSL_NO_DEPRECATED) uses POCO_OPENSSL_VERSION_PREREQ(maj,min,pat) which was introduced on main by PR#5246. Since #5246 was not cherry-picked, add the macro definition to Crypto.h alongside the existing 2-arg OPENSSL_VERSION_PREREQ macro.

…emoval (#5246)

matejk requested review from aleks-f and obiltschnig March 15, 2026 14:32

matejk force-pushed the openssl-fixes-and-cleanup branch from 641ac53 to 7de5073 Compare March 26, 2026 13:16

aleks-f requested a review from Copilot April 1, 2026 12:55

Copilot started reviewing on behalf of aleks-f April 1, 2026 12:58 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

matejk added 5 commits April 1, 2026 15:12

matejk force-pushed the openssl-fixes-and-cleanup branch from 7de5073 to 94d71e8 Compare April 1, 2026 13:13

matejk mentioned this pull request Apr 1, 2026

OpenSSLInitializer::uninitialize() Openssl 3.x #4451

Open

matejk merged commit e0ca996 into main Apr 1, 2026
102 checks passed

matejk deleted the openssl-fixes-and-cleanup branch April 1, 2026 17:17

matejk added a commit that referenced this pull request Apr 16, 2026

fix(Crypto,NetSSL): OpenSSL fixes, version macro cleanup, dead code r…

1ea8676

…emoval (#5246)

matejk added this to the Release 1.15.2 milestone Apr 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Crypto,NetSSL): OpenSSL fixes, version macro cleanup, dead code removal#5246

fix(Crypto,NetSSL): OpenSSL fixes, version macro cleanup, dead code removal#5246
matejk merged 6 commits intomainfrom
openssl-fixes-and-cleanup

matejk commented Mar 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

matejk commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

matejk commented Mar 15, 2026 •

edited

Loading