From f31260b0445f5649449da41555e1272a40ae4af7 Mon Sep 17 00:00:00 2001
From: Jiayuan Chen <mrpre@163.com>
Date: Tue, 28 Jul 2020 15:45:21 +0800
Subject: [PATCH] Add optional tls verification (#7502)
Adds an `optional` value to the previously boolean `tls-auth-clients` configuration keyword.
Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
---
redis.conf | 5 ++++-
src/cluster.c | 3 ++-
src/config.c | 8 +++++++-
src/server.h | 5 +++++
src/tls.c | 12 ++++++++++--
tests/unit/tls.tcl | 12 ++++++++++++
6 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/redis.conf b/redis.conf
index 8c53f015a..d4e3e47f0 100644
--- a/redis.conf
+++ b/redis.conf
@@ -159,9 +159,12 @@ tcp-keepalive 300
# By default, clients (including replica servers) on a TLS port are required
# to authenticate using valid client side certificates.
#
-# It is possible to disable authentication using this directive.
+# If "no" is specified, client certificates are not required and not accepted.
+# If "optional" is specified, client certificates are accepted and must be
+# valid if provided, but are not required.
#
# tls-auth-clients no
+# tls-auth-clients optional
# By default, a Redis replica does not attempt to establish a TLS connection
# with its master.
diff --git a/src/cluster.c b/src/cluster.c
index 5dcd69ff8..485683cf0 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -670,7 +670,8 @@ void clusterAcceptHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
return;
}
- connection *conn = server.tls_cluster ? connCreateAcceptedTLS(cfd,1) : connCreateAcceptedSocket(cfd);
+ connection *conn = server.tls_cluster ?
+ connCreateAcceptedTLS(cfd, TLS_CLIENT_AUTH_YES) : connCreateAcceptedSocket(cfd);
connNonBlock(conn);
connEnableTcpNoDelay(conn);
diff --git a/src/config.c b/src/config.c
index 9b6dc459c..10faa3bea 100644
--- a/src/config.c
+++ b/src/config.c
@@ -98,6 +98,12 @@ configEnum repl_diskless_load_enum[] = {
{NULL, 0}
};
+configEnum tls_auth_clients_enum[] = {
+ {"no", TLS_CLIENT_AUTH_NO},
+ {"yes", TLS_CLIENT_AUTH_YES},
+ {"optional", TLS_CLIENT_AUTH_OPTIONAL},
+ {NULL, 0}
+};
/* Output buffer limits presets. */
clientBufferLimitsConfig clientBufferLimitsDefaults[CLIENT_TYPE_OBUF_COUNT] = {
{0, 0, 0}, /* normal */
@@ -2226,7 +2232,7 @@ standardConfig configs[] = {
createIntConfig("tls-session-cache-timeout", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.tls_ctx_config.session_cache_timeout, 300, INTEGER_CONFIG, NULL, updateTlsCfgInt),
createBoolConfig("tls-cluster", NULL, MODIFIABLE_CONFIG, server.tls_cluster, 0, NULL, updateTlsCfgBool),
createBoolConfig("tls-replication", NULL, MODIFIABLE_CONFIG, server.tls_replication, 0, NULL, updateTlsCfgBool),
- createBoolConfig("tls-auth-clients", NULL, MODIFIABLE_CONFIG, server.tls_auth_clients, 1, NULL, NULL),
+ createEnumConfig("tls-auth-clients", NULL, MODIFIABLE_CONFIG, tls_auth_clients_enum, server.tls_auth_clients, TLS_CLIENT_AUTH_YES, NULL, NULL),
createBoolConfig("tls-prefer-server-ciphers", NULL, MODIFIABLE_CONFIG, server.tls_ctx_config.prefer_server_ciphers, 0, NULL, updateTlsCfgBool),
createBoolConfig("tls-session-caching", NULL, MODIFIABLE_CONFIG, server.tls_ctx_config.session_caching, 1, NULL, updateTlsCfgBool),
createStringConfig("tls-cert-file", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.cert_file, NULL, NULL, updateTlsCfg),
diff --git a/src/server.h b/src/server.h
index afd020759..745ae76a2 100644
--- a/src/server.h
+++ b/src/server.h
@@ -358,6 +358,11 @@ typedef long long ustime_t; /* microsecond time type. */
#define REPL_DISKLESS_LOAD_WHEN_DB_EMPTY 1
#define REPL_DISKLESS_LOAD_SWAPDB 2
+/* TLS Client Authentication */
+#define TLS_CLIENT_AUTH_NO 0
+#define TLS_CLIENT_AUTH_YES 1
+#define TLS_CLIENT_AUTH_OPTIONAL 2
+
/* Sets operations codes */
#define SET_OP_UNION 0
#define SET_OP_DIFF 1
diff --git a/src/tls.c b/src/tls.c
index 8b2bb58e1..07d1775f8 100644
--- a/src/tls.c
+++ b/src/tls.c
@@ -342,8 +342,16 @@ connection *connCreateAcceptedTLS(int fd, int require_auth) {
conn->c.fd = fd;
conn->c.state = CONN_STATE_ACCEPTING;
- if (!require_auth) {
- SSL_set_verify(conn->ssl, SSL_VERIFY_NONE, NULL);
+ switch (require_auth) {
+ case TLS_CLIENT_AUTH_NO:
+ SSL_set_verify(conn->ssl, SSL_VERIFY_NONE, NULL);
+ break;
+ case TLS_CLIENT_AUTH_OPTIONAL:
+ SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, NULL);
+ break;
+ default: /* TLS_CLIENT_AUTH_YES, also fall-secure */
+ SSL_set_verify(conn->ssl, SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL);
+ break;
}
SSL_set_fd(conn->ssl, conn->c.fd);
diff --git a/tests/unit/tls.tcl b/tests/unit/tls.tcl
index 2b04590cd..bb5b6d034 100644
--- a/tests/unit/tls.tcl
+++ b/tests/unit/tls.tcl
@@ -21,7 +21,19 @@ start_server {tags {"tls"}} {
catch {$s PING} e
assert_match {PONG} $e
+ r CONFIG SET tls-auth-clients optional
+
+ 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
+
+ set s [redis [srv 0 host] [srv 0 port]]
+ ::tls::import [$s channel]
+ catch {$s PING} e
+ assert_match {*error*} $e
}
test {TLS: Verify tls-protocols behaves as expected} {