Skip to content

Commit eb3f465

Browse files
authored
Fail fast on invalid certificates at TLS config load (valkey-io#2999)
This PR adds the certificates validation at TLS load, rejects invalid (expired/not-yet-valid) certificates: Apply to all TLS config paths: - Server certificates `tls-cert-file` - Server-side client certificates `tls-client-cert-file` - CA certificate file `tls-ca-cert-file` - CA certificate directory `tls-ca-cert-dir` (now eagerly loaded to be consistent with file-based CAs) Apply to both scenarios: - Server startup (initial TLS load) - Runtime reload vis `CONFIG SET` ### Implementation - Added `isCertValid` function to check if an X509 certificate is within its validity period (not expired, not future-dated) - Added `areAllCaCertsValid` function to iterate through all loaded CA certificates and validate them - Added `loadCaCertDir` function to eagerly load all certificates from a directory into the X509_STORE - Modified `createSSLContext` to validate: - Server/client certificates immediately after loading - All CA certificates after loading from file/directory ### Test results #### 1. Server startup (initial TLS load) ``` tls-cert-file ./tests/tls/server-expired.crt 41522:M 31 Dec 2025 16:13:18.851 # Server TLS certificate is invalid. Aborting TLS configuration. 41522:M 31 Dec 2025 16:13:18.851 # Failed to configure TLS. Check logs for more info. tls-client-cert-file ./tests/tls/client-expired.crt 41557:M 31 Dec 2025 16:14:43.296 # Client TLS certificate is invalid. Aborting TLS configuration. 41557:M 31 Dec 2025 16:14:43.296 # Failed to configure TLS. Check logs for more info. tls-ca-cert-file ./tests/tls/ca-expired.crt tls-ca-cert-dir ./tests/tls/ca-expired 41567:M 31 Dec 2025 16:15:15.635 # One or more loaded CA certificates are invalid. Aborting TLS configuration. 41567:M 31 Dec 2025 16:15:15.635 # Failed to configure TLS. Check logs for more info. ``` #### 2. Runtime reload via CONFIG SET ``` 127.0.0.1:6379> config set tls-cert-file ./tests/tls/server-expired.crt (error) ERR CONFIG SET failed (possibly related to argument 'tls-cert-file') - Unable to update TLS configuration. Check server logs. 62975:M 02 Jan 2026 20:10:43.588 # Server TLS certificate is invalid. Aborting TLS configuration. 62975:M 02 Jan 2026 20:10:43.588 # Failed applying new configuration. Possibly related to new tls-cert-file setting. Restoring previous settings. 127.0.0.1:6379> config set tls-client-cert-file ./tests/tls/client-expired.crt (error) ERR CONFIG SET failed (possibly related to argument 'tls-client-cert-file') - Unable to update TLS configuration. Check server logs. 62975:M 02 Jan 2026 20:10:57.972 # Client TLS certificate is invalid. Aborting TLS configuration. 62975:M 02 Jan 2026 20:10:57.972 # Failed applying new configuration. Possibly related to new tls-client-cert-file setting. Restoring previous settings. 127.0.0.1:6379> config set tls-ca-cert-file ./tests/tls/ca-expired.crt 127.0.0.1:6379> config set tls-ca-cert-dir ./tests/tls/ca-expired (error) ERR CONFIG SET failed (possibly related to argument 'tls-ca-cert-file') - Unable to update TLS configuration. Check server logs. 62975:M 02 Jan 2026 20:10:50.175 # One or more loaded CA certificates are invalid. Aborting TLS configuration. 62975:M 02 Jan 2026 20:10:50.175 # Failed applying new configuration. Possibly related to new tls-ca-cert-file setting. Restoring previous settings. ``` --------- Signed-off-by: Yang Zhao <zymy701@gmail.com>
1 parent 0422fc6 commit eb3f465

File tree

4 files changed

+373
-11
lines changed

4 files changed

+373
-11
lines changed

src/tls.c

Lines changed: 105 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
#include <sys/stat.h>
5353
#include <sys/uio.h>
5454
#include <arpa/inet.h>
55+
#include <dirent.h>
56+
#include <limits.h>
5557

5658
#define REDIS_TLS_PROTO_TLSv1 (1 << 0)
5759
#define REDIS_TLS_PROTO_TLSv1_1 (1 << 1)
@@ -210,6 +212,88 @@ static int tlsPasswordCallback(char *buf, int size, int rwflag, void *u) {
210212
return (int)pass_len;
211213
}
212214

215+
/* Check a single X509 certificate validity */
216+
static bool isCertValid(X509 *cert) {
217+
if (!cert) return false;
218+
const ASN1_TIME *not_before = X509_get0_notBefore(cert);
219+
const ASN1_TIME *not_after = X509_get0_notAfter(cert);
220+
if (!not_before || !not_after) return false;
221+
if (X509_cmp_current_time(not_before) > 0 ||
222+
X509_cmp_current_time(not_after) < 0) {
223+
return false;
224+
}
225+
return true;
226+
}
227+
228+
/* Load all certificates from a directory into the X509_STORE
229+
* Returns true on success, false on failure */
230+
static bool loadCaCertDir(SSL_CTX *ctx, const char *ca_cert_dir) {
231+
if (!ca_cert_dir) return true;
232+
233+
DIR *dir;
234+
struct dirent *entry;
235+
char full_path[PATH_MAX];
236+
X509_STORE *store = SSL_CTX_get_cert_store(ctx);
237+
238+
if (!store) {
239+
serverLog(LL_WARNING, "Failed to get X509_STORE from SSL_CTX");
240+
return false;
241+
}
242+
243+
dir = opendir(ca_cert_dir);
244+
if (!dir) {
245+
serverLog(LL_WARNING, "Failed to open CA certificate directory: %s", ca_cert_dir);
246+
return false;
247+
}
248+
249+
while ((entry = readdir(dir)) != NULL) {
250+
if (!strcmp(entry->d_name, ".") || !strcmp(entry->d_name, "..")) continue;
251+
252+
snprintf(full_path, sizeof(full_path), "%s/%s", ca_cert_dir, entry->d_name);
253+
FILE *fp = fopen(full_path, "r");
254+
if (!fp) continue;
255+
256+
X509 *cert = PEM_read_X509(fp, NULL, NULL, NULL);
257+
fclose(fp);
258+
259+
if (cert) {
260+
if (X509_STORE_add_cert(store, cert) != 1) {
261+
unsigned long err = ERR_peek_last_error();
262+
if (ERR_GET_REASON(err) != X509_R_CERT_ALREADY_IN_HASH_TABLE) {
263+
serverLog(LL_WARNING, "Failed to add CA certificate from %s to store", full_path);
264+
X509_free(cert);
265+
closedir(dir);
266+
return false;
267+
}
268+
ERR_clear_error();
269+
}
270+
X509_free(cert);
271+
}
272+
}
273+
274+
closedir(dir);
275+
return true;
276+
}
277+
278+
/* Iterate over all CA certs in the SSL_CTX and fail-fast if any are invalid */
279+
static bool areAllCaCertsValid(SSL_CTX *ctx) {
280+
X509_STORE *store = SSL_CTX_get_cert_store(ctx);
281+
if (!store) return false;
282+
STACK_OF(X509_OBJECT) *objs = X509_STORE_get0_objects(store);
283+
if (!objs) return false;
284+
for (int i = 0; i < sk_X509_OBJECT_num(objs); i++) {
285+
X509_OBJECT *obj = sk_X509_OBJECT_value(objs, i);
286+
int type = X509_OBJECT_get_type(obj);
287+
if (type == X509_LU_X509) {
288+
X509 *ca_cert = X509_OBJECT_get0_X509(obj);
289+
if (ca_cert && !isCertValid(ca_cert)) {
290+
return false;
291+
}
292+
}
293+
}
294+
return true;
295+
}
296+
213297
/* Create a *base* SSL_CTX using the SSL configuration provided. The base context
214298
* includes everything that's common for both client-side and server-side connections.
215299
*/
@@ -254,17 +338,33 @@ static SSL_CTX *createSSLContext(serverTLSContextConfig *ctx_config, int protoco
254338
goto error;
255339
}
256340

341+
if (!isCertValid(SSL_CTX_get0_certificate(ctx))) {
342+
serverLog(LL_WARNING, "%s TLS certificate is invalid. Aborting TLS configuration.", client ? "Client" : "Server");
343+
goto error;
344+
}
345+
257346
if (SSL_CTX_use_PrivateKey_file(ctx, key_file, SSL_FILETYPE_PEM) <= 0) {
258347
ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf));
259348
serverLog(LL_WARNING, "Failed to load private key: %s: %s", key_file, errbuf);
260349
goto error;
261350
}
262351

