TLS: Session caching configuration support. (#7420)

* TLS: Session caching configuration support.
* TLS: Remove redundant config initialization.


Former-commit-id: d3834c50699bc4f31f381d6d03d4c1b022380895

TLS.md

@@ -56,8 +56,6 @@ Note that unlike Redis, KeyDB fully supports multithreading of TLS connections.
 To-Do List
 ----------
 
-- [ ] Add session caching support. Check if/how it's handled by clients to
-  assess how useful/important it is.
 - [ ] redis-benchmark support. The current implementation is a mix of using
   hiredis for parsing and basic networking (establishing connections), but
   directly manipulating sockets for most actions. This will need to be cleaned

keydb.conf

@@ -199,6 +199,22 @@ tcp-keepalive 300
 #
 # tls-prefer-server-ciphers yes
 
+# By default, TLS session caching is enabled to allow faster and less expensive
+# reconnections by clients that support it. Use the following directive to disable
+# caching.
+#
+# tls-session-caching no
+
+# Change the default number of TLS sessions cached. A zero value sets the cache
+# to unlimited size. The default size is 20480.
+#
+# tls-session-cache-size 5000
+
+# Change the default timeout of cached TLS sessions. The default timeout is 300
+# seconds.
+#
+# tls-session-cache-timeout 60
+
 ################################# GENERAL #####################################
 
 # By default KeyDB does not run as a daemon. Use 'yes' if you need it.

src/aelocker.h

@@ -9,11 +9,14 @@ public:
     {
     }
 
-    void arm(client *c) // if a client is passed, then the client is already locked
+    void arm(client *c, bool fIfNeeded = false) // if a client is passed, then the client is already locked
     {
         if (m_fArmed)
             return;
         
+        if (fIfNeeded && aeThreadOwnsLock())
+            return;
+
         serverAssertDebug(!GlobalLocksAcquired());
 
         if (c != nullptr)

src/config.cpp

@@ -2180,7 +2180,7 @@ static int updateTlsCfg(char *val, char *prev, const char **err) {
     UNUSED(prev);
     UNUSED(err);
     if (tlsConfigure(&g_pserver->tls_ctx_config) == C_ERR) {
-        *err = "Unable to configure tls-cert-file. Check server logs.";
+        *err = "Unable to update TLS configuration. Check server logs.";
         return 0;
     }
     return 1;
@@ -2190,6 +2190,12 @@ static int updateTlsCfgBool(int val, int prev, const char **err) {
     UNUSED(prev);
     return updateTlsCfg(NULL, NULL, err);
 }
+
+static int updateTlsCfgInt(long long val, long long prev, const char **err) {
+    UNUSED(val);
+    UNUSED(prev);
+    return updateTlsCfg(NULL, NULL, err);
+}
 #endif  /* USE_OPENSSL */
 
 int fDummy = false;
@@ -2324,10 +2330,13 @@ standardConfig configs[] = {
 
 #ifdef USE_OPENSSL
     createIntConfig("tls-port", NULL, IMMUTABLE_CONFIG, 0, 65535, g_pserver->tls_port, 0, INTEGER_CONFIG, NULL, NULL), /* TCP port. */
+    createIntConfig("tls-session-cache-size", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, g_pserver->tls_ctx_config.session_cache_size, 20*1024, INTEGER_CONFIG, NULL, updateTlsCfgInt),
+    createIntConfig("tls-session-cache-timeout", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, g_pserver->tls_ctx_config.session_cache_timeout, 300, INTEGER_CONFIG, NULL, updateTlsCfgInt),
     createBoolConfig("tls-cluster", NULL, MODIFIABLE_CONFIG, g_pserver->tls_cluster, 0, NULL, NULL),
     createBoolConfig("tls-replication", NULL, MODIFIABLE_CONFIG, g_pserver->tls_replication, 0, NULL, NULL),
     createBoolConfig("tls-auth-clients", NULL, MODIFIABLE_CONFIG, g_pserver->tls_auth_clients, 1, NULL, NULL),
     createBoolConfig("tls-prefer-server-ciphers", NULL, MODIFIABLE_CONFIG, g_pserver->tls_ctx_config.prefer_server_ciphers, 0, NULL, updateTlsCfgBool),
+    createBoolConfig("tls-session-caching", NULL, MODIFIABLE_CONFIG, g_pserver->tls_ctx_config.session_caching, 1, NULL, updateTlsCfgBool),
     createStringConfig("tls-cert-file", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, g_pserver->tls_ctx_config.cert_file, NULL, NULL, updateTlsCfg),
     createStringConfig("tls-key-file", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, g_pserver->tls_ctx_config.key_file, NULL, NULL, updateTlsCfg),
     createStringConfig("tls-dh-params-file", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, g_pserver->tls_ctx_config.dh_params_file, NULL, NULL, updateTlsCfg),

src/networking.cpp

@@ -2308,8 +2308,9 @@ void commandProcessed(client *c) {
 int processCommandAndResetClient(client *c, int flags) {
     int deadclient = 0;
     serverTL->current_client = c;
-    AeLocker locker;
+    serverAssert(GlobalLocksAcquired());
-    if (processCommand(c, flags, locker) == C_OK) {
+    
+    if (processCommand(c, flags) == C_OK) {
         commandProcessed(c);
     }
     if (serverTL->current_client == NULL) deadclient = 1;
@@ -2465,7 +2466,8 @@ void readQueryFromClient(connection *conn) {
 
 void processClients()
 {
-    aeAcquireLock();
+    serverAssert(GlobalLocksAcquired());
+
     for (client *c : serverTL->vecclientsProcess) {
         /* There is more data in the client input buffer, continue parsing it
         * in case to check if there is a full command to execute. */
@@ -2477,7 +2479,6 @@ void processClients()
     {
         ProcessPendingAsyncWrites();
     }
-    aeReleaseLock();
     
     serverTL->vecclientsProcess.clear();
 }
@@ -3437,7 +3438,7 @@ void processEventsWhileBlocked(int iel) {
     
 
     aeReleaseLock();
-    serverAssertDebug(!GlobalLocksAcquired());
+    serverAssert(!GlobalLocksAcquired());
     try
     {
         while (iterations--) {

src/server.cpp

@@ -3636,12 +3636,12 @@ void call(client *c, int flags) {
  * If C_OK is returned the client is still alive and valid and
  * other operations can be performed by the caller. Otherwise
  * if C_ERR is returned the client was destroyed (i.e. after QUIT). */
-int processCommand(client *c, int callFlags, AeLocker &locker) {
+int processCommand(client *c, int callFlags) {
     AssertCorrectThread(c);
+    serverAssert(GlobalLocksAcquired());
 
     if (moduleHasCommandFilters())
     {
-        locker.arm(c);
         moduleCallCommandFilters(c);
     }
 
@@ -3693,9 +3693,6 @@ int processCommand(client *c, int callFlags, AeLocker &locker) {
 
     /* Check if the user can run this command according to the current
      * ACLs. */
-    if (c->puser && !(c->puser->flags & USER_FLAG_ALLCOMMANDS))
-        locker.arm(c);  // ACLs require the lock
-
     int acl_keypos;
     int acl_retval = ACLCheckCommandPerm(c,&acl_keypos);
     if (acl_retval != ACL_OK) {
@@ -3723,7 +3720,6 @@ int processCommand(client *c, int callFlags, AeLocker &locker) {
         !(c->cmd->getkeys_proc == NULL && c->cmd->firstkey == 0 &&
           c->cmd->proc != execCommand))
     {
-        locker.arm(c);
         int hashslot;
         int error_code;
         clusterNode *n = getNodeByQuery(c,c->cmd,c->argv,c->argc,
@@ -3739,9 +3735,6 @@ int processCommand(client *c, int callFlags, AeLocker &locker) {
         }
     }
 
-    if (!locker.isArmed())
-        locker.arm(c);
-
     incrementMvccTstamp();
 
     /* Handle the maxmemory directive.

src/server.h

@@ -1528,6 +1528,9 @@ typedef struct redisTLSContextConfig {
     char *ciphers;
     char *ciphersuites;
     int prefer_server_ciphers;
+    int session_caching;
+    int session_cache_size;
+    int session_cache_timeout;
 } redisTLSContextConfig;
 
 /*-----------------------------------------------------------------------------
@@ -2564,7 +2567,7 @@ int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *lev
 size_t freeMemoryGetNotCountedMemory();
 int freeMemoryIfNeeded(void);
 int freeMemoryIfNeededAndSafe(void);
-int processCommand(client *c, int callFlags, class AeLocker &locker);
+int processCommand(client *c, int callFlags);
 void setupSignalHandlers(void);
 struct redisCommand *lookupCommand(sds name);
 struct redisCommand *lookupCommandByCString(const char *s);

src/tls.cpp

@@ -150,8 +150,6 @@ void tlsInit(void) {
         serverLog(LL_WARNING, "OpenSSL: Failed to seed random number generator.");
     }
 
-    /* Server configuration */
-    g_pserver->tls_auth_clients = 1;    /* Secure by default */
     tlsInitThread();
 }
 
@@ -193,6 +191,15 @@ int tlsConfigure(redisTLSContextConfig *ctx_config) {
     SSL_CTX_set_options(ctx, SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS);
 #endif
 
+    if (ctx_config->session_caching) {
+        SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_SERVER);
+        SSL_CTX_sess_set_cache_size(ctx, ctx_config->session_cache_size);
+        SSL_CTX_set_timeout(ctx, ctx_config->session_cache_timeout);
+        SSL_CTX_set_session_id_context(ctx, (const unsigned char*) "KeyDB", 5);
+    } else {
+        SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_OFF);
+    }
+
     protocols = parseProtocolsConfig(ctx_config->protocols);
     if (protocols == -1) goto error;
 

tests/unit/introspection.tcl

@@ -78,17 +78,8 @@ start_server {tags {"introspection"}} {
             syslog-facility
             databases
             port
-            io-threads
             tls-port
-            tls-prefer-server-ciphers
+            io-threads
-            tls-cert-file
-            tls-key-file
-            tls-dh-params-file
-            tls-ca-cert-file
-            tls-ca-cert-dir
-            tls-protocols
-            tls-ciphers
-            tls-ciphersuites
             logfile
             unixsocketperm
             slaveof
@@ -101,6 +92,23 @@ start_server {tags {"introspection"}} {
             bgsave_cpulist
         }
 
+        if {!$::tls} {
+            append skip_configs {
+                tls-prefer-server-ciphers
+                tls-session-cache-timeout
+                tls-session-cache-size
+                tls-session-caching
+                tls-cert-file
+                tls-key-file
+                tls-dh-params-file
+                tls-ca-cert-file
+                tls-ca-cert-dir
+                tls-protocols
+                tls-ciphers
+                tls-ciphersuites
+            }
+        }
+
         set configs {}
         foreach {k v} [r config get *] {
             if {[lsearch $skip_configs $k] != -1} {