From c565067bf2ddf3ce88ed27d71d1d8816ee03047e Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Sat, 30 Nov 2024 03:04:33 +0000 Subject: [PATCH] Bug 5390: Non-POD SquidConfig::ssl_client::sslContext exit crash (#1952) Squid may crash when the SquidConfig global is auto-destructed after main() ends. Since SquidConfig global is used by cleanup code, we should keep its fields alive, essentially emulating "No New Globals" policy effects. This surgical fix will be followed up with more changes to address general OpenSSL cleanup problems exposed by this bug. This bug fix facilitates backporting by using FuturePeerContext shim. --- src/SquidConfig.h | 3 ++- src/cache_cf.cc | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/SquidConfig.h b/src/SquidConfig.h index 0598a2369b6..a79a6cdc4fc 100644 --- a/src/SquidConfig.h +++ b/src/SquidConfig.h @@ -505,7 +505,8 @@ class SquidConfig struct { Security::FuturePeerContext *defaultPeerContext; // TODO: Remove when FuturePeerContext above becomes PeerContext - Security::ContextPointer sslContext; + /// \deprecated Legacy storage. Use defaultPeerContext instead. + Security::ContextPointer *sslContext_; #if USE_OPENSSL char *foreignIntermediateCertsPath; acl_access *cert_error; diff --git a/src/cache_cf.cc b/src/cache_cf.cc index 34bf9c43bbf..24105a6ae05 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -959,8 +959,10 @@ configDoConfigure(void) if (Security::ProxyOutgoingConfig.encryptTransport) { debugs(3, 2, "initializing https:// proxy context"); - Config.ssl_client.sslContext = Security::ProxyOutgoingConfig.createClientContext(false); - if (!Config.ssl_client.sslContext) { + + const auto rawSslContext = Security::ProxyOutgoingConfig.createClientContext(false); + Config.ssl_client.sslContext_ = rawSslContext ? new Security::ContextPointer(rawSslContext) : nullptr; + if (!Config.ssl_client.sslContext_) { #if USE_OPENSSL fatal("ERROR: Could not initialize https:// proxy context"); #else @@ -968,9 +970,9 @@ configDoConfigure(void) #endif } #if USE_OPENSSL - Ssl::useSquidUntrusted(Config.ssl_client.sslContext.get()); + Ssl::useSquidUntrusted(Config.ssl_client.sslContext_->get()); #endif - Config.ssl_client.defaultPeerContext = new Security::FuturePeerContext(Security::ProxyOutgoingConfig, Config.ssl_client.sslContext); + Config.ssl_client.defaultPeerContext = new Security::FuturePeerContext(Security::ProxyOutgoingConfig, *Config.ssl_client.sslContext_); } for (const auto &p: CurrentCachePeers()) { @@ -3913,7 +3915,8 @@ configFreeMemory(void) Dns::ResolveClientAddressesAsap = false; delete Config.ssl_client.defaultPeerContext; Config.ssl_client.defaultPeerContext = nullptr; - Config.ssl_client.sslContext.reset(); + delete Config.ssl_client.sslContext_; + Config.ssl_client.sslContext_ = nullptr; #if USE_OPENSSL Ssl::unloadSquidUntrusted(); #endif