From 61733ded1426908c6b0c1b8c94f03cff3d20f32e Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Thu, 12 Sep 2019 11:10:22 +0300 Subject: [PATCH] TLS: Configuration options. Add configuration options for TLS protocol versions, ciphers/cipher suites selection, etc. --- TLS.md | 54 ++++------ redis.conf | 24 +++++ src/Makefile | 2 + src/config.c | 170 +++++++++++++++++++++--------- src/replication.c | 11 +- src/sentinel.c | 5 +- src/server.c | 9 +- src/server.h | 24 +++-- src/tls.c | 137 ++++++++++++++++++------ tests/integration/replication.tcl | 48 +++++---- tests/sentinel/run.tcl | 1 + tests/support/redis.tcl | 5 +- tests/unit/tls.tcl | 84 ++++++++++++++- 13 files changed, 414 insertions(+), 160 deletions(-) diff --git a/TLS.md b/TLS.md index c627f814c..76fe0be2e 100644 --- a/TLS.md +++ b/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 -implementation details between TLS and TCP. +All socket operations now go through a connection abstraction layer that hides +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 - to manipulate AE based on OpenSSL return codes. We can either propagate this - out of the thread, or explore ways of further optimizing MT I/O by having - event loops that live inside the thread and borrow connections in/out. +**Multi-threading I/O is not currently supported for TLS**, as a TLS connection +needs to do its own manipulation of AE events which is not thread safe. The +solution is probably to manage independent AE loops for I/O threads and longer +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 - and reflected into connection state, connection state validated before - certain operations, etc. - - Clean (non-errno) interface to report would-block. - - Consistent error reporting. +Sync IO for TLS is currently implemented in a hackish way, i.e. making the +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. -3. Sync IO for TLS is currently implemented in a hackish way, i.e. making the - 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. +To-Do List +========== -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. +Additional TLS Features +----------------------- +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 ------ diff --git a/redis.conf b/redis.conf index 2af422a93..1f966badb 100644 --- a/redis.conf +++ b/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. diff --git a/src/Makefile b/src/Makefile index b43b743dc..b020f17cf 100644 --- a/src/Makefile +++ b/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 diff --git a/src/config.c b/src/config.c index 792e45057..bb04cb45b 100644 --- a/src/config.c +++ b/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) { - zfree(server.tls_cert_file); - server.tls_cert_file = zstrdup(argv[1]); - } else if (!strcasecmp(argv[0],"tls-key-file") && argc == 2) { - zfree(server.tls_key_file); - 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]); +#ifdef USE_OPENSSL + } else if (!strcasecmp(argv[0],"tls-port") && argc == 2) { + server.tls_port = atoi(argv[1]); + if (server.port < 0 || server.port > 65535) { + err = "Invalid tls-port"; goto loaderr; + } } 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, - server.tls_dh_params_file, server.tls_ca_cert_file) == C_ERR) { + redisTLSContextConfig tmpctx = server.tls_ctx_config; + 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); - server.tls_cert_file = zstrdup(o->ptr); + zfree(server.tls_ctx_config.cert_file); + 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, - server.tls_dh_params_file, server.tls_ca_cert_file) == C_ERR) { + redisTLSContextConfig tmpctx = server.tls_ctx_config; + 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); - server.tls_key_file = zstrdup(o->ptr); + zfree(server.tls_ctx_config.key_file); + 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, - (char *) o->ptr, server.tls_ca_cert_file) == C_ERR) { + redisTLSContextConfig tmpctx = server.tls_ctx_config; + 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); - server.tls_dh_params_file = zstrdup(o->ptr); + zfree(server.tls_ctx_config.dh_params_file); + 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, - server.tls_dh_params_file, (char *) o->ptr) == C_ERR) { + redisTLSContextConfig tmpctx = server.tls_ctx_config; + 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); - server.tls_ca_cert_file = zstrdup(o->ptr); + zfree(server.tls_ctx_config.ca_cert_file); + 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); - config_get_string_field("tls-key-file",server.tls_key_file); - config_get_string_field("tls-dh-params-file",server.tls_dh_params_file); - config_get_string_field("tls-ca-cert-file",server.tls_ca_cert_file); +#ifdef USE_OPENSSL + config_get_string_field("tls-cert-file",server.tls_ctx_config.cert_file); + config_get_string_field("tls-key-file",server.tls_ctx_config.key_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); - rewriteConfigStringOption(state,"tls-key-file",server.tls_key_file,NULL); - rewriteConfigStringOption(state,"tls-dh-params-file",server.tls_dh_params_file,NULL); - rewriteConfigStringOption(state,"tls-ca-cert-file",server.tls_ca_cert_file,NULL); +#ifdef USE_OPENSSL + 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); + 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); diff --git a/src/replication.c b/src/replication.c index 9f5c19053..98a7dd9e6 100644 --- a/src/replication.c +++ b/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 ? - server.slave_announce_port : server.port); + int 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); - sdsfree(port); + "listening-port",portstr, NULL); + sdsfree(portstr); if (err) goto write_error; sdsfree(err); server.repl_state = REPL_STATE_RECEIVE_PORT; diff --git a/src/sentinel.c b/src/sentinel.c index 95f189485..92869805d 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -2612,8 +2612,9 @@ int sentinelSendHello(sentinelRedisInstance *ri) { return C_ERR; announce_ip = ip; } - announce_port = sentinel.announce_port ? - sentinel.announce_port : server.port; + if (sentinel.announce_port) announce_port = sentinel.announce_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), diff --git a/src/server.c b/src/server.c index 0f5837bc7..3de1347cb 100644 --- a/src/server.c +++ b/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, diff --git a/src/server.h b/src/server.h index df5e42c41..5c7849732 100644 --- a/src/server.h +++ b/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(const char *cert_file, const char *key_file, - const char *dh_params_file, const char *ca_cert_file); +int tlsConfigure(redisTLSContextConfig *ctx_config); #define redisDebug(fmt, ...) \ printf("DEBUG %s:%d > " fmt "\n", __FILE__, __LINE__, __VA_ARGS__) diff --git a/src/tls.c b/src/tls.c index 68d15ef9e..a77fbfe3d 100644 --- a/src/tls.c +++ b/src/tls.c @@ -38,10 +38,57 @@ #include #include +#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) { - return tlsConfigure(server.tls_cert_file, server.tls_key_file, - server.tls_dh_params_file, server.tls_ca_cert_file); + /* Server configuration */ + server.tls_auth_clients = 1; /* Secure by default */ } /* 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, - const char *dh_params_file, const char *ca_cert_file) { - +int tlsConfigure(redisTLSContextConfig *ctx_config) { 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) { - FILE *dhfile = fopen(dh_params_file, "r"); + if (ctx_config->dh_params_file) { + 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, - const char *dh_params_file, const char *ca_cert_file) { - UNUSED(cert_file); - UNUSED(key_file); - UNUSED(dh_params_file); - UNUSED(ca_cert_file); - return C_OK; -} - -int tlsConfigureServer(void) { +int tlsConfigure(redisTLSContextConfig *ctx_config) { + UNUSED(ctx_config); return C_OK; } @@ -723,6 +793,7 @@ connection *connCreateTLS(void) { connection *connCreateAcceptedTLS(int fd, int require_auth) { UNUSED(fd); + UNUSED(require_auth); return NULL; } diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index 192137e87..626a3dbb9 100644 --- a/tests/integration/replication.tcl +++ b/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]] - set master_start_metrics [get_cpu_metrics $master_statfile] - set start_time [clock seconds] + if {$measure_time} { + set master_statfile "/proc/$master_pid/stat" + 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] - set time_elapsed [expr {[clock seconds]-$start_time}] - set master_cpu [compute_cpu_usage $master_start_metrics $master_end_metrics] - set master_utime [lindex $master_cpu 0] - set master_stime [lindex $master_cpu 1] - if {$::verbose} { - puts "elapsed: $time_elapsed" - puts "master utime: $master_utime" - puts "master stime: $master_stime" - } - if {$all_drop == "all" || $all_drop == "slow"} { - assert {$master_utime < 30} - assert {$master_stime < 30} - } - if {$all_drop == "none" || $all_drop == "fast"} { - assert {$master_utime < 15} - assert {$master_stime < 15} + if {$measure_time} { + set master_end_metrics [get_cpu_metrics $master_statfile] + set time_elapsed [expr {[clock seconds]-$start_time}] + set master_cpu [compute_cpu_usage $master_start_metrics $master_end_metrics] + set master_utime [lindex $master_cpu 0] + set master_stime [lindex $master_cpu 1] + if {$::verbose} { + puts "elapsed: $time_elapsed" + puts "master utime: $master_utime" + puts "master stime: $master_stime" + } + if {$all_drop == "all" || $all_drop == "slow"} { + assert {$master_utime < 70} + assert {$master_stime < 70} + } + if {$all_drop == "none" || $all_drop == "fast"} { + assert {$master_utime < 15} + assert {$master_stime < 15} + } } # verify the data integrity diff --git a/tests/sentinel/run.tcl b/tests/sentinel/run.tcl index 9a2fcfb49..996af906a 100644 --- a/tests/sentinel/run.tcl +++ b/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 diff --git a/tests/support/redis.tcl b/tests/support/redis.tcl index be6a11c7a..a90ac7f29 100644 --- a/tests/support/redis.tcl +++ b/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] diff --git a/tests/unit/tls.tcl b/tests/unit/tls.tcl index 58acdb6a9..950f65557 100644 --- a/tests/unit/tls.tcl +++ b/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" + } } }