NoNewGlobals for OpenSSL-related structures (#2006)

These changes were anticipated in Bug 5390 fix (recent commit c565067):
https://bugs.squid-cache.org/show_bug.cgi?id=5390#c16

They eliminate all known OpenSSL-related globals:

* Security::ProxyOutgoingConfig
* Ssl::SquidUntrustedCerts
* Ssl::TheGeneratorRequests
* Ssl::TheGlobalContextStorage

Also applied AAA and range-based `for` loop upgrades to modified lines.

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 24105a6ae0504dbe845e33431ec6b7347b3549c2..0627a4dd1301c6a7d7309a55394c3786671235b8 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -957,10 +957,10 @@ configDoConfigure(void)
         Ssl::loadSquidUntrusted(Config.ssl_client.foreignIntermediateCertsPath);
 #endif
 
-    if (Security::ProxyOutgoingConfig.encryptTransport) {
+    if (Security::ProxyOutgoingConfig().encryptTransport) {
         debugs(3, 2, "initializing https:// proxy context");
 
-        const auto rawSslContext = Security::ProxyOutgoingConfig.createClientContext(false);
+        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
@@ -972,7 +972,7 @@ configDoConfigure(void)
 #if USE_OPENSSL
         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()) {
@@ -1112,7 +1112,7 @@ parse_obsolete(const char *name)
             throw TextException(ToSBuf("missing required parameter for obsolete directive: ", name), Here());
 
         tmp.append(token, strlen(token));
-        Security::ProxyOutgoingConfig.parse(tmp.c_str());
+        Security::ProxyOutgoingConfig().parse(tmp.c_str());
     }
 }
 

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 560573fac6237df74dc534cef85274985d9256bf..3bfcc9e1efe850a36acf4b1e1f15077afac2954e 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -3024,7 +3024,7 @@ NAME: tls_outgoing_options
 IFDEF: HAVE_LIBGNUTLS||USE_OPENSSL
 TYPE: securePeerOptions
 DEFAULT: min-version=1.0
-LOC: Security::ProxyOutgoingConfig
+LOC: Security::ProxyOutgoingConfig()
 DOC_START
        disable         Do not support https:// URLs.
 

diff --git a/src/client_side.cc b/src/client_side.cc
index deb64d19ea7199bf6cae04a30ba232cbc90c6f56..ee43129ca0bf0feafc1a79d68eaf45ab2357b0f2 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2647,7 +2647,7 @@ Security::ContextPointer
 ConnStateData::getTlsContextFromCache(const SBuf &cacheKey, const Ssl::CertificateProperties &certProperties)
 {
     debugs(33, 5, "Finding SSL certificate for " << cacheKey << " in cache");
-    Ssl::LocalContextStorage * ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
+    const auto ssl_ctx_cache = Ssl::TheGlobalContextStorage().getLocalStorage(port->s);
     if (const auto ctx = ssl_ctx_cache ? ssl_ctx_cache->get(cacheKey) : nullptr) {
         if (Ssl::verifySslCertificate(*ctx, certProperties)) {
             debugs(33, 5, "Cached SSL certificate for " << certProperties.commonName << " is valid");
@@ -2664,7 +2664,7 @@ ConnStateData::getTlsContextFromCache(const SBuf &cacheKey, const Ssl::Certifica
 void
 ConnStateData::storeTlsContextToCache(const SBuf &cacheKey, Security::ContextPointer &ctx)
 {
-    Ssl::LocalContextStorage *ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
+    const auto ssl_ctx_cache = Ssl::TheGlobalContextStorage().getLocalStorage(port->s);
     if (!ssl_ctx_cache || !ssl_ctx_cache->add(cacheKey, ctx)) {
         // If it is not in storage delete after using. Else storage deleted it.
         fd_table[clientConnection->fd].dynamicTlsContext = ctx;
@@ -3268,7 +3268,7 @@ clientHttpConnectionsOpen(void)
             }
             if (s->flags.tunnelSslBumping) {
                 // Create ssl_ctx cache for this port.
-                Ssl::TheGlobalContextStorage.addLocalStorage(s->s, s->secure.dynamicCertMemCacheSize);
+                Ssl::TheGlobalContextStorage().addLocalStorage(s->s, s->secure.dynamicCertMemCacheSize);
             }
         }
 #endif

diff --git a/src/main.cc b/src/main.cc
index f673d50ff642800e951d694865ee55accd8105ec..deaa3dc9019766d71f5ce2ee5aaee5d476c3c81b 100644 (file)
--- a/src/main.cc
+++ b/src/main.cc
@@ -857,7 +857,7 @@ mainReconfigureStart(void)
     htcpClosePorts();
 #endif
 #if USE_OPENSSL
-    Ssl::TheGlobalContextStorage.reconfigureStart();
+    Ssl::TheGlobalContextStorage().reconfigureStart();
 #endif
 #if USE_AUTH
     authenticateReset();

diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc
index 62bc08d0467943c1e391bad31e1ba9bc79739b4b..d505ef475b1229cf1725173aaaa92694216df2dd 100644 (file)
--- a/src/security/PeerOptions.cc
+++ b/src/security/PeerOptions.cc
@@ -21,7 +21,12 @@
 
 #include <bitset>
 
-Security::PeerOptions Security::ProxyOutgoingConfig;
+Security::PeerOptions &
+Security::ProxyOutgoingConfig()
+{
+    static const auto peerOptions = new PeerOptions();
+    return *peerOptions;
+}
 
 Security::PeerOptions::PeerOptions()
 {

diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h
index 5db3d5a7167100f3a6a95551e3e7f363d40e39f5..984a9afe1dc3f701a4233989fe1040ed2f242bb4 100644 (file)
--- a/src/security/PeerOptions.h
+++ b/src/security/PeerOptions.h
@@ -161,13 +161,13 @@ public:
 };
 
 /// configuration options for DIRECT server access
-extern PeerOptions ProxyOutgoingConfig;
+PeerOptions &ProxyOutgoingConfig();
 
 } // namespace Security
 
 // parse the tls_outgoing_options directive
 void parse_securePeerOptions(Security::PeerOptions *);
-#define free_securePeerOptions(x) Security::ProxyOutgoingConfig.clear()
+#define free_securePeerOptions(x) Security::ProxyOutgoingConfig().clear()
 #define dump_securePeerOptions(e,n,x) do { PackableStream os_(*(e)); os_ << n; (x).dumpCfg(os_,""); os_ << '\n'; } while (false)
 
 #endif /* SQUID_SRC_SECURITY_PEEROPTIONS_H */

diff --git a/src/ssl/context_storage.cc b/src/ssl/context_storage.cc
index 36d549c382906fe25066fec9cbc6b6eb91aeb085..a17a3f0b8b449e3b23b7b3a65858026889f27c4a 100644 (file)
--- a/src/ssl/context_storage.cc
+++ b/src/ssl/context_storage.cc
@@ -41,9 +41,9 @@ void Ssl::CertificateStorageAction::dump (StoreEntry *sentry)
     stream << "Port" << delimiter << "Max mem(KB)" << delimiter << "Cert number" << delimiter << "KB/cert" << delimiter << "Mem used(KB)" << delimiter << "Mem free(KB)" << endString;
 
     // Add info for each port.
-    for (std::map<Ip::Address, LocalContextStorage *>::iterator i = TheGlobalContextStorage.storage.begin(); i != TheGlobalContextStorage.storage.end(); ++i) {
-        stream << i->first << delimiter;
-        LocalContextStorage & ssl_store_policy(*(i->second));
+    for (const auto &i: TheGlobalContextStorage().storage) {
+        stream << i.first << delimiter;
+        const auto &ssl_store_policy = *i.second;
         const auto memoryPerEntry = ssl_store_policy.entries() ?
                                     ssl_store_policy.memoryUsed() / ssl_store_policy.entries() : 0;
         stream << ssl_store_policy.memLimit() / 1024 << delimiter;
@@ -120,5 +120,10 @@ void Ssl::GlobalContextStorage::reconfigureFinish()
     }
 }
 
-Ssl::GlobalContextStorage Ssl::TheGlobalContextStorage;
+Ssl::GlobalContextStorage &
+Ssl::TheGlobalContextStorage()
+{
+    static const auto storage = new GlobalContextStorage();
+    return *storage;
+}
 

diff --git a/src/ssl/context_storage.h b/src/ssl/context_storage.h
index 4dd49968c410c8d03100b63bfd375820f970c60f..caac510ba59c9e88a425303545a4356dcb379bb7 100644 (file)
--- a/src/ssl/context_storage.h
+++ b/src/ssl/context_storage.h
@@ -74,7 +74,7 @@ private:
 };
 
 /// Global cache for store all SSL server certificates.
-extern GlobalContextStorage TheGlobalContextStorage;
+GlobalContextStorage &TheGlobalContextStorage();
 } //namespace Ssl
 #endif // USE_OPENSSL
 

diff --git a/src/ssl/helper.cc b/src/ssl/helper.cc
index 42b4594b2e2de0b72e4b558f47d88cf1a41f0925..70635469183d4b712d183be00fcdba7ac07f21e4 100644 (file)
--- a/src/ssl/helper.cc
+++ b/src/ssl/helper.cc
@@ -57,6 +57,14 @@ typedef std::unordered_map<SBuf, GeneratorRequest*> GeneratorRequests;
 
 static void HandleGeneratorReply(void *data, const ::Helper::Reply &reply);
 
+/// pending Ssl::Helper requests (to all certificate generator helpers combined)
+static GeneratorRequests &
+TheGeneratorRequests()
+{
+    static auto generatorRequests = new GeneratorRequests();
+    return *generatorRequests;
+}
+
 } // namespace Ssl
 
 CBDATA_NAMESPACED_CLASS_INIT(Ssl, GeneratorRequest);
@@ -68,9 +76,6 @@ operator <<(std::ostream &os, const Ssl::GeneratorRequest &gr)
     return os << "crtGenRq" << gr.query.id.value << "/" << gr.requestors.size();
 }
 
