Skip to content

Fail fast on empty CA directory at TLS config load #3522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yang-z-o wants to merge 1 commit into
base: unstable
Choose a base branch
from
+17 −0
Open

Fail fast on empty CA directory at TLS config load #3522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ static bool loadCaCertDir(SSL_CTX *ctx, const char *ca_cert_dir) {
return false;
}

int loaded = 0;
Copy link
Copy Markdown

@wyuka wyuka Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can we rename this to be a little more descriptive like loaded_ca_certs_count?

while ((entry = readdir(dir)) != NULL) {
if (!strcmp(entry->d_name, ".") || !strcmp(entry->d_name, "..")) continue;

Expand All @@ -519,10 +520,17 @@ static bool loadCaCertDir(SSL_CTX *ctx, const char *ca_cert_dir) {
ERR_clear_error();
}
X509_free(cert);
loaded++;
}
}

closedir(dir);

if (loaded == 0) {
serverLog(LL_WARNING, "No CA certificates loaded from directory: %s", ca_cert_dir);
return false;
}

return true;
}

Expand Down
6 changes: 6 additions & 0 deletions tests/unit/tls.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,9 @@ start_server {tags {"tls"}} {

# Not-yet-valid CA certificate directory
test_tls_cert_rejection ca-dir $tlsdir/ca-notyet {*One or more loaded CA certificates are invalid*}

# Empty CA certificate directory
test_tls_cert_rejection ca-dir $tlsdir/ca-empty {*No CA certificates loaded from directory*}
}

proc test_tls_cert_rejection_runtime {r cert_type cert_path} {
Expand Down Expand Up @@ -509,6 +512,9 @@ start_server {tags {"tls"}} {

# Not-yet-valid CA certificate directory
test_tls_cert_rejection_runtime r ca-dir $tlsdir/ca-notyet

# Empty CA certificate directory
test_tls_cert_rejection_runtime r ca-dir $tlsdir/ca-empty
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions utils/gen-test-certs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
# tests/tls/ca-{expired,notyet}.crt Self signed invalid CA certificates.
# tests/tls/ca-expired/ Directory containing expired CA certificate.
# tests/tls/ca-notyet/ Directory containing not-yet-valid CA certificate.
# tests/tls/ca-empty/ Empty directory for testing empty dir rejection.
# tests/tls/ca-multi.crt CA bundle with multiple certs.
# tests/tls/ca-dir/ CA directory with hashed links.
# tests/tls/valkey.{crt,key} A certificate with no key usage/policy restrictions.
Expand Down Expand Up @@ -208,13 +209,15 @@ openssl ca -batch -config "$CA_CONFIG" \
# Create CA certificate directories for testing tls-ca-cert-dir with invalid certs
mkdir -p tests/tls/ca-expired
mkdir -p tests/tls/ca-notyet
mkdir -p tests/tls/ca-empty

cp tests/tls/ca-expired.crt tests/tls/ca-expired/
cp tests/tls/ca-notyet.crt tests/tls/ca-notyet/

echo "Created CA certificate test directories:"
echo " tests/tls/ca-expired/ (contains expired CA cert)"
echo " tests/tls/ca-notyet/ (contains not-yet-valid CA cert)"
echo " tests/tls/ca-empty/ (empty, for testing empty dir rejection)"

# Clean up temporary files
rm -f tests/tls/*-expired.csr tests/tls/*-notyet.csr tests/tls/ca-expired.csr tests/tls/ca-notyet.csr
Expand Down
Loading