Hi all,
While working on 4005 bug, we found some problems, which fixed with the
attached patch:
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.
The attached 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.
I must note that this patch changes the current behaviour of
dynamic_cert_mem_cache_size option of http_port:
Currently the size of cache is unlimited when it is set to 0, but this
patch disables the cache when this option set to 0.
Regards,
Christos
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
=== modified file 'src/base/LruMap.h'
--- src/base/LruMap.h 2013-05-16 07:06:50 +0000
+++ src/base/LruMap.h 2014-02-11 16:16:05 +0000
@@ -33,41 +33,41 @@
typedef typename std::list<Entry *>::iterator QueueIterator;
/// key:queue_item mapping for fast lookups by key
typedef std::map<std::string, QueueIterator> Map;
typedef typename Map::iterator MapIterator;
typedef std::pair<std::string, QueueIterator> MapPair;
LruMap(int ttl, size_t size);
~LruMap();
/// Search for an entry, and return a pointer
EntryValue *get(const char *key);
/// Add an entry to the map
bool add(const char *key, EntryValue *t);
/// Delete an entry from the map
bool del(const char *key);
/// (Re-)set the maximum size for this map
void setMemLimit(size_t aSize);
/// 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
int entries() const {return entries_;}
private:
LruMap(LruMap const &);
LruMap & operator = (LruMap const &);
bool expired(const Entry &e) const;
void trim();
void touch(const MapIterator &i);
bool del(const MapIterator &i);
void findEntry(const char *key, LruMap::MapIterator &i);
Map storage; ///< The Key/value * pairs
Queue index; ///< LRU cache index
int ttl;///< >0 ttl for caching, == 0 cache is disabled, < 0 store for ever
size_t memLimit_; ///< The maximum memory to use
int entries_; ///< The stored entries
};
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2014-02-10 09:59:19 +0000
+++ src/client_side.cc 2014-02-11 16:05:53 +0000
@@ -3844,52 +3844,53 @@
if (port->signPkey.get())
certProperties.signWithPkey.resetAndLock(port->signPkey.get());
}
signAlgorithm = certProperties.signAlgorithm;
}
void
ConnStateData::getSslContextStart()
{
assert(areAllContextsForThisConnection());
freeAllContexts();
/* careful: freeAllContexts() above frees request, host, etc. */
if (port->generateHostCertificates) {
Ssl::CertificateProperties certProperties;
buildSslCertGenerationParams(certProperties);
sslBumpCertKey = certProperties.dbKey().c_str();
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)) {
debugs(33, 5, HERE << "Cached SSL certificate for " << sslBumpCertKey << " is valid");
getSslContextDone(dynCtx);
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");
}
#if USE_SSL_CRTD
try {
debugs(33, 5, HERE << "Generating SSL certificate for " << certProperties.commonName << " using ssl_crtd.");
Ssl::CrtdMessage request_message(Ssl::CrtdMessage::REQUEST);
request_message.setCode(Ssl::CrtdMessage::code_new_certificate);
request_message.composeRequest(certProperties);
debugs(33, 5, HERE << "SSL crtd request: " << request_message.compose().c_str());
Ssl::Helper::GetInstance()->sslSubmit(request_message, sslCrtdHandleReplyWrapper, this);
return;
} catch (const std::exception &e) {
debugs(33, DBG_IMPORTANT, "ERROR: Failed to compose ssl_crtd " <<
"request for " << certProperties.commonName <<
" certificate: " << e.what() << "; will now block to " <<
"generate that certificate.");
// fall through to do blocking in-process generation.
@@ -3907,44 +3908,44 @@
void
ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew)
{
// Try to add generated ssl context to storage.
if (port->generateHostCertificates && isNew) {
if (signAlgorithm == Ssl::algSignTrusted) {
// Add signing certificate to the certificates chain
X509 *cert = port->signingCert.get();
if (SSL_CTX_add_extra_chain_cert(sslContext, cert)) {
// increase the certificate lock
CRYPTO_add(&(cert->references),1,CRYPTO_LOCK_X509);
} else {
const int ssl_error = ERR_get_error();
debugs(33, DBG_IMPORTANT, "WARNING: can not add signing certificate to SSL context chain: " << ERR_error_string(ssl_error, NULL));
}
Ssl::addChainToSslContext(sslContext, port->certsToChain.get());
}
//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;
}
} else {
debugs(33, 2, HERE << "Failed to generate SSL cert for " << sslConnectHostOrIp);
}
}
// If generated ssl context = NULL, try to use static ssl context.
if (!sslContext) {
if (!port->staticSslContext) {
debugs(83, DBG_IMPORTANT, "Closing SSL " << clientConnection->remote << " as lacking SSL context");
clientConnection->close();
return;
} else {
debugs(33, 5, HERE << "Using static ssl context.");
sslContext = port->staticSslContext.get();
}
}
=== modified file 'src/ssl/context_storage.cc'
--- src/ssl/context_storage.cc 2013-10-25 00:13:46 +0000
+++ src/ssl/context_storage.cc 2014-02-11 17:01:58 +0000
@@ -48,58 +48,62 @@
Ssl::GlobalContextStorage::GlobalContextStorage()
: reconfiguring(true)
{
RegisterAction("cached_ssl_cert", "Statistic of cached generated ssl certificates", &CertificateStorageAction::Create, 0, 1);
}
Ssl::GlobalContextStorage::~GlobalContextStorage()
{
for (std::map<Ip::Address, LocalContextStorage *>::iterator i = storage.begin(); i != storage.end(); ++i) {
delete i->second;
}
}
void Ssl::GlobalContextStorage::addLocalStorage(Ip::Address const & address, size_t size_of_store)
{
assert(reconfiguring);
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;
}
void Ssl::GlobalContextStorage::reconfigureFinish()
{
if (reconfiguring) {
reconfiguring = false;
// 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);
}
}
// 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)));
}
}
}
}
Ssl::GlobalContextStorage Ssl::TheGlobalContextStorage;
=== modified file 'src/ssl/context_storage.h'
--- src/ssl/context_storage.h 2013-10-25 00:13:46 +0000
+++ src/ssl/context_storage.h 2014-02-11 16:06:08 +0000
@@ -35,39 +35,39 @@
virtual void dump (StoreEntry *sentry);
/**
* We do not support aggregation of information across workers
* TODO: aggregate these stats
*/
virtual bool aggregatable() const { return false; }
};
typedef LruMap<SSL_CTX_Pointer, SSL_CTX_SIZE> LocalContextStorage;
/// Class for storing/manipulating LocalContextStorage per local listening address/port.
class GlobalContextStorage
{
friend class CertificateStorageAction;
public:
GlobalContextStorage();
~GlobalContextStorage();
/// 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:
/// Called by getLocalStorage method
void reconfigureFinish();
bool reconfiguring; ///< True if system reconfiguring now.
/// Storage used on configure or reconfigure.
std::map<Ip::Address, size_t> configureStorage;
/// Map for storing all local ip address and their local storages.
std::map<Ip::Address, LocalContextStorage *> storage;
};
/// Global cache for store all SSL server certificates.
extern GlobalContextStorage TheGlobalContextStorage;
} //namespace Ssl
#endif // USE_SSL
#endif // SQUID_SSL_CONTEXT_STORAGE_H
