dynamic_cert_mem_cache_size option related fixes

This patch fixes the following problems:

1) The dynamic_cert_mem_cache_size does not change on reconfigure

2) When dynamic_cert_mem_cache_size of http_port set to 0 then:

   a) The dynamic certs cache is grow unlimited.
      This patch just disables certificates caching when this option set to 0.

   b) Huge amount of memory appeared as free cache memory in  "Cached ssl
      certificates statistic" page of cache manager.
      This problem caused because of a signed to unsigned int conversion.

  This is a Measurement Factory project

diff --git a/src/base/LruMap.h b/src/base/LruMap.h
index 39eb44c82d1132c26171b0a5eaf78cdc0b27bd07..c558666812c8c3295b8e55cc0f4ece31303d9feb 100644 (file)
--- a/src/base/LruMap.h
+++ b/src/base/LruMap.h
@@ -50,7 +50,7 @@ public:
     /// The available size for the map
     size_t memLimit() const {return memLimit_;}
     /// The free space of the map
-    size_t freeMem() const { return (memLimit() - size());}
+    size_t freeMem() const { return (memLimit() > size() ? memLimit() - size() : 0);}
     /// The current size of the map
     size_t size() const {return (entries_ * EntryCost);}
     /// The number of stored entries

diff --git a/src/client_side.cc b/src/client_side.cc
index c1d13e6a381dd7e0e97dfe13feace6efab782d0e..3d9614df08fa4e2dfc6b7e0ec20efc7423dd9d13 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3888,9 +3888,9 @@ ConnStateData::getSslContextStart()
         assert(sslBumpCertKey.defined() && sslBumpCertKey[0] != '\0');
 
         debugs(33, 5, HERE << "Finding SSL certificate for " << sslBumpCertKey << " in cache");
-        Ssl::LocalContextStorage & ssl_ctx_cache(Ssl::TheGlobalContextStorage.getLocalStorage(port->s));
+        Ssl::LocalContextStorage *ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
         SSL_CTX * dynCtx = NULL;
-        Ssl::SSL_CTX_Pointer *cachedCtx = ssl_ctx_cache.get(sslBumpCertKey.termedBuf());
+        Ssl::SSL_CTX_Pointer *cachedCtx = ssl_ctx_cache ? ssl_ctx_cache->get(sslBumpCertKey.termedBuf()) : NULL;
         if (cachedCtx && (dynCtx = cachedCtx->get())) {
             debugs(33, 5, HERE << "SSL certificate for " << sslBumpCertKey << " have found in cache");
             if (Ssl::verifySslCertificate(dynCtx, certProperties)) {
@@ -3899,7 +3899,8 @@ ConnStateData::getSslContextStart()
                 return;
             } else {
                 debugs(33, 5, HERE << "Cached SSL certificate for " << sslBumpCertKey << " is out of date. Delete this certificate from cache");
-                ssl_ctx_cache.del(sslBumpCertKey.termedBuf());
+                if (ssl_ctx_cache)
+                    ssl_ctx_cache->del(sslBumpCertKey.termedBuf());
             }
         } else {
             debugs(33, 5, HERE << "SSL certificate for " << sslBumpCertKey << " haven't found in cache");
@@ -3951,10 +3952,10 @@ ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew)
         }
         //else it is self-signed or untrusted do not attrach any certificate
 
