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.

diff --git a/src/main.cc b/src/main.cc
index 0a7949e1e157e218301617305e1b099094c23b75..482ea54b4ac2c23a18e691e73a33e4099d72c7e5 100644 (file)
--- 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 21e0b368b40a49d909d804b1a6cca210c8ad8196..7e8dc50d4ceddf2a9d2e672d7617db609af30231 100644 (file)
--- 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 (file)
index 0000000..38480c1
--- /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 51130d200fc2031ed3c6a4190cf86c2b29a98a45..a978fc329e1fafc252e28a0c755d21bbfd38d27e 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -33,9 +33,8 @@
 
 #include <cerrno>
 
-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 95da6b2cfb3c83e07bd380815f406d174018f4a9..2ece667acd1d7b5a13c6c277eecb17b444d2edf5 100644 (file)
--- 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<Ssl::CertError> 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 05038142d92a27a8891c7e642688f59583a9daa5..3cc147a58c00c7cc11b8488da474460cc9317f3c 100644 (file)
--- 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