From 824d4656ffebfdd4289adbdcde1f8be42795350b Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Thu, 18 Feb 2016 10:03:29 +1300 Subject: [PATCH] SourceLayout: Move the Runner which manages SSL SessionCache to libsecurity Unfortunately the OpenSSL session cache callbacks cannot also be moved due to circular dependency issues. However, when those are resolved by later libsecurity API additions the callbacks will be much easier to shift. For now the three symbols shared between the two libraries are exposed by libsslsquid in the Ssl:: namespace. Cache initialization is now moved into the Runner. Binding its state initialization more tightly to the memory allocation and initialization. Which also removes the need for explicit main.cc dependency. One issue was uncovered during this: * While ssl/support.h was defining a destruct_session_cache() function that appeared to release the cache memory, it was not actually being used anywhere. Which unless a fortuitous sequence of events is happening means that the memory for the cache entries may not be released properly. On the other hand the cache should only be erased on shutdown so the effects of this are minor. The unused function has been removed and the issue is now expicitly noted in the Runner shutdown handling method for future investigation. --- src/main.cc | 3 - src/security/Makefile.am | 1 + src/security/Session.cc | 105 +++++++++++++++++++++++++++++ src/ssl/support.cc | 122 +++++----------------------------- src/ssl/support.h | 20 +++--- src/tests/stub_libsslsquid.cc | 2 - 6 files changed, 131 insertions(+), 122 deletions(-) create mode 100644 src/security/Session.cc diff --git a/src/main.cc b/src/main.cc index 0a7949e1e1..482ea54b4a 100644 --- a/src/main.cc +++ b/src/main.cc @@ -1152,9 +1152,6 @@ mainInitialize(void) #endif #if USE_OPENSSL - if (!configured_once) - Ssl::initialize_session_cache(); - if (Ssl::CertValidationHelper::GetInstance()) Ssl::CertValidationHelper::GetInstance()->Init(); #endif diff --git a/src/security/Makefile.am b/src/security/Makefile.am index 21e0b368b4..7e8dc50d4c 100644 --- a/src/security/Makefile.am +++ b/src/security/Makefile.am @@ -25,4 +25,5 @@ libsecurity_la_SOURCES= \ PeerOptions.h \ ServerOptions.cc \ ServerOptions.h \ + Session.cc \ Session.h diff --git a/src/security/Session.cc b/src/security/Session.cc new file mode 100644 index 0000000000..38480c1438 --- /dev/null +++ b/src/security/Session.cc @@ -0,0 +1,105 @@ +/* + * Copyright (C) 1996-2016 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#include "squid.h" +#include "anyp/PortCfg.h" +#include "base/RunnersRegistry.h" +#include "ipc/MemMap.h" +#include "security/Session.h" +#include "SquidConfig.h" + +#define SSL_SESSION_ID_SIZE 32 +#define SSL_SESSION_MAX_SIZE 10*1024 + +static bool +isTlsServer() +{ + for (AnyP::PortCfgPointer s = HttpPortList; s != nullptr; s = s->next) { + if (s->secure.encryptTransport) + return true; + if (s->flags.tunnelSslBumping) + return true; + } + + return false; +} + +void +initializeSessionCache() +{ +#if USE_OPENSSL + // Check if the MemMap keys and data are enough big to hold + // session ids and session data + assert(SSL_SESSION_ID_SIZE >= MEMMAP_SLOT_KEY_SIZE); + assert(SSL_SESSION_MAX_SIZE >= MEMMAP_SLOT_DATA_SIZE); + + int configuredItems = ::Config.SSL.sessionCacheSize / sizeof(Ipc::MemMap::Slot); + if (IamWorkerProcess() && configuredItems) + Ssl::SessionCache = new Ipc::MemMap(Ssl::SessionCacheName); + else { + Ssl::SessionCache = nullptr; + return; + } + + for (AnyP::PortCfgPointer s = HttpPortList; s != nullptr; s = s->next) { + if (s->secure.staticContext.get()) + Ssl::SetSessionCallbacks(s->secure.staticContext.get()); + } +#endif +} + +/// initializes shared memory segments used by MemStore +class SharedSessionCacheRr: public Ipc::Mem::RegisteredRunner +{ +public: + /* RegisteredRunner API */ + SharedSessionCacheRr(): owner(nullptr) {} + virtual void useConfig(); + virtual ~SharedSessionCacheRr(); + +protected: + virtual void create(); + +private: + Ipc::MemMap::Owner *owner; +}; + +RunnerRegistrationEntry(SharedSessionCacheRr); + +void +SharedSessionCacheRr::useConfig() +{ +#if USE_OPENSSL // while Ssl:: bits in use + if (Ssl::SessionCache || !isTlsServer()) //no need to configure ssl session cache. + return; + + Ipc::Mem::RegisteredRunner::useConfig(); + initializeSessionCache(); +#endif +} + +void +SharedSessionCacheRr::create() +{ + if (!isTlsServer()) //no need to configure ssl session cache. + return; + +#if USE_OPENSSL // while Ssl:: bits in use + if (int items = Config.SSL.sessionCacheSize / sizeof(Ipc::MemMap::Slot)) + owner = Ipc::MemMap::Init(Ssl::SessionCacheName, items); +#endif +} + +SharedSessionCacheRr::~SharedSessionCacheRr() +{ + // XXX: Enable after testing to reduce at-exit memory "leaks". + // delete Ssl::SessionCache; + + delete owner; +} + diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 51130d200f..a978fc329e 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -33,9 +33,8 @@ #include -static void setSessionCallbacks(Security::ContextPtr ctx); -Ipc::MemMap *SslSessionCache = NULL; -const char *SslSessionCacheName = "ssl_session_cache"; +Ipc::MemMap *Ssl::SessionCache = NULL; +const char *Ssl::SessionCacheName = "ssl_session_cache"; static Ssl::CertsIndexedList SquidUntrustedCerts; @@ -552,7 +551,7 @@ configureSslContext(Security::ContextPtr sslContext, AnyP::PortCfg &port) if (port.secure.parsedFlags & SSL_FLAG_DONT_VERIFY_DOMAIN) SSL_CTX_set_ex_data(sslContext, ssl_ctx_ex_index_dont_verify_domain, (void *) -1); - setSessionCallbacks(sslContext); + Ssl::SetSessionCallbacks(sslContext); return true; } @@ -1368,7 +1367,7 @@ Ssl::CertError::operator != (const CertError &ce) const static int store_session_cb(SSL *ssl, SSL_SESSION *session) { - if (!SslSessionCache) + if (!Ssl::SessionCache) return 0; debugs(83, 5, "Request to store SSL Session "); @@ -1384,7 +1383,7 @@ store_session_cb(SSL *ssl, SSL_SESSION *session) memset(key, 0, sizeof(key)); memcpy(key, id, idlen); int pos; - Ipc::MemMap::Slot *slotW = SslSessionCache->openForWriting((const cache_key*)key, pos); + Ipc::MemMap::Slot *slotW = Ssl::SessionCache->openForWriting((const cache_key*)key, pos); if (slotW) { int lenRequired = i2d_SSL_SESSION(session, NULL); if (lenRequired < MEMMAP_SLOT_DATA_SIZE) { @@ -1392,7 +1391,7 @@ store_session_cb(SSL *ssl, SSL_SESSION *session) lenRequired = i2d_SSL_SESSION(session, &p); slotW->set(key, NULL, lenRequired, squid_curtime + Config.SSL.session_ttl); } - SslSessionCache->closeForWriting(pos); + Ssl::SessionCache->closeForWriting(pos); debugs(83, 5, "wrote an ssl session entry of size " << lenRequired << " at pos " << pos); } return 0; @@ -1401,27 +1400,27 @@ store_session_cb(SSL *ssl, SSL_SESSION *session) static void remove_session_cb(SSL_CTX *, SSL_SESSION *sessionID) { - if (!SslSessionCache) + if (!Ssl::SessionCache) return ; debugs(83, 5, "Request to remove corrupted or not valid SSL Session "); int pos; - Ipc::MemMap::Slot const *slot = SslSessionCache->openForReading((const cache_key*)sessionID, pos); + Ipc::MemMap::Slot const *slot = Ssl::SessionCache->openForReading((const cache_key*)sessionID, pos); if (slot == NULL) return; - SslSessionCache->closeForReading(pos); + Ssl::SessionCache->closeForReading(pos); // TODO: // What if we are not able to remove the session? // Maybe schedule a job to remove it later? // For now we just have an invalid entry in cache until will be expired // The openSSL will reject it when we try to use it - SslSessionCache->free(pos); + Ssl::SessionCache->free(pos); } static SSL_SESSION * get_session_cb(SSL *, unsigned char *sessionID, int len, int *copy) { - if (!SslSessionCache) + if (!Ssl::SessionCache) return NULL; SSL_SESSION *session = NULL; @@ -1431,7 +1430,7 @@ get_session_cb(SSL *, unsigned char *sessionID, int len, int *copy) len << p[0] << ":" << p[1]); int pos; - Ipc::MemMap::Slot const *slot = SslSessionCache->openForReading((const cache_key*)sessionID, pos); + Ipc::MemMap::Slot const *slot = Ssl::SessionCache->openForReading((const cache_key*)sessionID, pos); if (slot != NULL) { if (slot->expire > squid_curtime) { const unsigned char *ptr = slot->p; @@ -1439,7 +1438,7 @@ get_session_cb(SSL *, unsigned char *sessionID, int len, int *copy) debugs(83, 5, "Session retrieved from cache at pos " << pos); } else debugs(83, 5, "Session in cache expired"); - SslSessionCache->closeForReading(pos); + Ssl::SessionCache->closeForReading(pos); } if (!session) @@ -1453,10 +1452,10 @@ get_session_cb(SSL *, unsigned char *sessionID, int len, int *copy) return session; } -static void -setSessionCallbacks(Security::ContextPtr ctx) +void +Ssl::SetSessionCallbacks(Security::ContextPtr ctx) { - if (SslSessionCache) { + if (Ssl::SessionCache) { SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_SERVER|SSL_SESS_CACHE_NO_INTERNAL); SSL_CTX_sess_set_new_cb(ctx, store_session_cb); SSL_CTX_sess_set_remove_cb(ctx, remove_session_cb); @@ -1464,94 +1463,5 @@ setSessionCallbacks(Security::ContextPtr ctx) } } -static bool -isSslServer() -{ - for (AnyP::PortCfgPointer s = HttpPortList; s != NULL; s = s->next) { - if (s->secure.encryptTransport) - return true; - if (s->flags.tunnelSslBumping) - return true; - } - - return false; -} - -#define SSL_SESSION_ID_SIZE 32 -#define SSL_SESSION_MAX_SIZE 10*1024 - -void -Ssl::initialize_session_cache() -{ - - if (!isSslServer()) //no need to configure ssl session cache. - return; - - // Check if the MemMap keys and data are enough big to hold - // session ids and session data - assert(SSL_SESSION_ID_SIZE >= MEMMAP_SLOT_KEY_SIZE); - assert(SSL_SESSION_MAX_SIZE >= MEMMAP_SLOT_DATA_SIZE); - - int configuredItems = ::Config.SSL.sessionCacheSize / sizeof(Ipc::MemMap::Slot); - if (IamWorkerProcess() && configuredItems) - SslSessionCache = new Ipc::MemMap(SslSessionCacheName); - else { - SslSessionCache = NULL; - return; - } - - for (AnyP::PortCfgPointer s = HttpPortList; s != NULL; s = s->next) { - if (s->secure.staticContext.get()) - setSessionCallbacks(s->secure.staticContext.get()); - } -} - -void -destruct_session_cache() -{ - delete SslSessionCache; -} - -/// initializes shared memory segments used by MemStore -class SharedSessionCacheRr: public Ipc::Mem::RegisteredRunner -{ -public: - /* RegisteredRunner API */ - SharedSessionCacheRr(): owner(NULL) {} - virtual void useConfig(); - virtual ~SharedSessionCacheRr(); - -protected: - virtual void create(); - -private: - Ipc::MemMap::Owner *owner; -}; - -RunnerRegistrationEntry(SharedSessionCacheRr); - -void -SharedSessionCacheRr::useConfig() -{ - Ipc::Mem::RegisteredRunner::useConfig(); -} - -void -SharedSessionCacheRr::create() -{ - if (!isSslServer()) //no need to configure ssl session cache. - return; - - int items; - items = Config.SSL.sessionCacheSize / sizeof(Ipc::MemMap::Slot); - if (items) - owner = Ipc::MemMap::Init(SslSessionCacheName, items); -} - -SharedSessionCacheRr::~SharedSessionCacheRr() -{ - delete owner; -} - #endif /* USE_OPENSSL */ diff --git a/src/ssl/support.h b/src/ssl/support.h index 95da6b2cfb..2ece667acd 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -56,6 +56,11 @@ namespace AnyP class PortCfg; }; +namespace Ipc +{ +class MemMap; +} + namespace Ssl { /// initialize the SSL library global state. @@ -102,6 +107,10 @@ public: /// Holds a list of certificate SSL errors typedef CbDataList CertErrors; +void SetSessionCallbacks(Security::ContextPtr); +extern Ipc::MemMap *SessionCache; +extern const char *SessionCacheName; + } //namespace Ssl /// \ingroup ServerProtocolSSLAPI @@ -304,17 +313,6 @@ int asn1timeToString(ASN1_TIME *tm, char *buf, int len); */ bool setClientSNI(SSL *ssl, const char *fqdn); -/** - \ingroup ServerProtocolSSLAPI - * Initializes the shared session cache if configured -*/ -void initialize_session_cache(); - -/** - \ingroup ServerProtocolSSLAPI - * Destroy the shared session cache if configured -*/ -void destruct_session_cache(); } //namespace Ssl #if _SQUID_WINDOWS_ diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index 05038142d9..3cc147a58c 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -82,8 +82,6 @@ int matchX509CommonNames(X509 *peer_cert, void *check_data, int (*check_func)(vo bool checkX509ServerValidity(X509 *cert, const char *server) STUB_RETVAL(false) int asn1timeToString(ASN1_TIME *tm, char *buf, int len) STUB_RETVAL(0) bool setClientSNI(SSL *ssl, const char *fqdn) STUB_RETVAL(false) -void initialize_session_cache() STUB -void destruct_session_cache() STUB } //namespace Ssl #endif -- 2.47.2