-        Ssl::LocalContextStorage & ssl_ctx_cache(Ssl::TheGlobalContextStorage.getLocalStorage(port->s));
+        Ssl::LocalContextStorage *ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
         assert(sslBumpCertKey.defined() && sslBumpCertKey[0] != '\0');
         if (sslContext) {
-            if (!ssl_ctx_cache.add(sslBumpCertKey.termedBuf(), new Ssl::SSL_CTX_Pointer(sslContext))) {
+            if (!ssl_ctx_cache || !ssl_ctx_cache->add(sslBumpCertKey.termedBuf(), new Ssl::SSL_CTX_Pointer(sslContext))) {
                 // If it is not in storage delete after using. Else storage deleted it.
                 fd_table[clientConnection->fd].dynamicSslContext = sslContext;
             }

diff --git a/src/ssl/context_storage.cc b/src/ssl/context_storage.cc
index c1746849077d444a4209a3313d99e413cdb3a01f..d9cf3e3033b231b0446f8686fbf6e759b14d0f29 100644 (file)
--- a/src/ssl/context_storage.cc
+++ b/src/ssl/context_storage.cc
@@ -65,16 +65,20 @@ void Ssl::GlobalContextStorage::addLocalStorage(Ip::Address const & address, siz
     configureStorage.insert(std::pair<Ip::Address, size_t>(address, size_of_store));
 }
 
-Ssl::LocalContextStorage & Ssl::GlobalContextStorage::getLocalStorage(Ip::Address const & address)
+Ssl::LocalContextStorage *Ssl::GlobalContextStorage::getLocalStorage(Ip::Address const & address)
 {
     reconfigureFinish();
     std::map<Ip::Address, LocalContextStorage *>::iterator i = storage.find(address);
-    assert (i != storage.end());
-    return *(i->second);
+
+    if (i == storage.end())
+        return NULL;
+    else
+        return i->second;
 }
 
 void Ssl::GlobalContextStorage::reconfigureStart()
 {
+    configureStorage.clear();
     reconfiguring = true;
 }
 
@@ -86,7 +90,7 @@ void Ssl::GlobalContextStorage::reconfigureFinish()
         // remove or change old local storages.
         for (std::map<Ip::Address, LocalContextStorage *>::iterator i = storage.begin(); i != storage.end(); ++i) {
             std::map<Ip::Address, size_t>::iterator conf_i = configureStorage.find(i->first);
-            if (conf_i == configureStorage.end()) {
+            if (conf_i == configureStorage.end() || conf_i->second <= 0) {
                 storage.erase(i);
             } else {
                 i->second->setMemLimit(conf_i->second);
@@ -95,7 +99,7 @@ void Ssl::GlobalContextStorage::reconfigureFinish()
 
         // add new local storages.
         for (std::map<Ip::Address, size_t>::iterator conf_i = configureStorage.begin(); conf_i != configureStorage.end(); ++conf_i ) {
-            if (storage.find(conf_i->first) == storage.end()) {
+            if (storage.find(conf_i->first) == storage.end() && conf_i->second > 0) {
                 storage.insert(std::pair<Ip::Address, LocalContextStorage *>(conf_i->first, new LocalContextStorage(-1, conf_i->second)));
             }
         }

diff --git a/src/ssl/context_storage.h b/src/ssl/context_storage.h
index 59dd08c7d58e242417c2026798707e2599b2d491..6efe7580d0f85e3c53a6c739bc7479b1b5cf99a7 100644 (file)
--- a/src/ssl/context_storage.h
+++ b/src/ssl/context_storage.h
@@ -52,7 +52,7 @@ public:
     /// Create new SSL context storage for the local listening address/port.
     void addLocalStorage(Ip::Address const & address, size_t size_of_store);
     /// Return the local storage for the given listening address/port.
-    LocalContextStorage & getLocalStorage(Ip::Address const & address);
+    LocalContextStorage *getLocalStorage(Ip::Address const & address);
     /// When reconfigring should be called this method.
     void reconfigureStart();
 private:

diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc
index 85e871dae5ddcfae41fa2466aa995605c39ca713..3ab73aa33a4219e7daaa40eeac0edbbaaefb045f 100644 (file)
--- a/src/tests/stub_libsslsquid.cc
+++ b/src/tests/stub_libsslsquid.cc
@@ -27,8 +27,8 @@ Ssl::Config Ssl::TheConfig;
 Ssl::CertificateStorageAction::Pointer Ssl::CertificateStorageAction::Create(const Mgr::Command::Pointer &cmd) STUB_RETSTATREF(Ssl::CertificateStorageAction::Pointer)
 void Ssl::CertificateStorageAction::dump(StoreEntry *sentry) STUB
 void Ssl::GlobalContextStorage::addLocalStorage(Ip::Address const & address, size_t size_of_store) STUB
-Ssl::LocalContextStorage & Ssl::GlobalContextStorage::getLocalStorage(Ip::Address const & address)
-{ fatal(STUB_API " required"); static Ssl::LocalContextStorage v(0,0); return v; }
+Ssl::LocalContextStorage *Ssl::GlobalContextStorage::getLocalStorage(Ip::Address const & address)
+{ fatal(STUB_API " required"); static Ssl::LocalContextStorage v(0,0); return &v; }
 void Ssl::GlobalContextStorage::reconfigureStart() STUB
 //Ssl::GlobalContextStorage Ssl::TheGlobalContextStorage;