diff --git a/keydb.conf b/keydb.conf index 4aa1c6083..04f66f766 100644 --- a/keydb.conf +++ b/keydb.conf @@ -265,16 +265,12 @@ tcp-keepalive 300 # Setup a allowlist of allowed Common Names (CNs)/Subject Alternative Names (SANs) # that are allowed to connect to this server. This includes both normal clients as # well as other servers connected for replication/clustering purposes. If nothing is -# specified, then no allowlist is used. Supports IPv4, DNS, RFC822, and URI SAN types. -# You can opt to either put all of the names on one line as follows: +# specified, then no allowlist is used and all certificates are accepted. +# Supports IPv4, DNS, RFC822, and URI SAN types. +# You can put multiple names on one line as follows: # # tls-allowlist ... # -# or place then all on their own seperate line (or a combination of the two): -# -# tls-allowlist -# tls-allowlist -# ... # # This configuration also allows for wildcard characters with glob style formatting # i.e. "*.com" would allow all clients to connect with a CN/SAN that ends with ".com" @@ -2073,4 +2069,4 @@ server-threads 2 # this to 1. Very write heavy workloads may benefit from higher numbers. # # By default KeyDB sets this to 2. -replica-weighting-factor 2 \ No newline at end of file +replica-weighting-factor 2 diff --git a/src/config.cpp b/src/config.cpp index a7a3133ef..e7cf6cc24 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -742,10 +742,14 @@ void loadServerConfigFromString(char *config) { err = "argument must be 'yes' or 'no'"; goto loaderr; } } else if (!strcasecmp(argv[0], "tls-allowlist")) { - if (!g_pserver->tls_allowlist_enabled) - g_pserver->tls_allowlist_enabled = true; + if (argc < 2) { + err = "must supply at least one element in the allow list"; goto loaderr; + } + if (!g_pserver->tls_allowlist.empty()) { + err = "tls-allowlist may only be set once"; goto loaderr; + } for (int i = 1; i < argc; i++) - g_pserver->tls_allowlist.insert(zstrdup(argv[i])); + g_pserver->tls_allowlist.emplace(argv[i], strlen(argv[i])); } else if (!strcasecmp(argv[0], "version-override") && argc == 2) { KEYDB_SET_VERSION = zstrdup(argv[1]); serverLog(LL_WARNING, "Warning version is overriden to: %s\n", KEYDB_SET_VERSION); @@ -866,8 +870,18 @@ void configSetCommand(client *c) { int err; const char *errstr = NULL; serverAssertWithInfo(c,c->argv[2],sdsEncodedObject(c->argv[2])); - serverAssertWithInfo(c,c->argv[3],sdsEncodedObject(c->argv[3])); - o = c->argv[3]; + + if (c->argc < 4 || c->argc > 4) { + o = nullptr; + // Variadic set is only supported for tls-allowlist + if (strcasecmp(szFromObj(c->argv[2]), "tls-allowlist")) { + addReplySubcommandSyntaxError(c); + return; + } + } else { + o = c->argv[3]; + serverAssertWithInfo(c,c->argv[3],sdsEncodedObject(c->argv[3])); + } /* Iterate the configs that are standard */ for (standardConfig *config = configs; config->name != NULL; config++) { @@ -1029,6 +1043,12 @@ void configSetCommand(client *c) { enableWatchdog(ll); else disableWatchdog(); + } config_set_special_field("tls-allowlist") { + g_pserver->tls_allowlist.clear(); + for (int i = 3; i < c->argc; ++i) { + robj *val = c->argv[i]; + g_pserver->tls_allowlist.emplace(szFromObj(val), sdslen(szFromObj(val))); + } /* Everything else is an error... */ } config_set_else { addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s", @@ -1232,6 +1252,14 @@ void configGetCommand(client *c) { addReplyBulkCString(c, g_pserver->fActiveReplica ? "yes" : "no"); matches++; } + if (stringmatch(pattern, "tls-allowlist", 1)) { + addReplyBulkCString(c,"tls-allowlist"); + addReplyArrayLen(c, (long)g_pserver->tls_allowlist.size()); + for (auto &elem : g_pserver->tls_allowlist) { + addReplyBulkCBuffer(c, elem.get(), elem.size()); // addReplyBulkSds will free which we absolutely don't want + } + matches++; + } setDeferredMapLen(c,replylen,matches); } @@ -1901,6 +1929,20 @@ int rewriteConfig(char *path, int force_all) { rewriteConfigStringOption(state, "version-override",KEYDB_SET_VERSION,KEYDB_REAL_VERSION); rewriteConfigOOMScoreAdjValuesOption(state); + if (!g_pserver->tls_allowlist.empty()) { + sds conf = sdsnew("tls-allowlist "); + for (auto &elem : g_pserver->tls_allowlist) { + conf = sdscatsds(conf, (sds)elem.get()); + conf = sdscat(conf, " "); + } + // trim the trailing space + sdsrange(conf, 0, -1); + rewriteConfigRewriteLine(state,"tls-allowlist",conf,1 /*force*/); + // note: conf is owned by rewriteConfigRewriteLine - no need to free + } else { + rewriteConfigMarkAsProcessed(state, "tls-allowlist"); // ensure the line is removed if it existed + } + /* Rewrite Sentinel config if in Sentinel mode. */ if (g_pserver->sentinel_mode) rewriteConfigSentinelOption(state); @@ -2907,7 +2949,7 @@ NULL }; addReplyHelp(c, help); - } else if (!strcasecmp(szFromObj(c->argv[1]),"set") && c->argc == 4) { + } else if (!strcasecmp(szFromObj(c->argv[1]),"set") && c->argc >= 3) { configSetCommand(c); } else if (!strcasecmp(szFromObj(c->argv[1]),"get") && c->argc == 3) { configGetCommand(c); diff --git a/src/server.h b/src/server.h index 05cceed86..1aad87c3e 100644 --- a/src/server.h +++ b/src/server.h @@ -2612,8 +2612,7 @@ struct redisServer { int tls_auth_clients; int tls_rotation; - int tls_allowlist_enabled; - std::unordered_set tls_allowlist; + std::set tls_allowlist; redisTLSContextConfig tls_ctx_config; /* cpu affinity */ diff --git a/src/tls.cpp b/src/tls.cpp index d914f3d7e..c2490bd56 100644 --- a/src/tls.cpp +++ b/src/tls.cpp @@ -484,14 +484,17 @@ bool tlsCheckAgainstAllowlist(const char * client){ /* Because of wildcard matching, we need to iterate over the entire set. * If we were doing simply straight matching, we could just directly * check to see if the client name is in the set in O(1) time */ - for (char * client_pattern: g_pserver->tls_allowlist){ - if (stringmatchlen(client_pattern, strlen(client_pattern), client, strlen(client), 1)) + for (auto &client_pattern: g_pserver->tls_allowlist){ + if (stringmatchlen(client_pattern.get(), client_pattern.size(), client, strlen(client), 1)) return true; } return false; } bool tlsValidateCertificateName(tls_connection* conn){ + if (g_pserver->tls_allowlist.empty()) + return true; // Empty list implies acceptance of all + X509 * cert = SSL_get_peer_certificate(conn->ssl); /* Check the common name (CN) of the certificate first */ X509_NAME_ENTRY * ne = X509_NAME_get_entry(X509_get_subject_name(cert), X509_NAME_get_index_by_NID(X509_get_subject_name(cert), NID_commonName, -1)); @@ -776,7 +779,7 @@ void tlsHandleEvent(tls_connection *conn, int mask) { conn->c.state = CONN_STATE_ERROR; } else { /* Validate name */ - if (g_pserver->tls_allowlist_enabled && !tlsValidateCertificateName(conn)){ + if (!tlsValidateCertificateName(conn)){ conn->c.state = CONN_STATE_ERROR; } else { conn->c.state = CONN_STATE_CONNECTED;