263-
if ((ctx_config->ca_cert_file || ctx_config->ca_cert_dir) &&
264-
SSL_CTX_load_verify_locations(ctx, ctx_config->ca_cert_file, ctx_config->ca_cert_dir) <= 0) {
265-
ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf));
266-
serverLog(LL_WARNING, "Failed to configure CA certificate(s) file/directory: %s", errbuf);
267-
goto error;
352+
if (ctx_config->ca_cert_file || ctx_config->ca_cert_dir) {
353+
if (SSL_CTX_load_verify_locations(ctx, ctx_config->ca_cert_file, ctx_config->ca_cert_dir) <= 0) {
354+
ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf));
355+
serverLog(LL_WARNING, "Failed to configure CA certificate(s) file/directory: %s", errbuf);
356+
goto error;
357+
}
358+
359+
if (!loadCaCertDir(ctx, ctx_config->ca_cert_dir)) {
360+
serverLog(LL_WARNING, "Failed to load CA certificates from directory: %s", ctx_config->ca_cert_dir);
361+
goto error;
362+
}
363+
364+
if (!areAllCaCertsValid(ctx)) {
365+
serverLog(LL_WARNING, "One or more loaded CA certificates are invalid. Aborting TLS configuration.");
366+
goto error;
367+
}
268368
}
269369