-/// pending Ssl::Helper requests (to all certificate generator helpers combined)
-static Ssl::GeneratorRequests TheGeneratorRequests;
-
 Helper::Client::Pointer Ssl::Helper::ssl_crtd;
 
 void Ssl::Helper::Init()
@@ -125,8 +130,8 @@ void Ssl::Helper::Submit(CrtdMessage const & message, HLPCB * callback, void * d
     SBuf rawMessage(message.compose().c_str()); // XXX: helpers cannot use SBuf
     rawMessage.append("\n", 1);
 
-    const auto pending = TheGeneratorRequests.find(rawMessage);
-    if (pending != TheGeneratorRequests.end()) {
+    const auto pending = TheGeneratorRequests().find(rawMessage);
+    if (pending != TheGeneratorRequests().end()) {
         pending->second->emplace(callback, data);
         debugs(83, 5, "collapsed request from " << data << " onto " << *pending->second);
         return;
@@ -135,7 +140,7 @@ void Ssl::Helper::Submit(CrtdMessage const & message, HLPCB * callback, void * d
     GeneratorRequest *request = new GeneratorRequest;
     request->query = rawMessage;
     request->emplace(callback, data);
-    TheGeneratorRequests.emplace(request->query, request);
+    TheGeneratorRequests().emplace(request->query, request);
     debugs(83, 5, "request from " << data << " as " << *request);
     // ssl_crtd becomes nil if Squid is reconfigured without SslBump or
     // certificate generation disabled in the new configuration
@@ -153,7 +158,7 @@ Ssl::HandleGeneratorReply(void *data, const ::Helper::Reply &reply)
 {
     const std::unique_ptr<Ssl::GeneratorRequest> request(static_cast<Ssl::GeneratorRequest*>(data));
     assert(request);
-    const auto erased = TheGeneratorRequests.erase(request->query);
+    const auto erased = TheGeneratorRequests().erase(request->query);
     assert(erased);
 
     for (auto &requestor: request->requestors) {

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 7026301b27232caf27f3059ef26ecb5adcf5c75b..d0e57fc1e2e039e4a099b6e512a4ddb0f4220fde 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -41,8 +41,6 @@
 // TODO: Move ssl_ex_index_* global variables from global.cc here.
 static int ssl_ex_index_verify_callback_parameters = -1;
 
-static Ssl::CertsIndexedList SquidUntrustedCerts;
-
 const EVP_MD *Ssl::DefaultSignHash = nullptr;
 
 std::vector<const char *> Ssl::BumpModeStr = {
@@ -73,6 +71,13 @@ protected:
     AnyP::Host needle_; ///< a name we are looking for
 };
 
+static CertsIndexedList &
+SquidUntrustedCerts()
+{
+    static auto untrustedCerts = new CertsIndexedList();
+    return *untrustedCerts;
+}
+
 } // namespace Ssl
 
 bool
@@ -1306,7 +1311,7 @@ Ssl::findIssuerCertificate(X509 *cert, const STACK_OF(X509) *serverCertificates,
     }
 
     // check untrusted certificates
-    if (const auto issuer = findCertIssuerFast(SquidUntrustedCerts, cert)) {
+    if (const auto issuer = findCertIssuerFast(SquidUntrustedCerts(), cert)) {
         X509_up_ref(issuer);
         return Security::CertPointer(issuer);
     }
@@ -1347,7 +1352,7 @@ static void
 completeIssuers(X509_STORE_CTX *ctx, STACK_OF(X509) &untrustedCerts)
 {
     debugs(83, 2,  "completing " << sk_X509_num(&untrustedCerts) <<
-           " OpenSSL untrusted certs using " << SquidUntrustedCerts.size() <<
+           " OpenSSL untrusted certs using " << Ssl::SquidUntrustedCerts().size() <<
            " configured untrusted certificates");
 
     const X509_VERIFY_PARAM *param = X509_STORE_CTX_get0_param(ctx);
@@ -1397,7 +1402,7 @@ VerifyCtxCertificates(X509_STORE_CTX *ctx, STACK_OF(X509) *extraCerts)
 
     // If the local untrusted certificates internal database is used
     // run completeIssuers to add missing certificates if possible.
-    if (SquidUntrustedCerts.size() > 0)
+    if (Ssl::SquidUntrustedCerts().size() > 0)
         completeIssuers(ctx, *untrustedCerts);
 
     X509_STORE_CTX_set0_untrusted(ctx, untrustedCerts.get()); // No locking/unlocking, just sets ctx->untrusted
@@ -1441,17 +1446,17 @@ Ssl::useSquidUntrusted(SSL_CTX *sslContext)
 bool
 Ssl::loadSquidUntrusted(const char *path)
 {
-    return Ssl::loadCerts(path, SquidUntrustedCerts);
+    return Ssl::loadCerts(path, SquidUntrustedCerts());
 }
 
 void
 Ssl::unloadSquidUntrusted()
 {
-    if (SquidUntrustedCerts.size()) {
-        for (Ssl::CertsIndexedList::iterator it = SquidUntrustedCerts.begin(); it != SquidUntrustedCerts.end(); ++it) {
-            X509_free(it->second);
+    if (SquidUntrustedCerts().size()) {
+        for (const auto &i: SquidUntrustedCerts()) {
+            X509_free(i.second);
         }
-        SquidUntrustedCerts.clear();
+        SquidUntrustedCerts().clear();
     }
 }
 

diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index ee57bca870c1b8c58591da74f88332dd03e50b7a..f7314a6c245292ec100bef0f93580c2fea5dcb47 100644 (file)
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -112,7 +112,7 @@ EncryptorAnswer &PeerConnector::answer() STUB_RETREF(EncryptorAnswer)
 }
 
 #include "security/PeerOptions.h"
-Security::PeerOptions Security::ProxyOutgoingConfig;
+Security::PeerOptions &Security::ProxyOutgoingConfig() STUB_RETREF(Security::PeerOptions)
 
 Security::PeerOptions::PeerOptions() {
 #if USE_OPENSSL

diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc
index a3cd170e6b21caef500d6343204f08f77f94ec88..0ef8a4ccccb03afd249f79c87bb99aa042894440 100644 (file)
--- a/src/tests/stub_libsslsquid.cc
+++ b/src/tests/stub_libsslsquid.cc
@@ -40,7 +40,6 @@ void Ssl::GlobalContextStorage::addLocalStorage(Ip::Address const &, size_t ) ST
 Ssl::LocalContextStorage *Ssl::GlobalContextStorage::getLocalStorage(Ip::Address const &)
 { fatal(STUB_API " required"); static LocalContextStorage v(0); return &v; }
 void Ssl::GlobalContextStorage::reconfigureStart() STUB
-//Ssl::GlobalContextStorage Ssl::TheGlobalContextStorage;
 
 #include "ssl/ErrorDetail.h"
 #include "ssl/support.h"