TLS: Configuration options.

Add configuration options for TLS protocol versions, ciphers/cipher
suites selection, etc.

TLS.md

@@ -48,45 +48,35 @@ both TCP and TLS available, but you'll need to assign different ports.
 To make a Replica connect to the master using TLS, use `--tls-replication yes`,
 and to make Redis Cluster use TLS across nodes use `--tls-cluster yes`.
 
-**NOTE: This is still very much work in progress and some configuration is still
-missing or may change.**
-
 Connections
 -----------
 
-Connection abstraction API is mostly done and seems to hold well for hiding
+All socket operations now go through a connection abstraction layer that hides
-implementation details between TLS and TCP.
+I/O and read/write event handling from the caller.
 
-1. Multi-threading I/O is not supported.  The main issue to address is the need
+**Multi-threading I/O is not currently supported for TLS**, as a TLS connection
-   to manipulate AE based on OpenSSL return codes.  We can either propagate this
+needs to do its own manipulation of AE events which is not thread safe. The
-   out of the thread, or explore ways of further optimizing MT I/O by having
+solution is probably to manage independent AE loops for I/O threads and longer
-   event loops that live inside the thread and borrow connections in/out.
+term association of connections with threads. This may potentially improve
+overall performance as well.
 
-2. Finish cleaning up the implementation.  Make sure all error cases are handled
+Sync IO for TLS is currently implemented in a hackish way, i.e. making the
-   and reflected into connection state, connection state validated before
+socket blocking and configuring socket-level timeout.  This means the timeout
-   certain operations, etc.
+value may not be so accurate, and there would be a lot of syscall overhead.
-    - Clean (non-errno) interface to report would-block.
+However I believe that getting rid of syncio completely in favor of pure async
-    - Consistent error reporting.
+work is probably a better move than trying to fix that. For replication it would
+probably not be so hard. For cluster keys migration it might be more difficult,
+but there are probably other good reasons to improve that part anyway.
 
-3. Sync IO for TLS is currently implemented in a hackish way, i.e. making the
+To-Do List
-   socket blocking and configuring socket-level timeout.  This means the timeout
+==========
-   value may not be so accurate, and there would be a lot of syscall overhead.
-   However I believe that getting rid of syncio completely in favor of pure
-   async work is probably a better move than trying to fix that. For replication
-   it would probably not be so hard. For cluster keys migration it might be more
-   difficult, but there are probably other good reasons to improve that part
-   anyway.
 
-TLS Features
+Additional TLS Features
-------------
+-----------------------
-
-1. Add metrics to INFO.
-2. Add certificate authentication configuration (i.e. option to skip client
-auth, master auth, etc.).
-3. Add TLS cipher configuration options.
-4. [Optional] Add session caching support. Check if/how it's handled by clients
-   to assess how useful/important it is.
 
+1. Add metrics to INFO?
+2. Add session caching support. Check if/how it's handled by clients to assess
+   how useful/important it is.
 
 redis-benchmark
 ---------------
@@ -100,8 +90,8 @@ probably to migrate to hiredis async mode.
 
 redis-cli
 ---------
-1. Support tls in --slave and --rdb
 
+1. Add support for TLS in --slave and --rdb modes.
 
 Others
 ------

redis.conf

@@ -173,6 +173,30 @@ tcp-keepalive 300
 #
 # tls-cluster yes
 
+# Explicitly specify TLS versions to support. Allowed values are case insensitive
+# and include "TLSv1", "TLSv1.1", "TLSv1.2", "TLSv1.3" (OpenSSL >= 1.1.1) or
+# "default" which is currently >= TLSv1.1.
+#
+# tls-protocols TLSv1.2
+
+# Configure allowed ciphers.  See the ciphers(1ssl) manpage for more information
+# about the syntax of this string.
+#
+# Note: this configuration applies only to <= TLSv1.2.
+#
+# tls-ciphers DEFAULT:!MEDIUM
+
+# Configure allowed TLSv1.3 ciphersuites.  See the ciphers(1ssl) manpage for more
+# information about the syntax of this string, and specifically for TLSv1.3
+# ciphersuites.
+#
+# tls-ciphersuites TLS_CHACHA20_POLY1305_SHA256
+
+# When choosing a cipher, use the server's preference instead of the client
+# preference. By default, the server follows the client's preference.
+#
+# tls-prefer-server-cipher yes
+
 ################################# GENERAL #####################################
 
 # By default Redis does not run as a daemon. Use 'yes' if you need it.

src/Makefile

@@ -93,6 +93,8 @@ else
 ifeq ($(uname_S),Darwin)
 	# Darwin
 	FINAL_LIBS+= -ldl
+	OPENSSL_CFLAGS=-I/usr/local/opt/openssl/include
+	OPENSSL_LDFLAGS=-L/usr/local/opt/openssl/lib
 else
 ifeq ($(uname_S),AIX)
         # AIX

src/config.c