270370
if (ctx_config->ciphers && !SSL_CTX_set_cipher_list(ctx, ctx_config->ciphers)) {

tests/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ match different external server configurations. All options are listed by
3939
Running with TLS requires the following preparations:
4040

4141
* Build Valkey is TLS support, e.g. using `make BUILD_TLS=yes`, or `make BUILD_TLS=module`.
42-
* Run `./utils/gen-test-certs.sh` to generate a root CA and a server certificate.
42+
* Run `./utils/gen-test-certs.sh` to generate a root CA, server certificates, and invalid certificates for testing.
4343
* Install TLS support for TCL, e.g. the `tcl-tls` package on Debian/Ubuntu.
4444

4545
Additional tests

tests/unit/tls.tcl

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,5 +329,111 @@ start_server {tags {"tls"}} {
329329
}
330330
}
331331
}
332+
333+
proc test_tls_cert_rejection {cert_type cert_path expected_error} {
334+
set tlsdir [file normalize ./tests/tls]
335+
set server_path [file normalize ./src/valkey-server]
336+
set server_cert $tlsdir/server.crt
337+
set server_key $tlsdir/server.key
338+
set client_cert $tlsdir/client.crt
339+
set client_key $tlsdir/client.key
340+
set ca_cert_file $tlsdir/ca.crt
341+
set ca_cert_dir ""
342+
343+
switch -- $cert_type {
344+
server { set server_cert $cert_path }
345+
client { set client_cert $cert_path }
346+
"ca-file" { set ca_cert_file $cert_path }
347+
"ca-dir" { set ca_cert_dir $cert_path; set ca_cert_file "" }
348+
}
349+
350+
set cmd [list $server_path --port 0 --tls-port 16379 \
351+
--tls-cert-file $server_cert --tls-key-file $server_key \
352+
--tls-client-cert-file $client_cert --tls-client-key-file $client_key]
353+
354+
if {$ca_cert_file ne ""} { lappend cmd --tls-ca-cert-file $ca_cert_file }
355+
if {$ca_cert_dir ne ""} { lappend cmd --tls-ca-cert-dir $ca_cert_dir }
356+
357+
if {$::tls_module} {
358+
lappend cmd --loadmodule [file normalize ./src/valkey-tls.so]
359+
}
360+
361+
catch {exec {*}$cmd 2>@1} err
362+
assert_match $expected_error $err
363+
}
364+
365+
test {TLS: Fail-fast on invalid certificates at startup} {
366+
set tlsdir [file normalize ./tests/tls]
367+
368+
# Expired server certificate
369+
test_tls_cert_rejection server $tlsdir/server-expired.crt {*Server TLS certificate is invalid*}
370+
371+
# Not-yet-valid server certificate
372+
test_tls_cert_rejection server $tlsdir/server-notyet.crt {*Server TLS certificate is invalid*}
373+
374+
# Expired client certificate
375+
test_tls_cert_rejection client $tlsdir/client-expired.crt {*Client TLS certificate is invalid*}
376+
377+
# Not-yet-valid client certificate
378+
test_tls_cert_rejection client $tlsdir/client-notyet.crt {*Client TLS certificate is invalid*}
379+
380+
# Expired CA certificate file
381+
test_tls_cert_rejection ca-file $tlsdir/ca-expired.crt {*One or more loaded CA certificates are invalid*}
382+
383+
# Not-yet-valid CA certificate file
384+
test_tls_cert_rejection ca-file $tlsdir/ca-notyet.crt {*One or more loaded CA certificates are invalid*}
385+
386+
# Expired CA certificate directory
387+
test_tls_cert_rejection ca-dir $tlsdir/ca-expired {*One or more loaded CA certificates are invalid*}
388+
389+
# Not-yet-valid CA certificate directory
390+
test_tls_cert_rejection ca-dir $tlsdir/ca-notyet {*One or more loaded CA certificates are invalid*}
391+
}
392+
393+
proc test_tls_cert_rejection_runtime {r cert_type cert_path} {
394+
switch -- $cert_type {
395+
server {
396+
catch {$r CONFIG SET tls-cert-file $cert_path} err
397+
}
398+
client {
399+
catch {$r CONFIG SET tls-client-cert-file $cert_path} err
400+
}
401+
"ca-file" {
402+
catch {$r CONFIG SET tls-ca-cert-file $cert_path} err
403+
}
404+
"ca-dir" {
405+
catch {$r CONFIG SET tls-ca-cert-dir $cert_path} err
406+
}
407+
}
408+
assert_match {*Unable to update TLS*} $err
409+
}
410+
411+
test {TLS: Fail-fast on invalid certificates at runtime} {
412+
set tlsdir [file normalize ./tests/tls]
413+
414+
# Expired server certificate
415+
test_tls_cert_rejection_runtime r server $tlsdir/server-expired.crt
416+
417+
# Not-yet-valid server certificate
418+
test_tls_cert_rejection_runtime r server $tlsdir/server-notyet.crt
419+
420+
# Expired client certificate
421+
test_tls_cert_rejection_runtime r client $tlsdir/client-expired.crt
422+
423+
# Not-yet-valid client certificate
424+
test_tls_cert_rejection_runtime r client $tlsdir/client-notyet.crt
425+
426+
# Expired CA certificate file
427+
test_tls_cert_rejection_runtime r ca-file $tlsdir/ca-expired.crt
428+
429+
# Not-yet-valid CA certificate file
430+
test_tls_cert_rejection_runtime r ca-file $tlsdir/ca-notyet.crt
431+
432+
# Expired CA certificate directory
433+
test_tls_cert_rejection_runtime r ca-dir $tlsdir/ca-expired
434+
435+
# Not-yet-valid CA certificate directory
436+
test_tls_cert_rejection_runtime r ca-dir $tlsdir/ca-notyet
437+
}
332438
}
333439
}

0 commit comments

Comments
 (0)