Skip to content

Commit

Permalink
Bug 5390: Non-POD SquidConfig::ssl_client::sslContext exit crash (#1952)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
eduard-bagdasaryan authored and squid-anubis committed Dec 1, 2024
1 parent ee98993 commit c565067
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
3 changes: 2 additions & 1 deletion src/SquidConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 8 additions & 5 deletions src/cache_cf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -959,18 +959,20 @@ 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
debugs(3, DBG_IMPORTANT, "ERROR: proxying https:// currently still requires --with-openssl");
#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()) {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit c565067

Please sign in to comment.