@@ -219,7 +219,7 @@ void queueLoadModule(sds path, sds *argv, int argc) {
 }
 
 void loadServerConfigFromString(char *config) {
-    char *err = NULL;
+    const char *err = NULL;
     int linenum = 0, totlines, i;
     int slaveof_linenum = 0;
     sds *lines;
@@ -286,15 +286,6 @@ void loadServerConfigFromString(char *config) {
             if (server.port < 0 || server.port > 65535) {
                 err = "Invalid port"; goto loaderr;
             }
-        } else if (!strcasecmp(argv[0],"tls-port") && argc == 2) {
-#ifdef USE_OPENSSL
-            server.tls_port = atoi(argv[1]);
-            if (server.port < 0 || server.port > 65535) {
-                err = "Invalid port"; goto loaderr;
-            }
-#else
-            err = "TLS not supported"; goto loaderr;
-#endif
         } else if (!strcasecmp(argv[0],"tcp-backlog") && argc == 2) {
             server.tcp_backlog = atoi(argv[1]);
             if (server.tcp_backlog < 0) {
@@ -806,24 +797,42 @@ void loadServerConfigFromString(char *config) {
                 err = sentinelHandleConfiguration(argv+1,argc-1);
                 if (err) goto loaderr;
             }
-        } else if (!strcasecmp(argv[0],"tls-cert-file") && argc == 2) {
+#ifdef USE_OPENSSL
-            zfree(server.tls_cert_file);
+        } else if (!strcasecmp(argv[0],"tls-port") && argc == 2) {
-            server.tls_cert_file = zstrdup(argv[1]);
+            server.tls_port = atoi(argv[1]);
-        } else if (!strcasecmp(argv[0],"tls-key-file") && argc == 2) {
+            if (server.port < 0 || server.port > 65535) {
-            zfree(server.tls_key_file);
+                err = "Invalid tls-port"; goto loaderr;
-            server.tls_key_file = zstrdup(argv[1]);
+            }
-        } else if (!strcasecmp(argv[0],"tls-dh-params-file") && argc == 2) {
-            zfree(server.tls_dh_params_file);
-            server.tls_dh_params_file = zstrdup(argv[1]);
-        } else if (!strcasecmp(argv[0],"tls-ca-cert-file") && argc == 2) {
-            zfree(server.tls_ca_cert_file);
-            server.tls_ca_cert_file = zstrdup(argv[1]);
         } else if (!strcasecmp(argv[0],"tls-cluster") && argc == 2) {
             server.tls_cluster = yesnotoi(argv[1]);
         } else if (!strcasecmp(argv[0],"tls-replication") && argc == 2) {
             server.tls_replication = yesnotoi(argv[1]);
         } else if (!strcasecmp(argv[0],"tls-auth-clients") && argc == 2) {
             server.tls_auth_clients = yesnotoi(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-cert-file") && argc == 2) {
+            zfree(server.tls_ctx_config.cert_file);
+            server.tls_ctx_config.cert_file = zstrdup(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-key-file") && argc == 2) {
+            zfree(server.tls_ctx_config.key_file);
+            server.tls_ctx_config.key_file = zstrdup(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-dh-params-file") && argc == 2) {
+            zfree(server.tls_ctx_config.dh_params_file);
+            server.tls_ctx_config.dh_params_file = zstrdup(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-ca-cert-file") && argc == 2) {
+            zfree(server.tls_ctx_config.ca_cert_file);
+            server.tls_ctx_config.ca_cert_file = zstrdup(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-protocols") && argc >= 2) {
+            zfree(server.tls_ctx_config.protocols);
+            server.tls_ctx_config.protocols = zstrdup(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-ciphers") && argc == 2) {
+            zfree(server.tls_ctx_config.ciphers);
+            server.tls_ctx_config.ciphers = zstrdup(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-ciphersuites") && argc == 2) {
+            zfree(server.tls_ctx_config.ciphersuites);
+            server.tls_ctx_config.ciphersuites = zstrdup(argv[1]);
+        } else if (!strcasecmp(argv[0],"tls-prefer-server-ciphers") && argc == 2) {
+            server.tls_ctx_config.prefer_server_ciphers = yesnotoi(argv[1]);
+#endif  /* USE_OPENSSL */
         } else {
             err = "Bad directive or wrong number of arguments"; goto loaderr;
         }
@@ -1268,46 +1277,90 @@ void configSetCommand(client *c) {
       "appendfsync",server.aof_fsync,aof_fsync_enum) {
     } config_set_enum_field(
       "repl-diskless-load",server.repl_diskless_load,repl_diskless_load_enum) {
-
+#ifdef USE_OPENSSL
     /* TLS fields. */
     } config_set_special_field("tls-cert-file") {
-        if (tlsConfigure((char *) o->ptr, server.tls_key_file,
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
-                    server.tls_dh_params_file, server.tls_ca_cert_file) == C_ERR) {
+        tmpctx.cert_file = (char *) o->ptr;
+        if (tlsConfigure(&tmpctx) == C_ERR) {
             addReplyError(c,
                     "Unable to configure tls-cert-file. Check server logs.");
             return;
         }
-        zfree(server.tls_cert_file);
+        zfree(server.tls_ctx_config.cert_file);
-        server.tls_cert_file = zstrdup(o->ptr);
+        server.tls_ctx_config.cert_file = zstrdup(o->ptr);
     } config_set_special_field("tls-key-file") {
-        if (tlsConfigure(server.tls_cert_file, (char *) o->ptr,
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
-                    server.tls_dh_params_file, server.tls_ca_cert_file) == C_ERR) {
+        tmpctx.key_file = (char *) o->ptr;
+        if (tlsConfigure(&tmpctx) == C_ERR) {
             addReplyError(c,
                     "Unable to configure tls-key-file. Check server logs.");
             return;
         }
-        zfree(server.tls_key_file);
+        zfree(server.tls_ctx_config.key_file);
-        server.tls_key_file = zstrdup(o->ptr);
+        server.tls_ctx_config.key_file = zstrdup(o->ptr);
     } config_set_special_field("tls-dh-params-file") {
-        if (tlsConfigure(server.tls_cert_file, server.tls_key_file,
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
-                    (char *) o->ptr, server.tls_ca_cert_file) == C_ERR) {
+        tmpctx.dh_params_file = (char *) o->ptr;
+        if (tlsConfigure(&tmpctx) == C_ERR) {
             addReplyError(c,
                     "Unable to configure tls-dh-params-file. Check server logs.");
             return;
         }
-        zfree(server.tls_dh_params_file);
+        zfree(server.tls_ctx_config.dh_params_file);
-        server.tls_dh_params_file = zstrdup(o->ptr);
+        server.tls_ctx_config.dh_params_file = zstrdup(o->ptr);
     } config_set_special_field("tls-ca-cert-file") {
-        if (tlsConfigure(server.tls_cert_file, server.tls_key_file,
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
-                    server.tls_dh_params_file, (char *) o->ptr) == C_ERR) {
+        tmpctx.ca_cert_file = (char *) o->ptr;
+        if (tlsConfigure(&tmpctx) == C_ERR) {
             addReplyError(c,
                     "Unable to configure tls-ca-cert-file. Check server logs.");
             return;
         }
-        zfree(server.tls_ca_cert_file);
+        zfree(server.tls_ctx_config.ca_cert_file);
-        server.tls_ca_cert_file = zstrdup(o->ptr);
+        server.tls_ctx_config.ca_cert_file = zstrdup(o->ptr);
     } config_set_bool_field("tls-auth-clients", server.tls_auth_clients) {
-    
+    } config_set_bool_field("tls-replication", server.tls_replication) {
+    } config_set_bool_field("tls-cluster", server.tls_cluster) {
+    } config_set_special_field("tls-protocols") {
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
+        tmpctx.protocols = (char *) o->ptr;
+        if (tlsConfigure(&tmpctx) == C_ERR) {
+            addReplyError(c,
+                    "Unable to configure tls-protocols. Check server logs.");
+            return;
+        }
+        zfree(server.tls_ctx_config.protocols);
+        server.tls_ctx_config.protocols = zstrdup(o->ptr);
+    } config_set_special_field("tls-ciphers") {
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
+        tmpctx.ciphers = (char *) o->ptr;
+        if (tlsConfigure(&tmpctx) == C_ERR) {
+            addReplyError(c,
+                "Unable to configure tls-ciphers. Check server logs.");
+            return;
+        }
+        zfree(server.tls_ctx_config.ciphers);
+        server.tls_ctx_config.ciphers = zstrdup(o->ptr);
+    } config_set_special_field("tls-ciphersuites") {
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
+        tmpctx.ciphersuites = (char *) o->ptr;
+        if (tlsConfigure(&tmpctx) == C_ERR) {
+            addReplyError(c,
+                "Unable to configure tls-ciphersuites. Check server logs.");
+            return;
+        }
+        zfree(server.tls_ctx_config.ciphersuites);
+        server.tls_ctx_config.ciphersuites = zstrdup(o->ptr);
+    } config_set_special_field("tls-prefer-server-ciphers") {
+        redisTLSContextConfig tmpctx = server.tls_ctx_config;
+        tmpctx.prefer_server_ciphers = yesnotoi(o->ptr);
+        if (tlsConfigure(&tmpctx) == C_ERR) {
+            addReplyError(c, "Unable to reconfigure TLS. Check server logs.");
+            return;
+        }
+        server.tls_ctx_config.prefer_server_ciphers = tmpctx.prefer_server_ciphers;
+#endif  /* USE_OPENSSL */
     /* Everyhing else is an error... */
     } config_set_else {
         addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s",
@@ -1381,10 +1434,15 @@ void configGetCommand(client *c) {
     config_get_string_field("pidfile",server.pidfile);
     config_get_string_field("slave-announce-ip",server.slave_announce_ip);
     config_get_string_field("replica-announce-ip",server.slave_announce_ip);
-    config_get_string_field("tls-cert-file",server.tls_cert_file);
+#ifdef USE_OPENSSL
-    config_get_string_field("tls-key-file",server.tls_key_file);
+    config_get_string_field("tls-cert-file",server.tls_ctx_config.cert_file);
-    config_get_string_field("tls-dh-params-file",server.tls_dh_params_file);
+    config_get_string_field("tls-key-file",server.tls_ctx_config.key_file);
-    config_get_string_field("tls-ca-cert-file",server.tls_ca_cert_file);
+    config_get_string_field("tls-dh-params-file",server.tls_ctx_config.dh_params_file);
+    config_get_string_field("tls-ca-cert-file",server.tls_ctx_config.ca_cert_file);
+    config_get_string_field("tls-protocols",server.tls_ctx_config.protocols);
+    config_get_string_field("tls-ciphers",server.tls_ctx_config.ciphers);
+    config_get_string_field("tls-ciphersuites",server.tls_ctx_config.ciphersuites);
+#endif
 
     /* Numerical values */
     config_get_numerical_field("maxmemory",server.maxmemory);
@@ -1476,7 +1534,8 @@ void configGetCommand(client *c) {
     config_get_bool_field("tls-cluster",server.tls_cluster);
     config_get_bool_field("tls-replication",server.tls_replication);
     config_get_bool_field("tls-auth-clients",server.tls_auth_clients);
-
+    config_get_bool_field("tls-prefer-server-ciphers",
+            server.tls_ctx_config.prefer_server_ciphers);
     /* Enum values */
     config_get_enum_field("maxmemory-policy",
             server.maxmemory_policy,maxmemory_policy_enum);
@@ -1590,6 +1649,7 @@ void configGetCommand(client *c) {
         }
         matches++;
     }
+
     setDeferredMapLen(c,replylen,matches);
 }
 
@@ -2200,9 +2260,6 @@ int rewriteConfig(char *path) {
     rewriteConfigNumericalOption(state,"cluster-announce-port",server.cluster_announce_port,CONFIG_DEFAULT_CLUSTER_ANNOUNCE_PORT);
     rewriteConfigNumericalOption(state,"cluster-announce-bus-port",server.cluster_announce_bus_port,CONFIG_DEFAULT_CLUSTER_ANNOUNCE_BUS_PORT);
     rewriteConfigNumericalOption(state,"tcp-backlog",server.tcp_backlog,CONFIG_DEFAULT_TCP_BACKLOG);
-    rewriteConfigYesNoOption(state,"tls-cluster",server.tls_cluster,0);
-    rewriteConfigYesNoOption(state,"tls-replication",server.tls_replication,0);
-    rewriteConfigYesNoOption(state,"tls-auth-clients",server.tls_auth_clients,1);
     rewriteConfigBindOption(state);
     rewriteConfigStringOption(state,"unixsocket",server.unixsocket,NULL);
     rewriteConfigOctalOption(state,"unixsocketperm",server.unixsocketperm,CONFIG_DEFAULT_UNIX_SOCKET_PERM);
@@ -2282,10 +2339,19 @@ int rewriteConfig(char *path) {
     rewriteConfigEnumOption(state,"supervised",server.supervised_mode,supervised_mode_enum,SUPERVISED_NONE);
     rewriteConfigNumericalOption(state,"rdb-key-save-delay",server.rdb_key_save_delay,CONFIG_DEFAULT_RDB_KEY_SAVE_DELAY);
     rewriteConfigNumericalOption(state,"key-load-delay",server.key_load_delay,CONFIG_DEFAULT_KEY_LOAD_DELAY);
-    rewriteConfigStringOption(state,"tls-cert-file",server.tls_cert_file,NULL);
+#ifdef USE_OPENSSL
-    rewriteConfigStringOption(state,"tls-key-file",server.tls_key_file,NULL);
+    rewriteConfigYesNoOption(state,"tls-cluster",server.tls_cluster,0);
-    rewriteConfigStringOption(state,"tls-dh-params-file",server.tls_dh_params_file,NULL);
+    rewriteConfigYesNoOption(state,"tls-replication",server.tls_replication,0);
-    rewriteConfigStringOption(state,"tls-ca-cert-file",server.tls_ca_cert_file,NULL);
+    rewriteConfigYesNoOption(state,"tls-auth-clients",server.tls_auth_clients,1);
+    rewriteConfigStringOption(state,"tls-cert-file",server.tls_ctx_config.cert_file,NULL);
+    rewriteConfigStringOption(state,"tls-key-file",server.tls_ctx_config.key_file,NULL);
+    rewriteConfigStringOption(state,"tls-dh-params-file",server.tls_ctx_config.dh_params_file,NULL);
+    rewriteConfigStringOption(state,"tls-ca-cert-file",server.tls_ctx_config.ca_cert_file,NULL);
+    rewriteConfigStringOption(state,"tls-protocols",server.tls_ctx_config.protocols,NULL);
+    rewriteConfigStringOption(state,"tls-ciphers",server.tls_ctx_config.ciphers,NULL);
+    rewriteConfigStringOption(state,"tls-ciphersuites",server.tls_ctx_config.ciphersuites,NULL);
+    rewriteConfigYesNoOption(state,"tls-prefer-server-ciphers",server.tls_ctx_config.prefer_server_ciphers,0);
+#endif
 
     /* Rewrite Sentinel config if in Sentinel mode. */
     if (server.sentinel_mode) rewriteConfigSentinelOption(state);

src/replication.c

@@ -2023,11 +2023,14 @@ void syncWithMaster(connection *conn) {
     /* Set the slave port, so that Master's INFO command can list the
      * slave listening port correctly. */
     if (server.repl_state == REPL_STATE_SEND_PORT) {
-        sds port = sdsfromlonglong(server.slave_announce_port ?
+        int port;
-            server.slave_announce_port : server.port);
+        if (server.slave_announce_port) port = server.slave_announce_port;
+        else if (server.tls_replication && server.tls_port) port = server.tls_port;
+        else port = server.port;
+        sds portstr = sdsfromlonglong(port);
         err = sendSynchronousCommand(SYNC_CMD_WRITE,conn,"REPLCONF",
-                "listening-port",port, NULL);
+                "listening-port",portstr, NULL);
-        sdsfree(port);
+        sdsfree(portstr);
         if (err) goto write_error;
         sdsfree(err);
         server.repl_state = REPL_STATE_RECEIVE_PORT;

src/sentinel.c

@@ -2612,8 +2612,9 @@ int sentinelSendHello(sentinelRedisInstance *ri) {
             return C_ERR;
         announce_ip = ip;
     }
-    announce_port = sentinel.announce_port ?
+    if (sentinel.announce_port) announce_port = sentinel.announce_port;
-                    sentinel.announce_port : server.port;
+    else if (server.tls_replication && server.tls_port) announce_port = server.tls_port;
+    else announce_port = server.port;
 
     /* Format and send the Hello message. */
     snprintf(payload,sizeof(payload),

src/server.c

@@ -2451,9 +2451,6 @@ void initServerConfig(void) {
      * script to the slave / AOF. This is the new way starting from
      * Redis 5. However it is possible to revert it via redis.conf. */
     server.lua_always_replicate_commands = 1;
-
-    /* TLS */
-    server.tls_auth_clients = 1;
 }
 
 extern char **environ;
@@ -2770,7 +2767,7 @@ void initServer(void) {
     server.clients_paused = 0;
     server.system_memory_size = zmalloc_get_memory_size();
 
-    if (server.tls_port && tlsConfigureServer() == C_ERR) {
+    if (server.tls_port && tlsConfigure(&server.tls_ctx_config) == C_ERR) {
         serverLog(LL_WARNING, "Failed to configure TLS. Check logs for more info.");
         exit(1);
     }
@@ -3943,7 +3940,7 @@ sds genRedisInfoString(char *section) {
 #endif
             (long) getpid(),
             server.runid,
-            server.port,
+            server.port ? server.port : server.tls_port,
             (intmax_t)uptime,
             (intmax_t)(uptime/(3600*24)),
             server.hz,
@@ -4554,7 +4551,7 @@ void redisAsciiArt(void) {
     if (!show_logo) {
         serverLog(LL_NOTICE,
             "Running mode=%s, port=%d.",
-            mode, server.port
+            mode, server.port ? server.port : server.tls_port
         );
     } else {
         snprintf(buf,1024*16,ascii_logo,

src/server.h

@@ -1029,6 +1029,21 @@ struct malloc_stats {
     size_t allocator_resident;
 };
 
+/*-----------------------------------------------------------------------------
+ * TLS Context Configuration
+ *----------------------------------------------------------------------------*/
+
+typedef struct redisTLSContextConfig {
+    char *cert_file;
+    char *key_file;
+    char *dh_params_file;
+    char *ca_cert_file;
+    char *protocols;
+    char *ciphers;
+    char *ciphersuites;
+    int prefer_server_ciphers;
+} redisTLSContextConfig;
+
 /*-----------------------------------------------------------------------------
  * Global server state
  *----------------------------------------------------------------------------*/
@@ -1427,11 +1442,8 @@ struct redisServer {
     /* TLS Configuration */
     int tls_cluster;
     int tls_replication;
-    char *tls_cert_file;
-    char *tls_key_file;
-    char *tls_dh_params_file;
-    char *tls_ca_cert_file;
     int tls_auth_clients;
+    redisTLSContextConfig tls_ctx_config;
 };
 
 typedef struct pubsubPattern {
@@ -2371,9 +2383,7 @@ int populateCommandTableParseFlags(struct redisCommand *c, char *strflags);
 
 /* TLS stuff */
 void tlsInit(void);
-int tlsConfigureServer(void);
+int tlsConfigure(redisTLSContextConfig *ctx_config);
-int tlsConfigure(const char *cert_file, const char *key_file,
-        const char *dh_params_file, const char *ca_cert_file);
 
 #define redisDebug(fmt, ...) \
     printf("DEBUG %s:%d > " fmt "\n", __FILE__, __LINE__, __VA_ARGS__)

src/tls.c

@@ -38,10 +38,57 @@
 #include <openssl/err.h>
 #include <openssl/rand.h>
 
+#define REDIS_TLS_PROTO_TLSv1       (1<<0)
+#define REDIS_TLS_PROTO_TLSv1_1     (1<<1)
+#define REDIS_TLS_PROTO_TLSv1_2     (1<<2)
+#define REDIS_TLS_PROTO_TLSv1_3     (1<<3)
+
+/* Use safe defaults */
+#ifdef TLS1_3_VERSION
+#define REDIS_TLS_PROTO_DEFAULT     (REDIS_TLS_PROTO_TLSv1_2|REDIS_TLS_PROTO_TLSv1_3)
+#else
+#define REDIS_TLS_PROTO_DEFAULT     (REDIS_TLS_PROTO_TLSv1_2)
+#endif
+
 extern ConnectionType CT_Socket;
 
 SSL_CTX *redis_tls_ctx;
 
+static int parseProtocolsConfig(const char *str) {
+    int i, count = 0;
+    int protocols = 0;
+
+    if (!str) return REDIS_TLS_PROTO_DEFAULT;
+    sds *tokens = sdssplitlen(str, strlen(str), " ", 1, &count);
+
+    if (!tokens) { 
+        serverLog(LL_WARNING, "Invalid tls-protocols configuration string");
+        return -1;
+    }
+    for (i = 0; i < count; i++) {
+        if (!strcasecmp(tokens[i], "tlsv1")) protocols |= REDIS_TLS_PROTO_TLSv1;
+        else if (!strcasecmp(tokens[i], "tlsv1.1")) protocols |= REDIS_TLS_PROTO_TLSv1_1;
+        else if (!strcasecmp(tokens[i], "tlsv1.2")) protocols |= REDIS_TLS_PROTO_TLSv1_2;
+        else if (!strcasecmp(tokens[i], "tlsv1.3")) {
+#ifdef TLS1_3_VERSION
+            protocols |= REDIS_TLS_PROTO_TLSv1_3;
+#else
+            serverLog(LL_WARNING, "TLSv1.3 is specified in tls-protocols but not supported by OpenSSL.");
+            protocols = -1;
+            break;
+#endif
+        } else {
+            serverLog(LL_WARNING, "Invalid tls-protocols specified. "
+                    "Use a combination of 'TLSv1', 'TLSv1.1', 'TLSv1.2' and 'TLSv1.3'.");
+            protocols = -1;
+            break;
+        }
+    }
+    sdsfreesplitres(tokens, count);
+
+    return protocols;
+}
+
 /* list of connections with pending data already read from the socket, but not
  * served to the reader yet. */
 static list *pending_list = NULL;
@@ -56,78 +103,109 @@ void tlsInit(void) {
     }
 
     pending_list = listCreate();
-}
 
-int tlsConfigureServer(void) {
+    /* Server configuration */
-    return tlsConfigure(server.tls_cert_file, server.tls_key_file,
+    server.tls_auth_clients = 1;    /* Secure by default */
-            server.tls_dh_params_file, server.tls_ca_cert_file);
 }
 
 /* Attempt to configure/reconfigure TLS. This operation is atomic and will
  * leave the SSL_CTX unchanged if fails.
  */
-int tlsConfigure(const char *cert_file, const char *key_file,
+int tlsConfigure(redisTLSContextConfig *ctx_config) {
-        const char *dh_params_file, const char *ca_cert_file) {
-
     char errbuf[256];
     SSL_CTX *ctx = NULL;
 
-    if (!cert_file) {
+    if (!ctx_config->cert_file) {
         serverLog(LL_WARNING, "No tls-cert-file configured!");
         goto error;
     }
 
-    if (!key_file) {
+    if (!ctx_config->key_file) {
         serverLog(LL_WARNING, "No tls-key-file configured!");
         goto error;
     }
 
-    if (!ca_cert_file) {
+    if (!ctx_config->ca_cert_file) {
         serverLog(LL_WARNING, "No tls-ca-cert-file configured!");
         goto error;
     }
 
-    ctx = SSL_CTX_new(TLS_method());
+    ctx = SSL_CTX_new(SSLv23_method());
+
+    SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2|SSL_OP_NO_SSLv3);
+    SSL_CTX_set_options(ctx, SSL_OP_SINGLE_DH_USE);
+
+#ifdef SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS
+    SSL_CTX_set_options(ctx, SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS);
+#endif
+
+    int protocols = parseProtocolsConfig(ctx_config->protocols);
+    if (protocols == -1) goto error;
+
+    if (!(protocols & REDIS_TLS_PROTO_TLSv1))
+        SSL_CTX_set_options(ctx, SSL_OP_NO_TLSv1);
+    if (!(protocols & REDIS_TLS_PROTO_TLSv1_1))
+        SSL_CTX_set_options(ctx, SSL_OP_NO_TLSv1_1);
+#ifdef SSL_OP_NO_TLSv1_2
+    if (!(protocols & REDIS_TLS_PROTO_TLSv1_2))
+        SSL_CTX_set_options(ctx, SSL_OP_NO_TLSv1_2);
+#endif
+#ifdef SSL_OP_NO_TLSv1_3
+    if (!(protocols & REDIS_TLS_PROTO_TLSv1_3))
+        SSL_CTX_set_options(ctx, SSL_OP_NO_TLSv1_3);
+#endif
+
+#ifdef SSL_OP_NO_COMPRESSION
+    SSL_CTX_set_options(ctx, SSL_OP_NO_COMPRESSION);
+#endif
+
+#ifdef SSL_OP_NO_CLIENT_RENEGOTIATION
+    SSL_CTX_set_options(ssl->ctx, SSL_OP_NO_CLIENT_RENEGOTIATION);
+#endif
+
+    if (ctx_config->prefer_server_ciphers)
+        SSL_CTX_set_options(ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
+
     SSL_CTX_set_mode(ctx, SSL_MODE_ENABLE_PARTIAL_WRITE|SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
     SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL);
     SSL_CTX_set_ecdh_auto(ctx, 1);
 
-    if (SSL_CTX_use_certificate_file(ctx, cert_file, SSL_FILETYPE_PEM) <= 0) {
+    if (SSL_CTX_use_certificate_file(ctx, ctx_config->cert_file, SSL_FILETYPE_PEM) <= 0) {
         ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf));
-        serverLog(LL_WARNING, "Failed to load certificate: %s: %s", cert_file, errbuf);
+        serverLog(LL_WARNING, "Failed to load certificate: %s: %s", ctx_config->cert_file, errbuf);
         goto error;
     }
         
-    if (SSL_CTX_use_PrivateKey_file(ctx, key_file, SSL_FILETYPE_PEM) <= 0) {
+    if (SSL_CTX_use_PrivateKey_file(ctx, ctx_config->key_file, SSL_FILETYPE_PEM) <= 0) {
         ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf));
-        serverLog(LL_WARNING, "Failed to load private key: %s: %s", key_file, errbuf);
+        serverLog(LL_WARNING, "Failed to load private key: %s: %s", ctx_config->key_file, errbuf);
         goto error;
     }
     
-    if (SSL_CTX_load_verify_locations(ctx, ca_cert_file, NULL) <= 0) {
+    if (SSL_CTX_load_verify_locations(ctx, ctx_config->ca_cert_file, NULL) <= 0) {
         ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf));
-        serverLog(LL_WARNING, "Failed to load CA certificate(s) file: %s: %s", ca_cert_file, errbuf);
+        serverLog(LL_WARNING, "Failed to load CA certificate(s) file: %s: %s", ctx_config->ca_cert_file, errbuf);
         goto error;
     }
 
-    if (dh_params_file) {
+    if (ctx_config->dh_params_file) {
-        FILE *dhfile = fopen(dh_params_file, "r");
+        FILE *dhfile = fopen(ctx_config->dh_params_file, "r");
         DH *dh = NULL;
         if (!dhfile) {
-            serverLog(LL_WARNING, "Failed to load %s: %s", dh_params_file, strerror(errno));
+            serverLog(LL_WARNING, "Failed to load %s: %s", ctx_config->dh_params_file, strerror(errno));
             goto error;
         }
 
         dh = PEM_read_DHparams(dhfile, NULL, NULL, NULL);
         fclose(dhfile);
         if (!dh) {
-            serverLog(LL_WARNING, "%s: failed to read DH params.", dh_params_file);
+            serverLog(LL_WARNING, "%s: failed to read DH params.", ctx_config->dh_params_file);
             goto error;
         }
 
         if (SSL_CTX_set_tmp_dh(ctx, dh) <= 0) {
             ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf));
-            serverLog(LL_WARNING, "Failed to load DH params file: %s: %s", dh_params_file, errbuf);
+            serverLog(LL_WARNING, "Failed to load DH params file: %s: %s", ctx_config->dh_params_file, errbuf);
             DH_free(dh);
             goto error;
         }
@@ -402,7 +480,7 @@ static void tlsHandleEvent(tls_connection *conn, int mask) {
              * risk of not calling the read handler again, make sure to add it
              * to a list of pending connection that should be handled anyway. */
             if ((mask & AE_READABLE)) {
-                if (SSL_has_pending(conn->ssl)) {
+                if (SSL_pending(conn->ssl) > 0) {
                     if (!conn->pending_list_node) {
                         listAddNodeTail(pending_list, conn);
                         conn->pending_list_node = listLast(pending_list);
@@ -704,16 +782,8 @@ void tlsProcessPendingData() {
 void tlsInit(void) {
 }
 
-int tlsConfigure(const char *cert_file, const char *key_file,
+int tlsConfigure(redisTLSContextConfig *ctx_config) {
-        const char *dh_params_file, const char *ca_cert_file) {
+    UNUSED(ctx_config);
-    UNUSED(cert_file);
-    UNUSED(key_file);
-    UNUSED(dh_params_file);
-    UNUSED(ca_cert_file);
-    return C_OK;
-}
-
-int tlsConfigureServer(void) {
     return C_OK;
 }
 
@@ -723,6 +793,7 @@ connection *connCreateTLS(void) {
 
 connection *connCreateAcceptedTLS(int fd, int require_auth) {
     UNUSED(fd);
+    UNUSED(require_auth);
 
     return NULL;
 }

tests/integration/replication.tcl

@@ -504,11 +504,15 @@ start_server {tags {"repl"}} {
     $master config set repl-diskless-sync-delay 1
     set master_host [srv 0 host]
     set master_port [srv 0 port]
+    set master_pid [srv 0 pid]
     # put enough data in the db that the rdb file will be bigger than the socket buffers
     # and since we'll have key-load-delay of 100, 10000 keys will take at least 1 second
     # we also need the replica to process requests during transfer (which it does only once in 2mb)
     $master debug populate 10000 test 10000
     $master config set rdbcompression no
+    # If running on Linux, we also measure utime/stime to detect possible I/O handling issues
+    set os [catch {exec unamee}]
+    set measure_time [expr {$os == "Linux"} ? 1 : 0]
     foreach all_drop {no slow fast all} {
         test "diskless $all_drop replicas drop during rdb pipe" {
             set replicas {}
@@ -533,9 +537,11 @@ start_server {tags {"repl"}} {
                     # using the log file since the replica only responds to INFO once in 2mb
                     wait_for_log_message -1 "*Loading DB in memory*" 8 800 10
 
-                    set master_statfile [format "/proc/%s/stat" [srv -2 pid]]
+                    if {$measure_time} {
-                    set master_start_metrics [get_cpu_metrics $master_statfile]
+                        set master_statfile "/proc/$master_pid/stat"
-                    set start_time [clock seconds]
+                        set master_start_metrics [get_cpu_metrics $master_statfile]
+                        set start_time [clock seconds]
+                    }
 
                     # wait a while so that the pipe socket writer will be
                     # blocked on write (since replica 0 is slow to read from the socket)
@@ -573,23 +579,25 @@ start_server {tags {"repl"}} {
                     }
 
                     # make sure we don't have a busy loop going thought epoll_wait
-                    set master_end_metrics [get_cpu_metrics $master_statfile]
+                    if {$measure_time} {
-                    set time_elapsed [expr {[clock seconds]-$start_time}]
+                        set master_end_metrics [get_cpu_metrics $master_statfile]
-                    set master_cpu [compute_cpu_usage $master_start_metrics $master_end_metrics]
+                        set time_elapsed [expr {[clock seconds]-$start_time}]
-                    set master_utime [lindex $master_cpu 0]
+                        set master_cpu [compute_cpu_usage $master_start_metrics $master_end_metrics]
-                    set master_stime [lindex $master_cpu 1]
+                        set master_utime [lindex $master_cpu 0]
-                    if {$::verbose} {
+                        set master_stime [lindex $master_cpu 1]
-                        puts "elapsed: $time_elapsed"
+                        if {$::verbose} {
-                        puts "master utime: $master_utime"
+                            puts "elapsed: $time_elapsed"
-                        puts "master stime: $master_stime"
+                            puts "master utime: $master_utime"
-                    }
+                            puts "master stime: $master_stime"
-                    if {$all_drop == "all" || $all_drop == "slow"} {
+                        }
-                        assert {$master_utime < 30}
+                        if {$all_drop == "all" || $all_drop == "slow"} {
-                        assert {$master_stime < 30}
+                            assert {$master_utime < 70}
-                    }
+                            assert {$master_stime < 70}
-                    if {$all_drop == "none" || $all_drop == "fast"} {
+                        }
-                        assert {$master_utime < 15}
+                        if {$all_drop == "none" || $all_drop == "fast"} {
-                        assert {$master_stime < 15}
+                            assert {$master_utime < 15}
+                            assert {$master_stime < 15}
+                        }
                     }
 
                     # verify the data integrity

tests/sentinel/run.tcl

@@ -6,6 +6,7 @@ cd tests/sentinel
 source ../instances.tcl
 
 set ::instances_count 5 ; # How many instances we use at max.
+set ::tlsdir "../../tls"
 
 proc main {} {
     parse_options

tests/support/redis.tcl

@@ -39,13 +39,14 @@ array set ::redis::callback {}
 array set ::redis::state {} ;# State in non-blocking reply reading
 array set ::redis::statestack {} ;# Stack of states, for nested mbulks
 
-proc redis {{server 127.0.0.1} {port 6379} {defer 0} {tls 0}} {
+proc redis {{server 127.0.0.1} {port 6379} {defer 0} {tls 0} {tlsoptions {}}} {
     if {$tls} {
         package require tls
         ::tls::init \
             -cafile "$::tlsdir/ca.crt" \
             -certfile "$::tlsdir/redis.crt" \
-            -keyfile "$::tlsdir/redis.key"
+            -keyfile "$::tlsdir/redis.key" \
+            {*}$tlsoptions
         set fd [::tls::socket $server $port]
     } else {
         set fd [socket $server $port]

tests/unit/tls.tcl

@@ -14,12 +14,92 @@ start_server {tags {"tls"}} {
             catch {$s PING} e
             assert_match {*error*} $e
 
-            set resp [r CONFIG SET tls-auth-clients no]
+            r CONFIG SET tls-auth-clients no
 
             set s [redis [srv 0 host] [srv 0 port]]
             ::tls::import [$s channel]
             catch {$s PING} e
             assert_match {PONG} $e
-        } {}
+
+            r CONFIG SET tls-auth-clients yes
+        }
+
+        test {TLS: Verify tls-protocols behaves as expected} {
+            r CONFIG SET tls-protocols TLSv1
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-tls1 0}]
+            catch {$s PING} e
+            assert_match {*I/O error*} $e
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-tls1 1}]
+            catch {$s PING} e
+            assert_match {PONG} $e
+
+            r CONFIG SET tls-protocols TLSv1.1
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-tls1.1 0}]
+            catch {$s PING} e
+            assert_match {*I/O error*} $e
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-tls1.1 1}]
+            catch {$s PING} e
+            assert_match {PONG} $e
+
+            r CONFIG SET tls-protocols TLSv1.2
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-tls1.2 0}]
+            catch {$s PING} e
+            assert_match {*I/O error*} $e
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-tls1.2 1}]
+            catch {$s PING} e
+            assert_match {PONG} $e
+
+            r CONFIG SET tls-protocols ""
+        }
+
+        test {TLS: Verify tls-ciphers behaves as expected} {
+            r CONFIG SET tls-protocols TLSv1.2
+            r CONFIG SET tls-ciphers "DEFAULT:-AES128-SHA256"
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-cipher "-ALL:AES128-SHA256"}]
+            catch {$s PING} e
+            assert_match {*I/O error*} $e
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-cipher "-ALL:AES256-SHA256"}]
+            catch {$s PING} e
+            assert_match {PONG} $e
+
+            r CONFIG SET tls-ciphers "DEFAULT"
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-cipher "-ALL:AES128-SHA256"}]
+            catch {$s PING} e
+            assert_match {PONG} $e
+
+            r CONFIG SET tls-protocols ""
+            r CONFIG SET tls-ciphers "DEFAULT"
+        }
+
+        test {TLS: Verify tls-prefer-server-ciphers behaves as expected} {
+            r CONFIG SET tls-protocols TLSv1.2
+            r CONFIG SET tls-ciphers "AES128-SHA256:AES256-SHA256"
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-cipher "AES256-SHA256:AES128-SHA256"}]
+            catch {$s PING} e
+            assert_match {PONG} $e
+
+            assert_equal "AES256-SHA256" [dict get [::tls::status [$s channel]] cipher]
+
+            r CONFIG SET tls-prefer-server-ciphers yes
+
+            set s [redis [srv 0 host] [srv 0 port] 0 1 {-cipher "AES256-SHA256:AES128-SHA256"}]
+            catch {$s PING} e
+            assert_match {PONG} $e
+
+            assert_equal "AES128-SHA256" [dict get [::tls::status [$s channel]] cipher]
+
+            r CONFIG SET tls-protocols ""
+            r CONFIG SET tls-ciphers "DEFAULT"
+        }
     }
 }