From 9873e378da91cf253e4b476ad3fc9fce24ef350c Mon Sep 17 00:00:00 2001 From: Christos Tsantilas Date: Wed, 19 Feb 2014 19:53:27 +0200 Subject: [PATCH] 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 --- src/base/LruMap.h | 2 +- src/client_side.cc | 11 ++++++----- src/ssl/context_storage.cc | 14 +++++++++----- src/ssl/context_storage.h | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-) 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 fad7390cca..24eb75f5f0 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3861,9 +3861,9 @@ ConnStateData::getSslContextStart() assert(sslBumpCertKey.size() > 0 && 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)) { @@ -3872,7 +3872,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"); @@ -3924,10 +3925,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.size() > 0 && 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 2753d590c0..ac84647e4d 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 20a6436234..52f6da97dd 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: -- 2.47.2