diff --git a/TLS.md b/TLS.md index 73c853d122..79af716ae4 100644 --- a/TLS.md +++ b/TLS.md @@ -37,14 +37,14 @@ invoked so sample certificates/keys are available): For TLS built-in mode: ./src/redis-server --tls-port 6379 --port 0 \ - --tls-cert-file ./tests/tls/redis.crt \ - --tls-key-file ./tests/tls/redis.key \ + --tls-cert-file ./tests/tls/server.crt \ + --tls-key-file ./tests/tls/server.key \ --tls-ca-cert-file ./tests/tls/ca.crt For TLS module mode: ./src/redis-server --tls-port 6379 --port 0 \ - --tls-cert-file ./tests/tls/redis.crt \ - --tls-key-file ./tests/tls/redis.key \ + --tls-cert-file ./tests/tls/server.crt \ + --tls-key-file ./tests/tls/server.key \ --tls-ca-cert-file ./tests/tls/ca.crt \ --loadmodule src/redis-tls.so @@ -58,6 +58,12 @@ To connect to this Redis server with `redis-cli`: This will disable TCP and enable TLS on port 6379. It's also possible to have both TCP and TLS available, but you'll need to assign different ports. +The server presents `server.crt` because `redis-cli` verifies, by default, that +the server certificate carries the address it connects to: `server.crt` carries +`127.0.0.1`, `::1` and `localhost` in its Subject Alternative Name, while +`redis.crt` carries no identity at all and would be rejected with an +explanation. See "Client tools" under "Peer certificate verification" below. + Note why that `redis-cli` invocation passes a certificate of its own: by default clients on a TLS port must present one that verifies against the configured CA, replicas connecting to a master included. `tls-auth-clients no` accepts no client @@ -96,11 +102,104 @@ only a cluster node opens a cluster bus port. Peer certificate verification ----------------------------- -By default, Redis TLS validates only that a peer certificate **chains to the -trusted CA** (`tls-ca-cert-file` / `tls-ca-cert-dir`) and is not expired. It does -**not** verify that the certificate belongs to the specific peer being contacted. +Two different rules apply, depending on who is connecting: -### Trust model and the CA assumption +* **Client tools** — `redis-cli`, `redis-benchmark` and Sentinel's connections to + the instances it monitors — verify the server certificate's **identity** by + default, the way a browser or `curl` does: besides chaining to the trusted CA, + the certificate must carry the name or address the tool connected to. See + "Client tools" below. +* **Server-to-server** connections — replication, the cluster bus and `MIGRATE` — + validate by default only that the peer certificate **chains to the trusted CA** + (`tls-ca-cert-file` / `tls-ca-cert-dir`) and is not expired, and verify an + identity only when `tls-expected-peer-name` is configured. The rest of this + section explains that model. + +### Client tools: redis-cli, redis-benchmark and Sentinel + +By default these tools require the server certificate to carry the **expected +identity** in its Subject Alternative Name (SAN) — the equivalent of libpq's +`sslmode=verify-full`. Without this, any certificate issued by a trusted CA is +accepted whatever identity it carries, so under a shared or public CA the holder +of an unrelated certificate could impersonate the server and capture the `AUTH` +credentials these tools send. + +The identity is taken from the most explicit source available: + +1. `--tls-expected-peer-name ` (redis-cli, redis-benchmark) or the + `tls-expected-peer-name` config (Sentinel): an explicit identity, independent + of the address dialed — a space-separated list of DNS names, any of which + may match. The tools' flag additionally accepts a single IP address; the + server config takes DNS names only (see its section below). This fits + deployments where every node's certificate carries a shared cluster-identity + SAN, including cluster mode where node addresses are learned from the + cluster itself. +2. `--sni `: the declared logical name of the server. When you connect to + an address but the server is known by a name, `--sni` both announces that + name to the server and is the identity the certificate is verified against. +3. The host that was dialed (`-h` for the tools; the configured or announced + address of an instance for Sentinel): an IP address is matched against the + certificate's `IP` SAN entries, a DNS name against its `DNS` entries (with + the Common Name as a fallback when the certificate has no DNS entries). + +Sentinel has no `--sni` equivalent, so for it the choice is simply: the configured +`tls-expected-peer-name` if there is one, otherwise the instance's address. + +Separately from which identity is verified, the dialed host — for Sentinel, the +instance's address — is also sent to the server as SNI whenever it is a DNS name. +Redis itself serves a single certificate and ignores SNI, so this matters for a +TLS proxy or load balancer in front of Redis, which may route the connection by +SNI or present the certificate issued for that name. + +The three verification levels, and how to select them: + +| Level | redis-cli / redis-benchmark | Sentinel | Verifies | +|---|---|---|---| +| full (default) | — | `sentinel tls-verify-hostnames yes` | CA chain **and** identity | +| chain only | `--no-verify-hostname` | `sentinel tls-verify-hostnames no` | CA chain | +| none | `--insecure` | — | nothing | + +`--no-verify-hostname` cannot be combined with `--tls-expected-peer-name`, nor can +`--insecure`: the former contradicts an explicit identity, the latter verifies +nothing at all, so an identity could never be checked. + +**Certificate requirements.** A certificate must carry the identity the tools +verify. In particular an address that is dialed as an **IP** — the common case, +and Sentinel's default since `resolve-hostnames` defaults to `no` — must appear +as an `IP:` SAN entry; there is no fallback to the Common Name for IP addresses. +Certificates issued for DNS names only, or with no SAN at all, fail with an +explanation naming the expected identity and the option to relax the check. When +upgrading, review the certificates of every instance the tools and Sentinel +connect to; `utils/gen-test-certs.sh` shows how to issue certificates carrying +IP and DNS identities. Alternatively, issue every instance's certificate with a +shared name and pin it with `tls-expected-peer-name`, which replaces the address +check. + +Sentinel logs an identity mismatch as a `-tls-identity-mismatch` event at WARNING +level (rate limited per link), naming the expected identity and the remedies, +since a Sentinel whose instance certificates lack their addresses would otherwise +lose every link with nothing but DEBUG-level reconnection events to show for it. +`sentinel tls-verify-hostnames` can be changed at runtime with +`SENTINEL CONFIG SET`; since the identity is checked when a link is established, +the change applies to connections made from then on, not to links already up. +It governs only the address check: a configured `tls-expected-peer-name` is +verified whether it is `yes` or `no`. + +A name beginning with a dot given to `--tls-expected-peer-name` is a parent-domain +pattern (see the note under `tls-expected-peer-name` below); the tools accept it +and print a warning about its wider scope. + +Unix-socket connections have no host name or address to verify against, so the +identity check applies to them only when `--sni` or `--tls-expected-peer-name` +provides one. + +Identity verification uses the OpenSSL `X509_VERIFY_PARAM` host API, available +since OpenSSL 1.0.2, under the same `TLS_NO_PEER_NAME_VERIFICATION` build flag +described below. When compiled out, the tools verify the CA chain only and say so +once; an explicit `--tls-expected-peer-name` is refused rather than silently +ignored. + +### Server-to-server connections: trust model and the CA assumption This default is safe **only under a dedicated, per-cluster CA**: since that CA signs certificates exclusively for your own nodes, "chains to the CA" is @@ -157,6 +256,12 @@ Notes: * Multiple names may be supplied as a single space-separated value (quote it in the config file, e.g. `tls-expected-peer-name "a.example.com b.example.com"`); a match against any one of them succeeds. +* Only DNS names are accepted; an IP address is rejected at configuration time. + This option names peers by stable DNS identities, and OpenSSL holds a single + expected IP address that must match in addition to any host names, which does + not fit a list where any entry may match. To verify a peer by its address, use + Sentinel's `tls-verify-hostnames` or the identity check the tools perform by + default. * Wildcards are matched from the **certificate**, not from this option: a certificate whose SAN is a full-label wildcard (`*.example.com`) matches a concrete name configured here (`node.example.com`), while a partial-label diff --git a/redis.conf b/redis.conf index 70e73c3662..ed75cd07c9 100644 --- a/redis.conf +++ b/redis.conf @@ -313,6 +313,13 @@ tcp-keepalive 300 # Multiple names may be given as a single space-separated value (quote it); a # match against any one of them succeeds. # +# Only DNS names are accepted; an IP address is rejected when the configuration +# is loaded or set. This option names peers by stable DNS identities, and +# OpenSSL holds a single expected IP address that must match in addition to any +# host names, which does not fit a list where any entry may match. To verify an +# instance by its address, use Sentinel's tls-verify-hostnames or the identity +# check redis-cli and redis-benchmark perform by default. +# # Wildcards are matched from the certificate, not from this option: a certificate # whose SAN is a full-label wildcard (e.g. "*.example.com") matches a concrete # name configured here (e.g. "node.example.com"), while a partial-label wildcard diff --git a/sentinel.conf b/sentinel.conf index c7ce1cba73..a81ff4b80f 100644 --- a/sentinel.conf +++ b/sentinel.conf @@ -350,6 +350,33 @@ SENTINEL resolve-hostnames no # SENTINEL announce-hostnames no +# TLS +# +# Sentinel connects to the masters, replicas and other sentinels it monitors +# over TLS when tls-replication is enabled, using the standard tls-* options +# (tls-ca-cert-file, tls-cert-file, tls-key-file, ...). These connections carry +# the auth-pass / auth-user credentials, so besides validating that an instance's +# certificate chains to the trusted CA, Sentinel verifies by default -- unless +# tls-expected-peer-name is configured, see below -- that the certificate carries +# the address it connected to (the IP, or the hostname when resolve-hostnames / +# announced hostnames are in use) in its Subject Alternative Name. A certificate +# from the trusted CA that lacks that identity is rejected, the link stays down, +# and a -tls-identity-mismatch event is logged. +# +# In particular an instance connected to by IP must carry that IP as an "IP" +# SAN entry; there is no fallback to the certificate's Common Name for IP +# addresses. When tls-expected-peer-name is configured (DNS names only, see +# redis.conf), that name is verified instead of the address. +# +# Set this to "no" to skip the address check and verify only that certificates +# chain to the trusted CA, as earlier versions did; a configured +# tls-expected-peer-name is still verified. It can be changed at runtime with +# SENTINEL CONFIG SET; the identity is checked when a link is established, so a +# change applies to connections made from then on, not to links that are +# already up. +# +# SENTINEL tls-verify-hostnames yes + # When master_reboot_down_after_period is set to 0, Sentinel does not fail over # when receiving a -LOADING response from a master. This was the only supported # behavior before version 7.0. diff --git a/src/cli_common.c b/src/cli_common.c index d61072b5ea..aebc7acee6 100644 --- a/src/cli_common.c +++ b/src/cli_common.c @@ -25,6 +25,7 @@ #ifdef USE_OPENSSL #include #include +#include #include #endif @@ -33,8 +34,100 @@ char *redisGitSHA1(void); char *redisGitDirty(void); +/* Validate a space-separated list of expected TLS peer names as given to + * --tls-expected-peer-name, before any connection is attempted. Mirrors the + * server's isValidTlsExpectedPeerName(): spaces are the only separators, so any + * other whitespace would be handed to OpenSSL as part of a name and silently + * never match a real SAN. An IP address must be the sole name: OpenSSL keeps a + * single expected IP, and combining it with DNS names requires BOTH to match + * (X509_VERIFY_PARAM ANDs the two kinds), which is never what a + * match-any-of-these list means. A name beginning with a dot is legitimate + * OpenSSL syntax for a parent domain (it matches any subdomain at any depth); + * like the server, we accept it but report how many such entries there are so + * the caller can warn about the wider scope. */ +const char *cliValidateExpectedPeerNames(const char *names, int *parent_domain_entries) { + if (parent_domain_entries) *parent_domain_entries = 0; + + for (const char *p = names; *p; p++) { + if (*p != ' ' && isspace((unsigned char)*p)) + return "expected peer names must not contain whitespace other than " + "spaces separating names"; + } + + int count = 0, nnames = 0, nips = 0, ndots = 0; + sds *tokens = sdssplitlen(names, strlen(names), " ", 1, &count); + if (tokens == NULL) + return "out of memory while parsing expected peer names"; + + for (int i = 0; i < count; i++) { + if (sdslen(tokens[i]) == 0) continue; /* runs of spaces */ + nnames++; + if (tlsIsIPAddressLiteral(tokens[i])) nips++; + else if (tokens[i][0] == '.') ndots++; + } + sdsfreesplitres(tokens, count); + + if (nnames == 0) + return "expected peer names contain no usable name"; + if (nips > 0 && nnames > 1) + return "an IP address must be the only expected peer name"; + + if (parent_domain_entries) *parent_domain_entries = ndots; + return NULL; +} + +#if defined(USE_OPENSSL) && CLI_TLS_SUPPORTS_VERIFY_NAME +/* Bind the SSL object to the expected peer identity(-ies): the server + * certificate must carry one of the space-separated `names` (DNS names, or a + * single IP address literal; see tlsVerifyParamAddIdentity() for how each kind + * is matched) in addition to chaining to the trusted CA. Applied per SSL object + * rather than on the shared SSL_CTX: the context is a process-wide static, while + * the identity is a per-connection property (e.g. each node the cluster manager + * dials). The names have been validated by cliValidateExpectedPeerNames(). + * Returns REDIS_OK, or REDIS_ERR with *err set. */ +static int cliSSLSetVerifyNames(SSL *ssl, const char *names, const char **err) { + X509_VERIFY_PARAM *param = SSL_get0_param(ssl); + + int count = 0; + sds *tokens = sdssplitlen(names, strlen(names), " ", 1, &count); + if (tokens == NULL) { + *err = "Out of memory while parsing expected peer names"; + return REDIS_ERR; + } + + /* The first name replaces the expected host list, each subsequent one is + * appended, so a match against any listed name succeeds. */ + int applied = 0, first = 1, ok = 1; + for (int i = 0; i < count; i++) { + if (sdslen(tokens[i]) == 0) continue; /* runs of spaces */ + int r = tlsVerifyParamAddIdentity(param, tokens[i], sdslen(tokens[i]), first); + first = 0; + if (r != 1) { + ok = 0; + break; + } + applied++; + } + sdsfreesplitres(tokens, count); + + if (!ok || applied == 0) { + *err = "Failed to set the expected peer certificate name(s)"; + return REDIS_ERR; + } + return REDIS_OK; +} +#endif + /* Wrapper around redisSecureConnection to avoid hiredis_ssl dependencies if * not building with TLS support. + * + * Besides the CA chain, the server certificate's identity is verified by + * default (the equivalent of libpq's "verify-full"): it must carry the name + * given to --sni, or else the host that was dialed (an IP or a DNS name), or + * the explicit --tls-expected-peer-name. --no-verify-hostname drops back to + * chain-only validation ("verify-ca"), --insecure to none. A unix socket has no + * name to verify against, so only --sni or --tls-expected-peer-name can bind + * such a connection to an identity. */ int cliSecureConnection(redisContext *c, cliSSLconfig config, const char **err) { #ifdef USE_OPENSSL @@ -96,13 +189,83 @@ int cliSecureConnection(redisContext *c, cliSSLconfig config, const char **err) return REDIS_ERR; } - if (config.sni && !SSL_set_tlsext_host_name(ssl, config.sni)) { + /* SNI: the explicit --sni value, else the dialed host when it is a DNS name + * (SNI cannot carry an IP literal). Redis itself serves a single certificate + * and ignores SNI; announcing the name is for a TLS proxy or load balancer + * in front of it, which may route by SNI or present the certificate issued + * for that name -- the identity verified below. c->tcp.host is NULL for + * unix-socket connections. */ + const char *sni = config.sni; + if (sni == NULL && c->tcp.host && !tlsIsIPAddressLiteral(c->tcp.host)) + sni = c->tcp.host; + if (sni && !SSL_set_tlsext_host_name(ssl, sni)) { *err = "Failed to configure SNI"; SSL_free(ssl); return REDIS_ERR; } - return redisInitiateSSL(c, ssl); + /* Identity verification. Most explicit source wins: the configured + * expected name(s), then the declared server name, then the dialed host. + * Skipped entirely under --insecure (nothing is verified at all, so a name + * check could not run) or --no-verify-hostname. */ + const char *identity = NULL; + if (!config.skip_cert_verify && !config.skip_hostname_verify) { + identity = config.expected_peer_name; + if (identity == NULL) identity = config.sni; + if (identity == NULL) identity = c->tcp.host; + if (identity && identity[0] == '\0') identity = NULL; + } + if (identity) { +#if CLI_TLS_SUPPORTS_VERIFY_NAME + if (cliSSLSetVerifyNames(ssl, identity, err) != REDIS_OK) { + SSL_free(ssl); + return REDIS_ERR; + } +#else + /* Compiled out (TLS_NO_PEER_NAME_VERIFICATION). An explicit request + * that cannot be honored must fail, not silently degrade; the implicit + * default degrades to chain-only validation, but says so once, like the + * server does for tls-expected-peer-name. */ + if (config.expected_peer_name) { + *err = "Peer certificate name verification is disabled in this build " + "(TLS_NO_PEER_NAME_VERIFICATION), so --tls-expected-peer-name " + "cannot be honored"; + SSL_free(ssl); + return REDIS_ERR; + } + static int warned = 0; + if (!warned) { + warned = 1; + fprintf(stderr, "Warning: this build cannot verify the server " + "certificate's identity (TLS_NO_PEER_NAME_VERIFICATION); only " + "the CA chain is verified.\n"); + } + identity = NULL; +#endif + } + + if (redisInitiateSSL(c, ssl) == REDIS_OK) return REDIS_OK; + + /* Handshake failed. hiredis does not take ownership of the SSL object in + * that case, so it is ours to free; before that, turn an identity mismatch + * into an actionable message instead of OpenSSL's bare "certificate verify + * failed". (Static buffer: connections are established from one thread, + * as the static ssl_ctx above already assumes.) */ +#if CLI_TLS_SUPPORTS_VERIFY_NAME + long vres = SSL_get_verify_result(ssl); + if (identity && (vres == X509_V_ERR_HOSTNAME_MISMATCH || vres == X509_V_ERR_IP_ADDRESS_MISMATCH)) { + static char errbuf[512]; + snprintf(errbuf, sizeof(errbuf), + "%s: the server certificate does not carry the expected identity '%s'. " + "Reissue it with that name in its Subject Alternative Name, verify a " + "different name with --tls-expected-peer-name, or verify only the CA " + "chain with --no-verify-hostname", + X509_verify_cert_error_string(vres), identity); + *err = errbuf; + } +#endif + SSL_free(ssl); + return REDIS_ERR; error: SSL_CTX_free(ssl_ctx); diff --git a/src/cli_common.h b/src/cli_common.h index efe9bf23a7..5857cd2ecf 100644 --- a/src/cli_common.h +++ b/src/cli_common.h @@ -20,6 +20,19 @@ #define CLI_TLS_SUPPORTS_GROUPS 0 #endif +/* Peer identity helpers shared with tls.c and sentinel.c: the address-literal + * test (always available) and, with OpenSSL, the build gate and the function + * that binds an identity to the verification parameters. */ +#include "tlshelpers.h" + +/* Whether server certificate identity verification is available in this build + * (OpenSSL >= 1.0.2, not compiled out with TLS_NO_PEER_NAME_VERIFICATION). */ +#ifdef USE_OPENSSL +#define CLI_TLS_SUPPORTS_VERIFY_NAME CONN_TLS_SUPPORTS_VERIFY_NAME +#else +#define CLI_TLS_SUPPORTS_VERIFY_NAME 0 +#endif + typedef struct cliSSLconfig { /* Requested SNI, or NULL */ char *sni; @@ -39,6 +52,13 @@ typedef struct cliSSLconfig { char* ciphersuites; /* Preferred TLS named groups list, or NULL */ char* groups; + /* Verify only that the server certificate chains to the trusted CA, not + * that it carries the expected identity (--no-verify-hostname). */ + int skip_hostname_verify; + /* Space-separated expected peer certificate name(s) to verify instead of + * the name derived from --sni or the dialed host, or NULL + * (--tls-expected-peer-name). */ + char *expected_peer_name; } cliSSLconfig; @@ -53,6 +73,13 @@ typedef struct cliConnInfo { int cliSecureConnection(redisContext *c, cliSSLconfig config, const char **err); +/* Validate a space-separated list of expected TLS peer names as given to + * --tls-expected-peer-name. Returns NULL if valid, or an error message. When + * valid and parent_domain_entries is non-NULL, it receives the number of + * entries beginning with a dot (OpenSSL parent-domain patterns), so the caller + * can surface their wider scope. */ +const char *cliValidateExpectedPeerNames(const char *names, int *parent_domain_entries); + ssize_t cliWriteConn(redisContext *c, const char *buf, size_t buf_len); int cliSecureInit(void); diff --git a/src/commands/sentinel-config.json b/src/commands/sentinel-config.json index a406125a5d..22f6d4ea9f 100644 --- a/src/commands/sentinel-config.json +++ b/src/commands/sentinel-config.json @@ -44,6 +44,16 @@ } ] }, + "tls-verify-hostnames": { + "oneOf": [ + { + "const": "yes" + }, + { + "const": "no" + } + ] + }, "announce-ip": { "type": "string" }, diff --git a/src/config.c b/src/config.c index 882dd591f0..f32b1af970 100644 --- a/src/config.c +++ b/src/config.c @@ -3004,6 +3004,35 @@ static int isValidTlsExpectedPeerName(char *val, const char **err) { return 0; } + /* The option takes DNS names only. It names peers by stable DNS identities, + * and the OpenSSL verification parameters hold a single expected IP address + * that must match in addition to any host names (see + * tlsVerifyParamAddIdentity()), which does not fit a list where any entry + * may match. Refuse an IP address here so the mistake surfaces at + * configuration time rather than as a failed handshake. Verifying a peer by + * its address is what Sentinel's tls-verify-hostnames and the tools' default + * identity check are for. */ + int count = 0; + unsigned char addr_buf[sizeof(struct in6_addr)]; + sds *tokens = sdssplitlen(val, strlen(val), " ", 1, &count); + if (!tokens) { + *err = "tls-expected-peer-name could not be parsed"; + return 0; + } + for (int i = 0; i < count; i++) { + if (sdslen(tokens[i]) == 0) continue; /* runs of spaces */ + if (inet_pton(AF_INET, tokens[i], addr_buf) == 1 || + inet_pton(AF_INET6, tokens[i], addr_buf) == 1) + { + sdsfreesplitres(tokens, count); + *err = "tls-expected-peer-name accepts DNS names only; to verify a peer " + "by its IP address use Sentinel's tls-verify-hostnames or the " + "tools' default identity check"; + return 0; + } + } + sdsfreesplitres(tokens, count); + return 1; } diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index ad22323370..cc581aa8e9 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -235,7 +235,10 @@ static redisContext *getRedisContext(const char *ip, int port, } if (config.tls==1) { const char *err = NULL; - if (cliSecureConnection(ctx, config.sslconfig, &err) == REDIS_ERR && err) { + if (cliSecureConnection(ctx, config.sslconfig, &err) == REDIS_ERR) { + /* A handshake failure (e.g. certificate verification) is reported + * on the context rather than through err. */ + if (err == NULL) err = ctx->errstr; fprintf(stderr, "Could not negotiate a TLS connection: %s\n", err); goto cleanup; } @@ -583,6 +586,14 @@ static void writeHandler(aeEventLoop *el, int fd, void *privdata, int mask) { * is actually writable. At worst we get EAGAIN. */ const ssize_t nwritten = cliWriteConn(c->context,ptr,writeLen); if (nwritten != writeLen) { + if (nwritten == -1 && c->context->err) { + /* Fatal connection error, e.g. a failed TLS handshake + * (certificate verification happens during the first + * writes on a non-blocking context). cliWriteConn reports + * it as EAGAIN, which would otherwise spin forever here. */ + fprintf(stderr,"Error: %s\n",c->context->errstr); + exit(1); + } if (nwritten == -1 && errno != EAGAIN) { if (errno != EPIPE) fprintf(stderr, "Error writing to the server: %s\n", strerror(errno)); @@ -660,7 +671,11 @@ static client createClient(char *cmd, size_t len, client from, int thread_id) { } if (config.tls==1) { const char *err = NULL; - if (cliSecureConnection(c->context, config.sslconfig, &err) == REDIS_ERR && err) { + if (cliSecureConnection(c->context, config.sslconfig, &err) == REDIS_ERR) { + /* A handshake failure is reported on the context rather than + * through err (on this non-blocking context the handshake only + * begins here; a later failure surfaces in writeHandler). */ + if (err == NULL) err = c->context->errstr; fprintf(stderr, "Could not negotiate a TLS connection: %s\n", err); exit(1); } @@ -1524,6 +1539,11 @@ int parseOptions(int argc, char **argv) { config.sslconfig.cacert = strdup(argv[++i]); } else if (!strcmp(argv[i],"--insecure")) { config.sslconfig.skip_cert_verify = 1; + } else if (!strcmp(argv[i],"--no-verify-hostname")) { + config.sslconfig.skip_hostname_verify = 1; + } else if (!strcmp(argv[i],"--tls-expected-peer-name")) { + if (lastarg) goto invalid; + config.sslconfig.expected_peer_name = strdup(argv[++i]); } else if (!strcmp(argv[i],"--cert")) { if (lastarg) goto invalid; config.sslconfig.cert = strdup(argv[++i]); @@ -1561,13 +1581,24 @@ invalid: usage: tls_usage = #ifdef USE_OPENSSL -" --tls Establish a secure TLS connection.\n" -" --sni Server name indication for TLS.\n" +" --tls Establish a secure TLS connection. The server certificate\n" +" must chain to the trusted CA and carry the expected\n" +" identity: the name given to --sni, or else the host that\n" +" was dialed (as an IP or DNS SAN).\n" +" --sni Server name indication for TLS. Also the identity the\n" +" server certificate is verified against.\n" " --cacert CA Certificate file to verify with.\n" " --cacertdir Directory where trusted CA certificates are stored.\n" " If neither cacert nor cacertdir are specified, the default\n" " system-wide trusted root certs configuration will apply.\n" " --insecure Allow insecure TLS connection by skipping cert validation.\n" +" --no-verify-hostname\n" +" Verify only that the server certificate chains to the\n" +" trusted CA, not that it carries the expected identity.\n" +" --tls-expected-peer-name \n" +" Verify the server certificate against these identities\n" +" instead of the dialed host or SNI: a space-separated list\n" +" of DNS names (any match succeeds) or a single IP address.\n" " --cert Client certificate to authenticate with.\n" " --key Private key file to authenticate with.\n" " --tls-ciphers Sets the list of preferred ciphers (TLSv1.2 and below)\n" @@ -1772,6 +1803,35 @@ int main(int argc, char **argv) { tag = ""; #ifdef USE_OPENSSL + /* Identity verification options. --no-verify-hostname drops to chain-only + * validation, so it contradicts an explicit expected identity; --insecure + * verifies nothing at all, so an expected identity could never be checked. */ + if (config.sslconfig.expected_peer_name) { + if (config.sslconfig.skip_hostname_verify) { + fprintf(stderr, "Options --tls-expected-peer-name and --no-verify-hostname " + "are mutually exclusive.\n"); + exit(1); + } + if (config.sslconfig.skip_cert_verify) { + fprintf(stderr, "Option --insecure cannot be combined with " + "--tls-expected-peer-name.\n"); + exit(1); + } + int parent_domains = 0; + const char *name_err = cliValidateExpectedPeerNames(config.sslconfig.expected_peer_name, + &parent_domains); + if (name_err) { + fprintf(stderr, "Invalid --tls-expected-peer-name: %s.\n", name_err); + exit(1); + } + if (parent_domains) { + fprintf(stderr, "Warning: --tls-expected-peer-name has %d entr%s beginning " + "with a dot; such an entry is a parent-domain pattern that " + "matches any subdomain at any depth, not a single host.\n", + parent_domains, parent_domains == 1 ? "y" : "ies"); + } + } + if (config.tls) { cliSecureInit(); } diff --git a/src/redis-cli.c b/src/redis-cli.c index 460750b4b0..c94f14ff28 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -1752,7 +1752,11 @@ static int cliConnect(int flags) { if (!context->err && config.tls) { const char *err = NULL; if (cliSecureConnection(context, config.sslconfig, &err) == REDIS_ERR && err) { - fprintf(stderr, "Could not negotiate a TLS connection: %s\n", err); + /* Honor CC_QUIET like the generic failure path below: probe + * connections would otherwise report the same failure that the + * real connection attempt reports right after. */ + if (!(flags & CC_QUIET)) + fprintf(stderr, "Could not negotiate a TLS connection: %s\n", err); redisFree(context); context = NULL; return REDIS_ERR; @@ -3091,6 +3095,10 @@ static int parseOptions(int argc, char **argv) { config.sslconfig.ciphers = argv[++i]; } else if (!strcmp(argv[i],"--insecure")) { config.sslconfig.skip_cert_verify = 1; + } else if (!strcmp(argv[i],"--no-verify-hostname")) { + config.sslconfig.skip_hostname_verify = 1; + } else if (!strcmp(argv[i],"--tls-expected-peer-name") && !lastarg) { + config.sslconfig.expected_peer_name = argv[++i]; #ifdef TLS1_3_VERSION } else if (!strcmp(argv[i],"--tls-ciphersuites") && !lastarg) { config.sslconfig.ciphersuites = argv[++i]; @@ -3178,6 +3186,35 @@ static int parseOptions(int argc, char **argv) { exit(1); } + /* Identity verification options. --no-verify-hostname drops to chain-only + * validation, so it contradicts an explicit expected identity; --insecure + * verifies nothing at all, so an expected identity could never be checked. */ + if (config.sslconfig.expected_peer_name) { + if (config.sslconfig.skip_hostname_verify) { + fprintf(stderr, "Options --tls-expected-peer-name and --no-verify-hostname " + "are mutually exclusive.\n"); + exit(1); + } + if (config.sslconfig.skip_cert_verify) { + fprintf(stderr, "Option --insecure cannot be combined with " + "--tls-expected-peer-name.\n"); + exit(1); + } + int parent_domains = 0; + const char *name_err = cliValidateExpectedPeerNames(config.sslconfig.expected_peer_name, + &parent_domains); + if (name_err) { + fprintf(stderr, "Invalid --tls-expected-peer-name: %s.\n", name_err); + exit(1); + } + if (parent_domains) { + fprintf(stderr, "Warning: --tls-expected-peer-name has %d entr%s beginning " + "with a dot; such an entry is a parent-domain pattern that " + "matches any subdomain at any depth, not a single host.\n", + parent_domains, parent_domains == 1 ? "y" : "ies"); + } + } + return i; } @@ -3199,13 +3236,24 @@ static void usage(int err) { FILE *target = err ? stderr: stdout; const char *tls_usage = #ifdef USE_OPENSSL -" --tls Establish a secure TLS connection.\n" -" --sni Server name indication for TLS.\n" +" --tls Establish a secure TLS connection. The server certificate\n" +" must chain to the trusted CA and carry the expected\n" +" identity: the name given to --sni, or else the host that\n" +" was dialed (as an IP or DNS SAN).\n" +" --sni Server name indication for TLS. Also the identity the\n" +" server certificate is verified against.\n" " --cacert CA Certificate file to verify with.\n" " --cacertdir Directory where trusted CA certificates are stored.\n" " If neither cacert nor cacertdir are specified, the default\n" " system-wide trusted root certs configuration will apply.\n" " --insecure Allow insecure TLS connection by skipping cert validation.\n" +" --no-verify-hostname\n" +" Verify only that the server certificate chains to the\n" +" trusted CA, not that it carries the expected identity.\n" +" --tls-expected-peer-name \n" +" Verify the server certificate against these identities\n" +" instead of the dialed host or SNI: a space-separated list\n" +" of DNS names (any match succeeds) or a single IP address.\n" " --cert Client certificate to authenticate with.\n" " --key Private key file to authenticate with.\n" " --tls-ciphers Sets the list of preferred ciphers (TLSv1.2 and below)\n" diff --git a/src/sentinel.c b/src/sentinel.c index 372e4b6406..4e58d40016 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -13,6 +13,7 @@ #if USE_OPENSSL == 1 /* BUILD_YES */ #include "openssl/ssl.h" #include "hiredis_ssl.h" +#include "tlshelpers.h" #endif #include "async.h" @@ -27,6 +28,9 @@ extern char **environ; #if USE_OPENSSL == 1 /* BUILD_YES */ extern SSL_CTX *redis_tls_ctx; extern SSL_CTX *redis_tls_client_ctx; +/* Peer identity helpers from tls.c (linked in with USE_OPENSSL == 1). */ +extern int tlsSetVerifyName(SSL *ssl, const char *names); +extern int tlsSetVerifyIdentity(SSL *ssl, const char *name); #endif #define REDIS_SENTINEL_PORT 26379 @@ -85,6 +89,8 @@ static mstime_t sentinel_default_failover_timeout = 60*3*1000; #define SENTINEL_DEFAULT_DENY_SCRIPTS_RECONFIG 1 #define SENTINEL_DEFAULT_RESOLVE_HOSTNAMES 0 #define SENTINEL_DEFAULT_ANNOUNCE_HOSTNAMES 0 +#define SENTINEL_DEFAULT_TLS_VERIFY_HOSTNAMES 1 +#define SENTINEL_TLS_IDENTITY_WARN_PERIOD 60000 /* Rate limit for identity-mismatch warnings, ms. */ /* Failover machine different states. */ #define SENTINEL_FAILOVER_STATE_NONE 0 /* No failover in progress. */ @@ -157,6 +163,12 @@ typedef struct instanceLink { if the link is idle and must be reconnected. */ mstime_t last_reconn_time; /* Last reconnection attempt performed when the link was down. */ + mstime_t last_tls_identity_warn; /* Last time a TLS identity mismatch was + logged for this link (rate limiting). */ + int tls_identity_err; /* X509 error recorded by sentinelTLSVerifyCallback() + when a certificate on this link failed the identity + check, 0 if none. Reported through a live instance, + and reset, by sentinelReportTLSIdentityMismatch(). */ } instanceLink; typedef struct sentinelRedisInstance { @@ -255,6 +267,8 @@ struct sentinelState { char *sentinel_auth_user; /* Username for ACLs AUTH against other sentinel. */ int resolve_hostnames; /* Support use of hostnames, assuming DNS is well configured. */ int announce_hostnames; /* Announce hostnames instead of IPs when we have them. */ + int tls_verify_hostnames; /* Verify that each instance's TLS certificate + carries the address we connect to. */ } sentinel; /* A script execution job. */ @@ -455,7 +469,8 @@ const char *preMonitorCfgName[] = { "current-epoch", "myid", "resolve-hostnames", - "announce-hostnames" + "announce-hostnames", + "tls-verify-hostnames" }; /* Returns 1 if the string contains control characters (0x00-0x1F or 0x7F), @@ -505,6 +520,7 @@ void initSentinel(void) { sentinel.sentinel_auth_user = NULL; sentinel.resolve_hostnames = SENTINEL_DEFAULT_RESOLVE_HOSTNAMES; sentinel.announce_hostnames = SENTINEL_DEFAULT_ANNOUNCE_HOSTNAMES; + sentinel.tls_verify_hostnames = SENTINEL_DEFAULT_TLS_VERIFY_HOSTNAMES; memset(sentinel.myid,0,sizeof(sentinel.myid)); server.sentinel_config = NULL; } @@ -1027,6 +1043,8 @@ instanceLink *createInstanceLink(void) { link->cc_conn_time = 0; link->pc_conn_time = 0; link->last_reconn_time = 0; + link->last_tls_identity_warn = 0; + link->tls_identity_err = 0; link->pc_last_activity = 0; /* We set the act_ping_time to "now" even if we actually don't have yet * a connection with the node, nor we sent a ping. @@ -2010,6 +2028,11 @@ const char *sentinelHandleConfiguration(char **argv, int argc) { if ((sentinel.announce_hostnames = yesnotoi(argv[1])) == -1) { return "Please specify yes or no for the announce-hostnames option."; } + } else if (!strcasecmp(argv[0],"tls-verify-hostnames") && argc == 2) { + /* tls-verify-hostnames */ + if ((sentinel.tls_verify_hostnames = yesnotoi(argv[1])) == -1) { + return "Please specify yes or no for the tls-verify-hostnames option."; + } } else if (!strcasecmp(argv[0],"master-reboot-down-after-period") && argc == 3) { /* master-reboot-down-after-period */ ri = sentinelGetMasterByName(argv[1]); @@ -2058,6 +2081,12 @@ void rewriteConfigSentinelOption(struct rewriteConfigState *state) { rewriteConfigRewriteLine(state,"sentinel announce-hostnames",line, sentinel.announce_hostnames != SENTINEL_DEFAULT_ANNOUNCE_HOSTNAMES); + /* sentinel tls-verify-hostnames. */ + line = sdscatprintf(sdsempty(), "sentinel tls-verify-hostnames %s", + sentinel.tls_verify_hostnames ? "yes" : "no"); + rewriteConfigRewriteLine(state,"sentinel tls-verify-hostnames",line, + sentinel.tls_verify_hostnames != SENTINEL_DEFAULT_TLS_VERIFY_HOSTNAMES); + /* For every master emit a "sentinel monitor" config entry. */ dictInitIterator(&di, sentinel.masters); while((de = dictNext(&di)) != NULL) { @@ -2384,18 +2413,135 @@ void sentinelSetClientName(sentinelRedisInstance *ri, redisAsyncContext *c, char } } -static int instanceLinkNegotiateTLS(redisAsyncContext *context) { +#if USE_OPENSSL == 1 /* BUILD_YES */ +#if CONN_TLS_SUPPORTS_VERIFY_NAME +/* The identity an instance's certificate is verified against, or NULL when no + * identity is verified -- used to name it in the mismatch event below. The + * configured tls-expected-peer-name wins: it is an + * identity anchored in local configuration that every instance carries. + * Otherwise it is the address the instance is known by -- the hostname the + * operator configured or the instance announced, or its IP -- which is what a + * TLS client normally verifies. Unlike redis-cli, Sentinel has no separate SNI + * setting: the address returned here is also what the link sends as SNI when + * it is a DNS name, so there is no distinct server name to prefer over it. */ +static const char *sentinelInstanceExpectedTLSIdentity(sentinelRedisInstance *ri) { + const char *name = server.tls_ctx_config.expected_peer_name; + if (name && name[0]) return name; + if (sentinel.tls_verify_hostnames) return ri->addr->hostname; + return NULL; +} + +/* OpenSSL verify callback installed on every instance link. It changes no + * verdict -- the handshake still fails on a mismatch -- it only records an + * identity mismatch on the link, for sentinelReportTLSIdentityMismatch() to + * report: otherwise the failure would surface as nothing more than a + * DEBUG-level link reconnection event, and a Sentinel monitoring instances + * whose certificates lack their address would go dark silently. + * + * The callback is handed the link, never an instance. Links to other + * Sentinels are shared between the masters that know them, and the instance + * that negotiated a connection can be released -- SENTINEL REMOVE or RESET of + * its master, an address switch -- while the handshake is still pending, with + * the link kept alive by another master's instance. The link does outlive the + * connection: it owns the hiredis context that owns this SSL object, and + * releaseInstanceLink() frees the contexts before the link itself, just as the + * contexts' own data pointer relies on. */ +static int sentinelTLSVerifyCallback(int preverify_ok, X509_STORE_CTX *store_ctx) { + if (preverify_ok) return 1; + + int err = X509_STORE_CTX_get_error(store_ctx); + if (err != X509_V_ERR_HOSTNAME_MISMATCH && err != X509_V_ERR_IP_ADDRESS_MISMATCH) + return preverify_ok; + + SSL *ssl = X509_STORE_CTX_get_ex_data(store_ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); + instanceLink *link = ssl ? SSL_get_app_data(ssl) : NULL; + if (link) link->tls_identity_err = err; + return preverify_ok; +} +#endif +#endif + +/* Report the TLS identity mismatch that sentinelTLSVerifyCallback() recorded + * on this instance's link, if any, then clear it. Called from + * sentinelReconnectInstance(), which runs for every instance holding a down + * link and so always has a live instance to name in the event -- unlike the + * callback, see there. Rate limited per link, since a down link is retried + * every second. */ +static void sentinelReportTLSIdentityMismatch(sentinelRedisInstance *ri) { + instanceLink *link = ri->link; + int err = link->tls_identity_err; + if (!err) return; + link->tls_identity_err = 0; +#if USE_OPENSSL == 1 && CONN_TLS_SUPPORTS_VERIFY_NAME + mstime_t now = mstime(); + if (now - link->last_tls_identity_warn < SENTINEL_TLS_IDENTITY_WARN_PERIOD) return; + link->last_tls_identity_warn = now; + const char *expected = sentinelInstanceExpectedTLSIdentity(ri); + sentinelEvent(LL_WARNING, "-tls-identity-mismatch", ri, + "%@ #%s: the certificate does not carry the expected identity '%s'. " + "Reissue it with that name in its Subject Alternative Name, set " + "tls-expected-peer-name to a name every instance carries, or set " + "'sentinel tls-verify-hostnames no' to verify only the CA chain.", + X509_verify_cert_error_string(err), expected ? expected : "?"); +#endif +} + +/* Negotiate TLS on a freshly connected link to `ri`. Besides the CA chain + * validation the shared context performs, the instance's certificate must + * carry the expected identity (see sentinelInstanceExpectedTLSIdentity): + * these links carry the credentials sentinelSendAuthIfNeeded() sends, so + * without that any certificate issued by the trusted CA could impersonate a + * master, replica or sentinel and capture them. Fails closed if the identity + * cannot be applied. */ +static int instanceLinkNegotiateTLS(redisAsyncContext *context, sentinelRedisInstance *ri) { #if USE_OPENSSL == 1 /* BUILD_YES */ if (!redis_tls_ctx) return C_ERR; SSL *ssl = SSL_new(redis_tls_client_ctx ? redis_tls_client_ctx : redis_tls_ctx); if (!ssl) return C_ERR; + /* Announce the instance's name (SNI) when it is one. Redis itself ignores + * SNI; this is for a TLS proxy or load balancer in front of an instance, + * which may route by SNI or present the certificate issued for that name, + * the identity verified below. SNI cannot carry an IP literal, and the + * address is an IP unless resolve-hostnames or announced hostnames are in + * use. */ + const char *addr = ri->addr->hostname; + if (!tlsIsIPAddressLiteral(addr)) SSL_set_tlsext_host_name(ssl, addr); + + const char *peer_name = server.tls_ctx_config.expected_peer_name; + int rc = C_OK, verifying = 0; + if (peer_name && peer_name[0]) { + rc = tlsSetVerifyName(ssl, peer_name); + verifying = 1; + } else if (sentinel.tls_verify_hostnames) { + rc = tlsSetVerifyIdentity(ssl, addr); + verifying = 1; + } + if (rc != C_OK) { + sentinelEvent(LL_WARNING, "-tls-identity", ri, + "%@ #Failed to set the expected TLS identity for this link"); + SSL_free(ssl); + return C_ERR; + } +#if CONN_TLS_SUPPORTS_VERIFY_NAME + if (verifying) { + /* The link, not the instance: see sentinelTLSVerifyCallback(). */ + SSL_set_app_data(ssl, ri->link); + SSL_set_verify(ssl, SSL_get_verify_mode(ssl), sentinelTLSVerifyCallback); + } +#else + /* Compiled out: tlsSetVerifyName()/tlsSetVerifyIdentity() have warned and + * set no expectation, so there is no mismatch for a callback to report. */ + UNUSED(verifying); +#endif + if (redisInitiateSSL(&context->c, ssl) == REDIS_ERR) { SSL_free(ssl); return C_ERR; } #else UNUSED(context); + UNUSED(ri); #endif return C_OK; } @@ -2413,6 +2559,10 @@ void sentinelReconnectInstance(sentinelRedisInstance *ri) { if (now - ri->link->last_reconn_time < sentinel_ping_period) return; ri->link->last_reconn_time = now; + /* A TLS identity mismatch that brought this link down is reported here, + * through an instance that is certainly alive. */ + sentinelReportTLSIdentityMismatch(ri); + /* Commands connection. */ if (link->cc == NULL) { @@ -2434,7 +2584,7 @@ void sentinelReconnectInstance(sentinelRedisInstance *ri) { if (!link->cc) { sentinelEvent(LL_DEBUG,"-cmd-link-reconnection",ri,"%@ #Failed to establish connection"); } else if (!link->cc->err && server.tls_replication && - (instanceLinkNegotiateTLS(link->cc) == C_ERR)) { + (instanceLinkNegotiateTLS(link->cc, ri) == C_ERR)) { sentinelEvent(LL_DEBUG,"-cmd-link-reconnection",ri,"%@ #Failed to initialize TLS"); instanceLinkCloseConnection(link,link->cc); } else if (link->cc->err) { @@ -2464,8 +2614,12 @@ void sentinelReconnectInstance(sentinelRedisInstance *ri) { if (!link->pc) { sentinelEvent(LL_DEBUG,"-pubsub-link-reconnection",ri,"%@ #Failed to establish connection"); } else if (!link->pc->err && server.tls_replication && - (instanceLinkNegotiateTLS(link->pc) == C_ERR)) { + (instanceLinkNegotiateTLS(link->pc, ri) == C_ERR)) { sentinelEvent(LL_DEBUG,"-pubsub-link-reconnection",ri,"%@ #Failed to initialize TLS"); + /* Close it like the command link above: a context left in link->pc + * is never attached to the event loop, yet counts as connected + * below, so the Pub/Sub link would never be retried. */ + instanceLinkCloseConnection(link,link->pc); } else if (link->pc->err) { sentinelEvent(LL_DEBUG,"-pubsub-link-reconnection",ri,"%@ #%s", link->pc->errstr); @@ -3218,6 +3372,7 @@ void sentinelConfigSetCommand(client *c) { "resolve-hostnames", "announce-port", "announce-hostnames", + "tls-verify-hostnames", "loglevel", NULL}; static dict *options_dict = NULL; @@ -3256,6 +3411,8 @@ void sentinelConfigSetCommand(client *c) { if ((yesnotoi(val->ptr)) == -1) goto badfmt; } else if (!strcasecmp(option, "announce-hostnames")) { if ((yesnotoi(val->ptr)) == -1) goto badfmt; + } else if (!strcasecmp(option, "tls-verify-hostnames")) { + if ((yesnotoi(val->ptr)) == -1) goto badfmt; } else if (!strcasecmp(option, "announce-port")) { if (getLongLongFromObject(val, &numval) == C_ERR || numval < 0 || numval > 65535) goto badfmt; @@ -3295,6 +3452,10 @@ void sentinelConfigSetCommand(client *c) { val = c->argv[++i]; numval = yesnotoi(val->ptr); sentinel.announce_hostnames = numval; + } else if (!strcasecmp(option, "tls-verify-hostnames") && moreargs > 0) { + val = c->argv[++i]; + numval = yesnotoi(val->ptr); + sentinel.tls_verify_hostnames = numval; } else if (!strcasecmp(option, "announce-ip") && moreargs > 0) { val = c->argv[++i]; if (sentinel.announce_ip) sdsfree(sentinel.announce_ip); @@ -3361,6 +3522,12 @@ void sentinelConfigGetCommand(client *c) { dictAdd(d, "announce-hostnames", NULL); matches++; } + if (stringmatch(pattern, "tls-verify-hostnames", 1) && !dictFind(d, "tls-verify-hostnames")) { + addReplyBulkCString(c,"tls-verify-hostnames"); + addReplyBulkCString(c,sentinel.tls_verify_hostnames ? "yes" : "no"); + dictAdd(d, "tls-verify-hostnames", NULL); + matches++; + } if (stringmatch(pattern, "announce-ip", 1) && !dictFind(d, "announce-ip")) { addReplyBulkCString(c,"announce-ip"); addReplyBulkCString(c,sentinel.announce_ip ? sentinel.announce_ip : ""); diff --git a/src/tls.c b/src/tls.c index 18b780be71..89ce615aa6 100644 --- a/src/tls.c +++ b/src/tls.c @@ -43,22 +43,11 @@ #define REDIS_TLS_PROTO_DEFAULT (REDIS_TLS_PROTO_TLSv1_2) #endif -/* Peer certificate name verification (tls-expected-peer-name) relies on the - * X509_VERIFY_PARAM host API, available since OpenSSL 1.0.2. Defining - * TLS_NO_PEER_NAME_VERIFICATION compiles the feature out on any OpenSSL version; - * it is also the required opt-out for building against an older OpenSSL, since - * building there without it is a hard error (so a build cannot accidentally lack - * the check). When compiled out and the option is set, each affected connection - * warns and proceeds (CA validation still applies). */ -#if defined(TLS_NO_PEER_NAME_VERIFICATION) -#define CONN_TLS_SUPPORTS_VERIFY_NAME 0 -#elif OPENSSL_VERSION_NUMBER >= 0x10002000L -#define CONN_TLS_SUPPORTS_VERIFY_NAME 1 -#else -#error "tls-expected-peer-name requires OpenSSL 1.0.2 or newer. Build against a \ -newer OpenSSL, or define TLS_NO_PEER_NAME_VERIFICATION to build without peer \ -certificate name verification." -#endif +/* CONN_TLS_SUPPORTS_VERIFY_NAME: whether peer certificate identity verification + * (tls-expected-peer-name) is available in this build; see tlshelpers.h. When it + * is compiled out and the option is set, each affected connection warns and + * proceeds (CA validation still applies). */ +#include "tlshelpers.h" /* TLS groups rely on SSL_CTX_set1_groups_list(), or on the * older SSL_CTX_set1_curves_list() name. Build failure is intentional when the @@ -999,32 +988,28 @@ static int connTLSAccept(connection *_conn, ConnectionCallbackFunc accept_handle * Returns C_OK on success, C_ERR if a name could not be applied or the value * contained no usable name. On a TLS_NO_PEER_NAME_VERIFICATION build (OpenSSL * < 1.0.2) it cannot enforce the name; it logs a warning and returns C_OK so the - * connection still proceeds (CA validation still applies). */ -static int tlsSetVerifyName(SSL *ssl, const char *names) { + * connection still proceeds (CA validation still applies). + * + * Not static: Sentinel applies tls-expected-peer-name to its hiredis-based + * links through it. Sentinel's TLS code is compiled only for linked-in TLS + * builds (USE_OPENSSL == 1), where this file is part of the server binary, so a + * module build leaves no undefined reference. */ +int tlsSetVerifyName(SSL *ssl, const char *names) { #if CONN_TLS_SUPPORTS_VERIFY_NAME - /* Use the X509_VERIFY_PARAM_* host API (available since OpenSSL 1.0.2) rather - * than the SSL_set1_host()/SSL_add1_host()/SSL_set_hostflags() convenience - * wrappers (introduced only in OpenSSL 1.1.0). This keeps certificate name - * verification available on every OpenSSL version that actually provides the - * hostname-checking machinery, matching the older versions the rest of this - * file still supports. */ X509_VERIFY_PARAM *param = SSL_get0_param(ssl); - /* Reject partial-label wildcards (e.g. "f*.example.com"); a full-label - * "*.example.com" still matches. */ - X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); - int count = 0; sds *tokens = sdssplitlen(names, strlen(names), " ", 1, &count); if (!tokens) return C_ERR; + /* The first name replaces the expected host list, each subsequent one is + * appended, so a match against any listed name succeeds. The config + * validator admits DNS names only, so the shared helper's IP branch is never + * taken here. */ int first = 1, ok = 1; for (int i = 0; i < count; i++) { if (sdslen(tokens[i]) == 0) continue; /* skip empty tokens from runs of spaces */ - /* First name via set1_host (replaces), each subsequent via add1_host - * (appends); a match against any listed name succeeds. */ - int r = first ? X509_VERIFY_PARAM_set1_host(param, tokens[i], 0) - : X509_VERIFY_PARAM_add1_host(param, tokens[i], 0); + int r = tlsVerifyParamAddIdentity(param, tokens[i], sdslen(tokens[i]), first); first = 0; if (r != 1) { ok = 0; break; } } @@ -1048,6 +1033,29 @@ static int tlsSetVerifyName(SSL *ssl, const char *names) { #endif } +/* Bind the SSL object to a single expected peer identity, the way a TLS client + * verifies the host it dials: an IP address literal is matched against the + * certificate's IP SAN entries (no fallback), anything else against its DNS SAN + * entries (with the CN as a fallback per OpenSSL rules). Used by Sentinel for + * the address of each instance it connects to. Returns C_OK on success, C_ERR + * if the name is empty or could not be applied. On a + * TLS_NO_PEER_NAME_VERIFICATION build it cannot enforce the identity; it logs a + * warning and returns C_OK so the connection proceeds with CA validation only, + * matching tlsSetVerifyName(). Not static, for the same reason as that one. */ +int tlsSetVerifyIdentity(SSL *ssl, const char *name) { + if (!name || !name[0]) return C_ERR; +#if CONN_TLS_SUPPORTS_VERIFY_NAME + X509_VERIFY_PARAM *param = SSL_get0_param(ssl); + return tlsVerifyParamAddIdentity(param, name, strlen(name), 1) == 1 ? C_OK : C_ERR; +#else + UNUSED(ssl); + serverLog(LL_WARNING, + "Peer certificate identity verification is disabled in this build " + "(TLS_NO_PEER_NAME_VERIFICATION); the identity of %s is not verified.", name); + return C_OK; +#endif +} + /* Set the expected peer name(s) to verify on an already-created SSL connection. * Used on the accepting side (e.g. the cluster bus) to verify the connecting * peer's client certificate. No-op when name is empty. */ diff --git a/src/tlshelpers.h b/src/tlshelpers.h new file mode 100644 index 0000000000..956999d669 --- /dev/null +++ b/src/tlshelpers.h @@ -0,0 +1,103 @@ +/* + * Copyright (c) 2026-Present, Redis Ltd. + * All rights reserved. + * + * Licensed under your choice of (a) the Redis Source Available License 2.0 + * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the + * GNU Affero General Public License v3 (AGPLv3). + */ + +#ifndef __REDIS_TLSHELPERS_H +#define __REDIS_TLSHELPERS_H + +/* Definitions shared by every translation unit that performs TLS peer + * certificate identity verification: tls.c (server-to-server connections and + * the cluster bus, possibly built as a module), sentinel.c (its hiredis-based + * links to the instances it monitors) and cli_common.c (redis-cli and + * redis-benchmark, which do not link tls.c). Everything is header-only, since + * these units share no object file. The OpenSSL-dependent part is compiled + * only when OpenSSL is available, so the header may be included + * unconditionally. */ + +#include +#include +#include /* AF_INET, AF_INET6 */ +#include /* struct in6_addr */ +#include /* inet_pton(); on FreeBSD it provides none of the above */ + +/* Return 1 if `name` is an IPv4 or IPv6 address literal, 0 otherwise. This + * decides whether an identity is matched against a certificate's IP or DNS + * entries, and whether it may be sent as SNI (which cannot carry an address). */ +static inline int tlsIsIPAddressLiteral(const char *name) { + unsigned char buf[sizeof(struct in6_addr)]; + return inet_pton(AF_INET, name, buf) == 1 || inet_pton(AF_INET6, name, buf) == 1; +} + +#ifdef USE_OPENSSL +#include +#include + +/* Peer certificate identity verification -- tls-expected-peer-name, Sentinel's + * tls-verify-hostnames and the tools' default identity check -- relies on the + * OpenSSL X509_VERIFY_PARAM host/IP API, available since OpenSSL 1.0.2. + * + * Building against an older OpenSSL is a hard error by default, so a build + * cannot silently lack the check. Defining TLS_NO_PEER_NAME_VERIFICATION + * compiles the feature out on any OpenSSL version: it is both the required + * opt-out for building against an older OpenSSL and a kill switch. Each caller + * documents what it does when compiled out; the common rule is that an implicit + * check degrades to CA chain validation with a warning, while an explicitly + * configured identity that cannot be honored is refused. */ +#if defined(TLS_NO_PEER_NAME_VERIFICATION) +#define CONN_TLS_SUPPORTS_VERIFY_NAME 0 +#elif OPENSSL_VERSION_NUMBER >= 0x10002000L +#define CONN_TLS_SUPPORTS_VERIFY_NAME 1 +#else +#error "TLS peer certificate identity verification requires OpenSSL 1.0.2 or \ +newer. Build against a newer OpenSSL, or define TLS_NO_PEER_NAME_VERIFICATION to \ +build without it." +#endif + +#if CONN_TLS_SUPPORTS_VERIFY_NAME +/* Bind one expected peer identity to OpenSSL's verification parameters. Every + * identity check -- tls-expected-peer-name, Sentinel's address check and the + * tools' default check -- goes through this one function, so OpenSSL's + * subtleties live here and nowhere else: + * + * - An IP address literal is matched against the certificate's IP SAN entries + * (set1_ip_asc). The host matcher never consults those, so an address handed + * to it would silently never match. Anything else is matched against the DNS + * SAN entries, with the CN as a fallback when there are none (set1_host / + * add1_host). OpenSSL keeps a single expected IP and, when host names are set + * as well, requires BOTH to match -- callers accepting a list of identities + * must therefore not mix the two kinds. + * - `replace` selects set1_host, which replaces the expected host list, over + * add1_host, which appends to it so that any listed host may match. + * - The length is passed explicitly: OpenSSL reads a length of 0 as "call + * strlen", but BoringSSL rejects a zero length outright. + * - Partial-label wildcards in the certificate ("f*.example.com") are rejected; + * full-label ones ("*.example.com") still match. + * + * The X509_VERIFY_PARAM_* host API (available since OpenSSL 1.0.2) is used + * rather than the SSL_set1_host()/SSL_add1_host()/SSL_set_hostflags() + * convenience wrappers (introduced only in OpenSSL 1.1.0). This keeps + * certificate name verification available on every OpenSSL version that + * actually provides the hostname-checking machinery, matching the older + * versions the callers still support. It is also why the function takes the + * X509_VERIFY_PARAM obtained with SSL_get0_param() rather than the SSL object. + * + * Returns 1 on success, 0 on failure, like the OpenSSL calls it wraps. */ +static inline int tlsVerifyParamAddIdentity(X509_VERIFY_PARAM *param, const char *name, + size_t len, int replace) +{ + X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + if (tlsIsIPAddressLiteral(name)) + return X509_VERIFY_PARAM_set1_ip_asc(param, name); + return replace ? X509_VERIFY_PARAM_set1_host(param, name, len) + : X509_VERIFY_PARAM_add1_host(param, name, len); +} +#endif /* CONN_TLS_SUPPORTS_VERIFY_NAME */ + +#endif /* USE_OPENSSL */ + +#endif /* __REDIS_TLSHELPERS_H */ diff --git a/tests/sentinel/tests/15-config-set-config-get.tcl b/tests/sentinel/tests/15-config-set-config-get.tcl index 16b3020006..00835851cb 100644 --- a/tests/sentinel/tests/15-config-set-config-get.tcl +++ b/tests/sentinel/tests/15-config-set-config-get.tcl @@ -24,6 +24,20 @@ test "SENTINEL CONFIG GET for patterns" { S 1 SENTINEL CONFIG SET announce-port 0 announce-hostnames no } +test "SENTINEL CONFIG GET * reports every option" { + # Every option, by name. Besides testing the wildcard, this puts a reply + # holding all options into every run's request/response log, so the + # reply-schema validator (daily reply-schemas-validator job) rejects an + # option that is missing from src/commands/sentinel-config.json: its GET + # schema has additionalProperties false. When adding an option, add it to + # that schema and to this list. + set expected [lsort { + resolve-hostnames announce-hostnames announce-ip announce-port + sentinel-user sentinel-pass loglevel tls-verify-hostnames + }] + assert_equal $expected [lsort [dict keys [S 1 SENTINEL CONFIG GET *]]] +} + test "SENTINEL CONFIG SET duplicate variables" { catch {[S 1 SENTINEL CONFIG SET resolve-hostnames yes announce-port 1234 announce-port 100]} e if {![string match "*Duplicate argument*" $e]} { diff --git a/tests/sentinel/tests/17-tls-verify-hostnames.tcl b/tests/sentinel/tests/17-tls-verify-hostnames.tcl new file mode 100644 index 0000000000..3ffea4684d --- /dev/null +++ b/tests/sentinel/tests/17-tls-verify-hostnames.tcl @@ -0,0 +1,177 @@ +# Sentinel verifies the TLS certificate of every instance it connects to against +# the address it is known by (tls-verify-hostnames, default yes), in addition to +# CA chain validation. These links carry the auth-pass/auth-user credentials, so +# without the check any certificate issued by the trusted CA could impersonate a +# monitored master, replica or sentinel and capture them. +# +# Fixtures: the harness's instance certificate (tests/tls/server.crt) carries +# IP:127.0.0.1 in its SAN, so the default passes. tests/tls/redis.crt chains to +# the same CA but carries no SAN at all: switching an instance to it makes that +# instance's identity unverifiable. + +if {$::tls} { + source "../tests/includes/init-tests.tcl" + + set redis_crt [format "%s/../../tls/redis.crt" [pwd]] + set redis_key [format "%s/../../tls/redis.key" [pwd]] + set server_crt [format "%s/../../tls/server.crt" [pwd]] + set server_key [format "%s/../../tls/server.key" [pwd]] + + # Drop an instance's client connections so the sentinels handshake again: + # the certificate identity is checked only when a link is established. + proc drop_instance_links {id} { + catch {R $id client kill type normal} + catch {R $id client kill type pubsub} + } + + # Make a redis instance present a different certificate, then drop its + # existing client connections so the sentinels handshake again. + proc set_instance_cert {id crt key} { + R $id config set tls-cert-file $crt tls-key-file $key + drop_instance_links $id + } + + # The entry of the replica listening on $port, as seen by sentinel $sid. + proc replica_entry {sid port} { + foreach replica [S $sid SENTINEL REPLICAS mymaster] { + if {[dict get $replica port] == $port} { return $replica } + } + return {} + } + + proc count_identity_warnings {sid} { + set fd [open [file join "sentinel_${sid}" "log.txt"] r] + set n [regexp -all -- {-tls-identity-mismatch} [read $fd]] + close $fd + return $n + } + + # Sentinel has no CONFIG SET for tls-expected-peer-name (it is a server + # option, not a sentinel one), so apply it through the config file and a + # restart. An empty name clears it. + proc sentinel_set_expected_peer_name {id name} { + set cfgfile [file join "sentinel_${id}" "sentinel.conf"] + set f [open $cfgfile r] + set lines {} + foreach line [split [read $f] "\n"] { + if {![string match "tls-expected-peer-name*" $line]} { lappend lines $line } + } + close $f + # Quote the value: a space-separated list is a single argument. + if {$name ne ""} { lappend lines "tls-expected-peer-name \"$name\"" } + set f [open $cfgfile w] + puts $f [join $lines "\n"] + close $f + kill_instance sentinel $id + restart_instance sentinel $id + } + + set replica_id 1 + set replica_port [get_instance_attrib redis $replica_id port] + + test "(tls-verify-hostnames) Identity verification is on by default and the links are up" { + # Earlier units may leave sentinel 0's loglevel anywhere (01-conf-update + # cycles it down to "nothing"); the log assertions below need WARNING + # events to reach the log file. + S 0 SENTINEL CONFIG SET loglevel notice + assert_equal {yes} [lindex [S 0 SENTINEL CONFIG GET tls-verify-hostnames] 1] + wait_for_condition 100 100 { + [dict get [S 0 SENTINEL MASTER mymaster] flags] eq "master" && + [replica_entry 0 $replica_port] ne {} && + ![string match "*s_down*" [dict get [replica_entry 0 $replica_port] flags]] + } else { + fail "Links not up although every certificate carries the instance address" + } + } + + test "(tls-verify-hostnames) An instance whose certificate lacks its address is flagged down" { + set_instance_cert $replica_id $redis_crt $redis_key + # Every sentinel now fails the handshake with this replica: it becomes + # subjectively down, and the mismatch is logged as a WARNING-level event + # naming the expected identity, not buried at DEBUG level. + wait_for_condition 100 100 { + [string match "*s_down*" [dict get [replica_entry 0 $replica_port] flags]] + } else { + fail "Replica not flagged s_down although its certificate lacks its address" + } + wait_for_condition 100 100 { + [count_identity_warnings 0] > 0 + } else { + # The log tail tells whether events reached the file at all (e.g. a + # loglevel left behind by an earlier unit) or the mismatch never + # happened. + puts "--- sentinel 0 log tail:" + puts [exec tail -20 sentinel_0/log.txt] + fail "No -tls-identity-mismatch event logged by sentinel 0" + } + set fd [open "sentinel_0/log.txt" r]; set log [read $fd]; close $fd + assert_match {*expected identity '127.0.0.1'*} $log + assert_match {*sentinel tls-verify-hostnames no*} $log + } + + test "(tls-verify-hostnames) Setting it to no accepts the certificate again" { + S 0 SENTINEL CONFIG SET tls-verify-hostnames no + assert_equal {no} [lindex [S 0 SENTINEL CONFIG GET tls-verify-hostnames] 1] + wait_for_condition 100 100 { + ![string match "*s_down*" [dict get [replica_entry 0 $replica_port] flags]] + } else { + fail "Replica still s_down on sentinel 0 with tls-verify-hostnames no" + } + # Control: sentinel 1 still has tls-verify-hostnames yes, so it must + # still reject this replica's certificate. That shows the certificate is + # still unacceptable, and that sentinel 0 accepts it now only because of + # the setting we changed on sentinel 0. + wait_for_condition 100 100 { + [string match "*s_down*" [dict get [replica_entry 1 $replica_port] flags]] + } else { + fail "Sentinel 1, still verifying, does not flag the replica s_down" + } + } + + test "(tls-verify-hostnames) The setting survives a config rewrite and restart" { + kill_instance sentinel 0 + restart_instance sentinel 0 + assert_equal {no} [lindex [S 0 SENTINEL CONFIG GET tls-verify-hostnames] 1] + S 0 SENTINEL CONFIG SET tls-verify-hostnames yes + } + + test "(tls-verify-hostnames) tls-expected-peer-name overrides the address check" { + # With address verification back on, sentinel 0 rejects the replica again + # -- once the link is re-established, since the identity is verified at + # handshake time and the existing link was accepted under "no". + drop_instance_links $replica_id + wait_for_condition 100 100 { + [string match "*s_down*" [dict get [replica_entry 0 $replica_port] flags]] + } else { + fail "Replica not flagged s_down after re-enabling tls-verify-hostnames" + } + # A configured expected name replaces the address check, and any listed + # name may match. The harness certificate matches through its DNS SAN + # (localhost); redis.crt has no DNS SAN at all, so OpenSSL falls back to + # its Common Name (Generic-cert) -- which is why the same replica that + # fails the address check passes here. + sentinel_set_expected_peer_name 0 "localhost Generic-cert" + wait_for_condition 100 100 { + [dict get [S 0 SENTINEL MASTER mymaster] flags] eq "master" && + [replica_entry 0 $replica_port] ne {} && + ![string match "*s_down*" [dict get [replica_entry 0 $replica_port] flags]] + } else { + fail "Links not up with tls-expected-peer-name matching every certificate" + } + } + + test "(tls-verify-hostnames) Restore the instance certificate and defaults" { + set_instance_cert $replica_id $server_crt $server_key + sentinel_set_expected_peer_name 0 "" + assert_equal {yes} [lindex [S 0 SENTINEL CONFIG GET tls-verify-hostnames] 1] + foreach_sentinel_id id { + wait_for_condition 100 100 { + [dict get [S $id SENTINEL MASTER mymaster] flags] eq "master" && + [replica_entry $id $replica_port] ne {} && + ![string match "*s_down*" [dict get [replica_entry $id $replica_port] flags]] + } else { + fail "Sentinel $id links not back up after restoring the certificate" + } + } + } +} diff --git a/tests/sentinel/tests/18-tls-shared-link-release.tcl b/tests/sentinel/tests/18-tls-shared-link-release.tcl new file mode 100644 index 0000000000..afb272cd1f --- /dev/null +++ b/tests/sentinel/tests/18-tls-shared-link-release.tcl @@ -0,0 +1,150 @@ +# Links to other Sentinels are shared between the masters that know them, so +# the TLS identity check must not hold on to the instance that negotiated a +# connection: SENTINEL REMOVE of that instance's master releases it while the +# handshake may still be pending, with the link kept alive by another master's +# instance. The OpenSSL verify callback used to keep a pointer to that instance +# and dereferenced it after the release, a use-after-free that AddressSanitizer +# reports. It now records the mismatch on the link, and the reconnect path +# reports it through whichever instance still holds the link. +# +# The pending handshake is produced with a fake Sentinel: a plain TCP acceptor +# that holds the connections it receives without speaking TLS, then completes +# them on demand with a certificate lacking the dialed address +# (tests/tls/redis.crt). Sentinel 1 announces the fake's port, so the other +# sentinels dial the fake believing it is sentinel 1. + +if {$::tls} { + source "../tests/includes/init-tests.tcl" + + # The fake sentinel. Accepted connections are held while ::fake_hold is set + # and closed at once afterwards. + set ::fake_conns {} + set ::fake_hold 1 + proc fake_accept {chan addr port} { + fconfigure $chan -translation binary -blocking 0 + if {$::fake_hold} { + lappend ::fake_conns $chan + } else { + catch {close $chan} + } + } + set fake_server [socket -server fake_accept -myaddr 127.0.0.1 0] + set fake_port [lindex [fconfigure $fake_server -sockname] 2] + + # Without a callback the tls package prints every failed handshake to + # stderr, and failing is what the handshakes below are meant to do. + proc fake_tls_callback {args} { return 1 } + + # wait_for_condition sleeps without servicing the event loop, which the + # accept callback above needs; this variant does. + proc wait_for_condition_servicing_events {maxtries delay e _else elsescript} { + while {[incr maxtries -1] >= 0} { + update + if {[uplevel 1 [list expr $e]]} return + after $delay + } + uplevel 1 $elsescript + } + + # Sentinel $sid's entry for the sentinel with run ID $runid under $master, + # or {} if unknown. + proc sentinel_entry {sid master runid} { + foreach s [S $sid SENTINEL SENTINELS $master] { + if {[dict get $s runid] eq $runid} { return $s } + } + return {} + } + + proc count_log_matches {sid pattern} { + set fd [open [file join "sentinel_${sid}" "log.txt"] r] + set n [regexp -all -- $pattern [read $fd]] + close $fd + return $n + } + + set s1_runid [S 1 SENTINEL MYID] + set master_host [get_instance_attrib redis 0 host] + set master_port [get_instance_attrib redis 0 port] + set second_host [get_instance_attrib redis 4 host] + set second_port [get_instance_attrib redis 4 port] + + test "(shared link) Sentinel 0 logs at NOTICE level" { + # Earlier units may leave the loglevel anywhere. + S 0 SENTINEL CONFIG SET loglevel notice + } + + test "(shared link) The other sentinels dial sentinel 1 at the fake's port" { + S 1 SENTINEL CONFIG SET announce-port $fake_port + # Each of the four other sentinels switches sentinel 1's address and + # connects to the fake, where its TLS handshake stays pending. + wait_for_condition_servicing_events 200 100 { + [llength $::fake_conns] >= 4 + } else { + fail "the other sentinels did not connect to the fake sentinel" + } + set e [sentinel_entry 0 mymaster $s1_runid] + assert {$e ne {}} + assert_equal $fake_port [dict get $e port] + } + + test "(shared link) A second master shares sentinel 0's pending link to sentinel 1" { + # Redis 4 becomes a master of its own, monitored by sentinels 0 and 1. + R 4 replicaof no one + S 1 SENTINEL MONITOR second $second_host $second_port 2 + S 0 SENTINEL MONITOR second $second_host $second_port 2 + # Sentinel 1's hello messages on the new master carry the fake's port; + # sentinel 0 learns it there and shares the link it already holds for + # the same run ID under mymaster, the one with the pending handshake. + wait_for_condition_servicing_events 200 100 { + [sentinel_entry 0 second $s1_runid] ne {} && + [dict get [sentinel_entry 0 second $s1_runid] link-refcount] == 2 + } else { + fail "sentinel 0 did not share its link to sentinel 1 with the second master" + } + } + + test "(shared link) Removing the master whose instance negotiated the connection keeps the link" { + S 0 SENTINEL REMOVE mymaster + assert_equal 1 [dict get [sentinel_entry 0 second $s1_runid] link-refcount] + } + + test "(shared link) A mismatch after the release is reported through the surviving instance" { + # Complete every held handshake as a server presenting redis.crt, which + # chains to the CA but carries no address, so each client rejects it + # with an identity mismatch. Sentinel 0's verify callback runs now, + # after the instance that negotiated its connection has been freed. + set ::fake_hold 0 + foreach chan $::fake_conns { + catch { + fconfigure $chan -blocking 1 + tls::import $chan -server 1 -require 0 -request 0 \ + -command fake_tls_callback \ + -cafile "$::tlsdir/ca.crt" \ + -certfile "$::tlsdir/redis.crt" \ + -keyfile "$::tlsdir/redis.key" + tls::handshake $chan + } + catch {close $chan} + } + set ::fake_conns {} + + # Sentinel 0 is alive -- without the fix an AddressSanitizer build dies + # here -- and, on its next reconnection attempt, reports the mismatch + # naming the second master, holder of the surviving instance. + assert_equal PONG [S 0 PING] + wait_for_condition_servicing_events 100 100 { + [count_log_matches 0 {-tls-identity-mismatch sentinel [^\n]* @ second }] > 0 + } else { + fail "sentinel 0 did not report the identity mismatch through the surviving instance" + } + assert_equal PONG [S 0 PING] + } + + test "(shared link) Cleanup" { + close $fake_server + S 1 SENTINEL CONFIG SET announce-port 0 + catch {S 0 SENTINEL REMOVE second} + catch {S 1 SENTINEL REMOVE second} + R 4 replicaof $master_host $master_port + } +} diff --git a/tests/unit/cluster/tls-peer-impersonation.tcl b/tests/unit/cluster/tls-peer-impersonation.tcl index fdf430e7fa..43382219d3 100644 --- a/tests/unit/cluster/tls-peer-impersonation.tcl +++ b/tests/unit/cluster/tls-peer-impersonation.tcl @@ -20,6 +20,14 @@ # but has no SAN. if {$::tls} { + # Node $idx's view of the member with node ID $id, or {} if unknown. + proc cluster_node_view {idx id} { + foreach n [get_cluster_nodes $idx] { + if {[dict get $n id] eq $id} { return $n } + } + return {} + } + set san_crt [format "%s/tests/tls/san.crt" [pwd]] set san_key [format "%s/tests/tls/san.key" [pwd]] @@ -86,19 +94,58 @@ if {$::tls} { # was refused before any packet could be handled. wait_for_log_messages 0 {"*Error accepting cluster node connection*certificate verify failed*"} 0 50 100 - # The forged FAIL must not have been processed. + # The forged FAIL must not have been processed. The next test is the + # positive control for this line: the same packet, sent through a + # certificate carrying the SAN, does produce it. assert_equal 0 [count_log_message 0 "FAIL message received"] # Node 2 must not be flagged failed in node 0's view. - set node2 {} - foreach n [get_cluster_nodes 0] { - if {[dict get $n id] eq $failed_target} { set node2 $n; break } - } + set node2 [cluster_node_view 0 $failed_target] assert {$node2 ne {}} assert_equal 0 [cluster_has_flag $node2 fail] # The victim remains responsive. assert_equal [R 0 PING] {PONG} } + + test "The same forged FAIL is processed when the cert carries the SAN (positive control)" { + # Positive control for the previous test: it proves the forged packet is + # well-formed and acted upon, and that "FAIL message received" is the + # line logged when it is, so the absence asserted above is meaningful. + # The packet is replayed byte for byte; only the certificate differs: + # san.crt carries redis.local, so the accept-side check passes and the + # forged FAIL about node 2 is processed exactly as the threat model + # describes. + # + # Node 2 is a master with slots, so node 0 would keep the FAIL flag for + # twice cluster-node-timeout (6 s here) before trusting it again. Lower + # the timeout on node 0 alone for the duration of this test so the + # cluster is restored well within a second. Node 0 by itself cannot + # reach the quorum needed to fail anyone for real, so the other nodes, + # which keep their timeout, are unaffected. + set old_timeout [lindex [R 0 CONFIG GET cluster-node-timeout] 1] + R 0 CONFIG SET cluster-node-timeout 300 + + # Keep the socket open until the packet has been handled, so the close + # cannot race the read. + set fd [::tls::socket \ + -cafile "$::tlsdir/ca.crt" \ + -certfile "$::tlsdir/san.crt" \ + -keyfile "$::tlsdir/san.key" \ + $target_host $target_bus_port] + fconfigure $fd -translation binary -buffering full + puts -nonewline $fd $packet + flush $fd + wait_for_log_messages 0 \ + [list "*FAIL message received from ${spoofed_sender}*about ${failed_target}*"] 0 50 100 + close $fd + + # Node 0 flagged node 2 as failed on that line; wait for it to clear the + # flag again, then leave the cluster as we found it. + wait_for_log_messages 0 [list "*Clear FAIL state for node ${failed_target}*"] 0 50 100 + R 0 CONFIG SET cluster-node-timeout $old_timeout + assert_equal 0 [cluster_has_flag [cluster_node_view 0 $failed_target] fail] + wait_for_cluster_state ok + } } } diff --git a/tests/unit/tls.tcl b/tests/unit/tls.tcl index d1cd41fe95..d3376609a5 100644 --- a/tests/unit/tls.tcl +++ b/tests/unit/tls.tcl @@ -456,6 +456,34 @@ if {$::tls} { } } + test {TLS: tls-expected-peer-name honors a full-label wildcard SAN but not a partial one} { + # Wildcards are matched from the certificate, not from the option: the + # cert's "*.wild.local" matches the concrete name node.wild.local, ... + start_server [list overrides [list tls-expected-peer-name "node.wild.local"]] { + set replica [srv 0 client] + $replica replicaof $master_host $master_port + wait_for_condition 50 100 { + [string match {*master_link_status:up*} [$replica info replication]] + } else { + fail "Replication link did not come up through the certificate's full-label wildcard" + } + } + # ... while its partial-label "f*.partial.local" never matches, although + # OpenSSL would accept foo.partial.local without + # X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS. + start_server [list overrides [list tls-expected-peer-name "foo.partial.local"]] { + set replica [srv 0 client] + $replica replicaof $master_host $master_port + wait_for_condition 50 100 { + [string match {*master_link_status:up*} [$replica info replication]] || + [count_log_message 0 "certificate verify failed"] > 0 + } else { + fail "Replica never resolved its connection attempt (neither up nor errored)" + } + assert_no_match {*master_link_status:up*} [$replica info replication] + } + } + test {TLS: tls-expected-peer-name matches any of multiple listed names} { # Start with a single non-matching name so the link stays down; then set a # space-separated list via CONFIG SET (a single argument, so it is not split) @@ -635,5 +663,167 @@ if {$::tls} { assert_equal {node.cluster.local} [lindex [r config get tls-expected-peer-name] 1] r config set tls-expected-peer-name "" } + + test {TLS: tls-expected-peer-name rejects IP addresses} { + # The option takes DNS names only. An IP address is refused at config + # time -- alone or mixed with names, IPv4 or IPv6 -- rather than being + # accepted and failing the handshake. + foreach val {"127.0.0.1" "10.0.0.1 redis.local" "redis.local ::1"} { + catch {r config set tls-expected-peer-name $val} e + assert_match {*DNS names only*} $e + } + assert_equal {} [lindex [r config get tls-expected-peer-name] 1] + } + } +} + +# redis-cli / redis-benchmark verify the server certificate's identity by default +# ("verify-full"): besides chaining to the trusted CA, the certificate must carry +# the name given to --sni, or else the host that was dialed (an IP or DNS name). +# --no-verify-hostname drops back to chain-only validation ("verify-ca"), +# --insecure to none, and --tls-expected-peer-name verifies an explicit identity +# instead of the dialed host / SNI. +# +# Fixtures: the harness's default server certificate (tests/tls/server.crt) +# carries IP:127.0.0.1, IP:::1 and DNS:localhost; tests/tls/san.crt carries only +# DNS names (redis.local, cluster.local); tests/tls/redis.crt carries no SAN at +# all. All three chain to the same test CA, so without identity verification the +# tools would accept any of them for any address. +if {$::tls} { + source tests/support/cli.tcl + source tests/support/benchmark.tcl + + set redis_crt [format "%s/tests/tls/redis.crt" [pwd]] + set redis_key [format "%s/tests/tls/redis.key" [pwd]] + set san_crt [format "%s/tests/tls/san.crt" [pwd]] + set san_key [format "%s/tests/tls/san.key" [pwd]] + + start_server {tags {"tls" "cli"}} { + set host [srv 0 host] + set port [srv 0 port] + + test {TLS: redis-cli verifies the dialed IP against the certificate by default} { + exec {*}[rediscli $host $port] PING + } {PONG} + + test {TLS: redis-cli verifies the dialed DNS name against the certificate by default} { + exec {*}[rediscli localhost $port] PING + } {PONG} + + test {TLS: redis-cli verifies the name given to --sni} { + # server.crt does not carry redis.local, so declaring that name fails + # the handshake even though the dialed IP is in the certificate. + catch {exec {*}[rediscli $host $port {--sni redis.local}] PING} e + assert_match {*does not carry the expected identity 'redis.local'*} $e + assert_match {*--no-verify-hostname*} $e + + exec {*}[rediscli $host $port {--sni redis.local --no-verify-hostname}] PING + } {PONG} + + test {TLS: redis-cli rejects contradictory identity options} { + catch {exec {*}[rediscli $host $port {--tls-expected-peer-name redis.local --no-verify-hostname}] PING} e + assert_match {*mutually exclusive*} $e + catch {exec {*}[rediscli $host $port {--insecure --tls-expected-peer-name redis.local}] PING} e + assert_match {*--insecure cannot be combined*} $e + } + + test {TLS: redis-cli validates --tls-expected-peer-name values} { + catch {exec {*}[rediscli $host $port [list --tls-expected-peer-name " "]] PING} e + assert_match {*no usable name*} $e + catch {exec {*}[rediscli $host $port [list --tls-expected-peer-name "redis.local\tcluster.local"]] PING} e + assert_match {*whitespace*} $e + # An IP address must be the only name: OpenSSL requires both the IP + # and a DNS name to match when the two kinds are combined. + catch {exec {*}[rediscli $host $port [list --tls-expected-peer-name "127.0.0.1 redis.local"]] PING} e + assert_match {*only expected peer name*} $e + } + + test {TLS: redis-benchmark verifies the server identity by default} { + set out [exec {*}[redisbenchmark $host $port {-t ping -n 1 -c 1 -q}]] + assert_match {*PING*} $out + } + + # A trusted certificate that carries no identity at all. + start_server [list overrides [list tls-cert-file $redis_crt tls-key-file $redis_key]] { + set chost [srv 0 host] + set cport [srv 0 port] + + test {TLS: redis-cli rejects a trusted certificate lacking the dialed address} { + catch {exec {*}[rediscli $chost $cport] PING} e + assert_match "*does not carry the expected identity '$chost'*" $e + assert_match {*--no-verify-hostname*} $e + } + + test {TLS: --no-verify-hostname and --insecure accept a certificate lacking the address} { + assert_equal PONG [exec {*}[rediscli $chost $cport {--no-verify-hostname}] PING] + assert_equal PONG [exec {*}[rediscli $chost $cport {--insecure}] PING] + # --insecure already implies no identity check; combining is harmless. + assert_equal PONG [exec {*}[rediscli $chost $cport {--insecure --no-verify-hostname}] PING] + } + + test {TLS: the identity is verified before AUTH is sent} { + r config set requirepass secret + r acl log reset + # Mismatch: the handshake fails, so the password never leaves the + # client and the server records no authentication attempt. + catch {exec {*}[rediscli $chost $cport {-a wrongpass --no-auth-warning}] PING} e + assert_match {*does not carry the expected identity*} $e + assert_equal 0 [llength [r acl log]] + # The same client with identity verification off does send AUTH. + catch {exec {*}[rediscli $chost $cport {--no-verify-hostname -a wrongpass --no-auth-warning}] PING} e + assert_match {*WRONGPASS*} $e + assert_equal 1 [llength [r acl log]] + r config set requirepass "" + } + + test {TLS: redis-benchmark rejects a trusted certificate lacking the dialed address} { + catch {exec {*}[redisbenchmark $chost $cport {-t ping -n 1 -c 1 -q}]} e + assert_match {*does not carry the expected identity*} $e + set out [exec {*}[redisbenchmark $chost $cport {--no-verify-hostname -t ping -n 1 -c 1 -q}]] + assert_match {*PING*} $out + } + } + + # A trusted certificate carrying DNS names but not the dialed IP. + start_server [list overrides [list tls-cert-file $san_crt tls-key-file $san_key]] { + set bhost [srv 0 host] + set bport [srv 0 port] + + test {TLS: a certificate without the dialed IP is accepted when --sni names it} { + catch {exec {*}[rediscli $bhost $bport] PING} e + assert_match "*does not carry the expected identity '$bhost'*" $e + assert_equal PONG [exec {*}[rediscli $bhost $bport {--sni redis.local}] PING] + catch {exec {*}[rediscli $bhost $bport {--sni wrong.local}] PING} e + assert_match {*does not carry the expected identity 'wrong.local'*} $e + } + + test {TLS: --tls-expected-peer-name overrides the dialed host and SNI} { + assert_equal PONG [exec {*}[rediscli $bhost $bport {--tls-expected-peer-name cluster.local}] PING] + # The explicit identity wins over a non-matching SNI. + assert_equal PONG [exec {*}[rediscli $bhost $bport {--sni wrong.local --tls-expected-peer-name cluster.local}] PING] + # Any listed name may match. + assert_equal PONG [exec {*}[rediscli $bhost $bport [list --tls-expected-peer-name "absent.local cluster.local"]] PING] + catch {exec {*}[rediscli $bhost $bport {--tls-expected-peer-name absent.local}] PING} e + assert_match {*does not carry the expected identity 'absent.local'*} $e + } + + test {TLS: --tls-expected-peer-name honors a full-label wildcard SAN but not a partial one} { + # Same wildcard rules as the server's option, through the shared + # helper: "*.wild.local" in the cert matches node.wild.local, the + # partial-label "f*.partial.local" does not match foo.partial.local. + assert_equal PONG [exec {*}[rediscli $bhost $bport {--tls-expected-peer-name node.wild.local}] PING] + catch {exec {*}[rediscli $bhost $bport {--tls-expected-peer-name foo.partial.local}] PING} e + assert_match {*does not carry the expected identity 'foo.partial.local'*} $e + } + + test {TLS: --tls-expected-peer-name warns about parent-domain entries} { + # A leading dot is OpenSSL's parent-domain pattern: it matches any + # subdomain at any depth (here redis.local), so its scope is + # surfaced rather than left silent, like the server does. + set out [exec {*}[rediscli $bhost $bport {--tls-expected-peer-name .local}] PING 2>@1] + assert_match {*beginning with a dot*} $out + assert_match {*PONG*} $out + } + } } } diff --git a/utils/gen-test-certs.sh b/utils/gen-test-certs.sh index d7adb88e80..0ddd91a666 100755 --- a/utils/gen-test-certs.sh +++ b/utils/gen-test-certs.sh @@ -5,10 +5,21 @@ # tests/tls/ca.{crt,key} Self signed CA certificate. # tests/tls/redis.{crt,key} A certificate with no key usage/policy restrictions. # tests/tls/client.{crt,key} A certificate restricted for SSL client usage. -# tests/tls/server.{crt,key} A certificate restricted for SSL server usage. +# tests/tls/server.{crt,key} A certificate restricted for SSL server usage. Its +# SAN carries the loopback identities (IP 127.0.0.1, +# IP ::1, DNS localhost), so clients that verify the +# server's identity by default (redis-cli, +# redis-benchmark, Sentinel) accept the test servers. +# redis.{crt,key} above deliberately has no SAN and +# doubles as the "certificate without an identity" +# fixture for the negative tests. # tests/tls/san.{crt,key} A certificate carrying subjectAltName DNS entries # (redis.local, cluster.local), used to exercise -# tls-expected-peer-name verification. +# tls-expected-peer-name verification, plus two +# wildcard entries for the wildcard rules: the +# full-label "*.wild.local" matches any one label, +# the partial-label "f*.partial.local" is rejected +# (X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS). # tests/tls/redis.dh DH Params file. generate_cert() { @@ -48,6 +59,7 @@ cat > tests/tls/openssl.cnf <<_END_ [ server_cert ] keyUsage = digitalSignature, keyEncipherment nsCertType = server +subjectAltName = IP:127.0.0.1, IP:::1, DNS:localhost [ client_cert ] keyUsage = digitalSignature, keyEncipherment @@ -55,7 +67,7 @@ nsCertType = client [ san_cert ] keyUsage = digitalSignature, keyEncipherment -subjectAltName = DNS:redis.local, DNS:cluster.local +subjectAltName = DNS:redis.local, DNS:cluster.local, DNS:*.wild.local, DNS:f*.partial.local _END_ generate_cert server "Server-only" "-extfile tests/tls/openssl.cnf -extensions server_cert"