From: Christos Tsantilas Date: Tue, 4 Mar 2014 10:14:42 +0000 (-0700) Subject: dynamic_cert_mem_cache_size option related fixes X-Git-Tag: SQUID_3_4_4~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=60338839faf92ff1cc53b515968c426c83395496;p=thirdparty%2Fsquid.git 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 39eb44c82d..c558666812 100644 --- 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 c1d13e6a38..3d9614df08 100644 --- 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 c174684907..d9cf3e3033 100644 --- 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(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::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::iterator i = storage.begin(); i != storage.end(); ++i) { std::map::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::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(conf_i->first, new LocalContextStorage(-1, conf_i->second))); } } diff --git a/src/ssl/context_storage.h b/src/ssl/context_storage.h index 59dd08c7d5..6efe7580d0 100644 --- 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 85e871dae5..3ab73aa33a 100644 --- 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;