From f13f13bda503d7d32b8030824d81faefb2181db1 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 18 Oct 2026 11:52:40 -0400 Subject: [PATCH] Name the CA file or directory when loading it fails With OpenSSL 3, a missing or unreadable CA file failed with "OpenSSL error: -2147483646 : Unknown SSL error", because ERR_reason_error_string() returns NULL for system errors and throwSSLError() had no fallback. I now fall back to ERR_error_string_n(), which formats the errno text, and prepareSSLContext() prefixes CA file and directory load failures with the path. The error queue is cleared before each CA load and after throwSSLError() reads it, so a stale entry cannot be reported as the reason of an unrelated failure. The comment above the disabled X509_check_ip() block no longer claims IP addresses go unverified, since OpenSSL 3.0 SSL_set1_host() checks IP literals. --- clickhouse/base/sslsocket.cpp | 59 ++++++++++++++++++++++------------- ut/ssl_ut.cpp | 28 ++++++++++++++--- 2 files changed, 60 insertions(+), 27 deletions(-) diff --git a/clickhouse/base/sslsocket.cpp b/clickhouse/base/sslsocket.cpp index a9e3e6da1..fd4ceed99 100644 --- a/clickhouse/base/sslsocket.cpp +++ b/clickhouse/base/sslsocket.cpp @@ -31,10 +31,20 @@ std::string getCertificateInfo(X509* cert) void throwSSLError(SSL * ssl, int error, const char * /*location*/, const char * /*statement*/, const std::string prefix = "OpenSSL error: ") { const auto detail_error = ERR_get_error(); - auto reason = ERR_reason_error_string(detail_error); - reason = reason ? reason : "Unknown SSL error"; + std::string reason_str; + if (const auto reason = ERR_reason_error_string(detail_error)) { + reason_str = reason; + } else if (detail_error != 0) { + // OpenSSL 3 has no static reason string for system errors (errno from fopen and the like). + char buf[256]; + ERR_error_string_n(detail_error, buf, sizeof(buf)); + reason_str = buf; + } else { + reason_str = "Unknown SSL error"; + } + // Leftover entries would be reported as the reason of a later, unrelated failure. + ERR_clear_error(); - std::string reason_str = reason; if (ssl) { // Print certificate only if handshake isn't completed if (auto ssl_session = SSL_get_session(ssl); ssl_session && SSL_get_state(ssl) != TLS_ST_OK) @@ -129,16 +139,20 @@ SSL_CTX * prepareSSLContext(const clickhouse::SSLParams & context_params) { if (context_params.use_default_ca_locations) HANDLE_SSL_CTX_ERROR(SSL_CTX_set_default_verify_paths(ctx.get())); + const auto load_verify_locations = [&ctx](const char* file, const char* dir, const std::string& what) { + // Otherwise an entry queued by unrelated earlier work would be reported as the reason. + ERR_clear_error(); + if (!SSL_CTX_load_verify_locations(ctx.get(), file, dir)) + throwSSLError(nullptr, static_cast(ERR_peek_error()), LOCATION, "SSL_CTX_load_verify_locations", + "Failed to load " + what + ": "); + }; if (!context_params.path_to_ca_directory.empty()) - HANDLE_SSL_CTX_ERROR( - SSL_CTX_load_verify_locations( - ctx.get(), - nullptr, - context_params.path_to_ca_directory.c_str()) - ); + load_verify_locations(nullptr, context_params.path_to_ca_directory.c_str(), + "CA directory '" + context_params.path_to_ca_directory + "'"); - for (const auto & f : context_params.path_to_ca_files) - HANDLE_SSL_CTX_ERROR(SSL_CTX_load_verify_locations(ctx.get(), f.c_str(), nullptr)); + for (const auto& f : context_params.path_to_ca_files) { + load_verify_locations(f.c_str(), nullptr, "CA file '" + f + "'"); + } if (context_params.context_options != -1) SSL_CTX_set_options(ctx.get(), context_params.context_options); @@ -255,17 +269,18 @@ SSLSocket::SSLSocket(const NetworkAddress& addr, const SocketTimeoutParams& time + "\nServer certificate: " + getCertificateInfo(SSL_get_peer_certificate(ssl))); } - // Host name verification is done by OpenSSL itself, however if we are connecting to an ip-address, - // no verification is made, so we have to do it manually. - // Just in case if this is ever required, leave it here commented out. -// if (ip_addr) { -// // if hostname is actually an IP address -// HANDLE_SSL_ERROR(ssl, X509_check_ip( -// SSL_get_peer_certificate(ssl), -// ASN1_STRING_get0_data(ip_addr.get()), -// ASN1_STRING_length(ip_addr.get()), -// 0)); -// } + // SSL_set1_host() above makes OpenSSL verify the peer name. Since OpenSSL 3.0 it also + // recognizes IP literals and checks them against the certificate's IP SANs; older + // releases match an IP literal only as a DNS name. The manual IP check below is kept + // for reference in case a build ever needs it. + // if (ip_addr) { + // // if hostname is actually an IP address + // HANDLE_SSL_ERROR(ssl, X509_check_ip( + // SSL_get_peer_certificate(ssl), + // ASN1_STRING_get0_data(ip_addr.get()), + // ASN1_STRING_length(ip_addr.get()), + // 0)); + // } } void SSLSocket::validateParams(const SSLParams & ssl_params) { diff --git a/ut/ssl_ut.cpp b/ut/ssl_ut.cpp index 1cb7dd359..8551a420a 100644 --- a/ut/ssl_ut.cpp +++ b/ut/ssl_ut.cpp @@ -1,12 +1,16 @@ /** Collection of integration tests that validate TLS-connectivity to a CH server */ -#include "readonly_client_test.h" -#include "connection_failed_client_test.h" -#include "utils.h" - -#include +#include #include #include +#include + +#include +#include + +#include "connection_failed_client_test.h" +#include "readonly_client_test.h" +#include "utils.h" namespace { using namespace clickhouse; @@ -123,6 +127,20 @@ TEST(OpenSSLConfiguration, InValidValues) { } } +TEST(SSLContext, MissingCAFileErrorNamesFileAndReason) { + const std::string path = "/nonexistent/clickhouse-cpp-ut/ca.pem"; + SSLParams params{}; + params.path_to_ca_files = {path}; + + try { + SSLContext context(params); + FAIL() << "Expected OpenSSLError for a missing CA file"; + } catch (const OpenSSLError& e) { + const std::string what = e.what(); + EXPECT_NE(what.find(path), std::string::npos) << what; + EXPECT_NE(what.find(std::strerror(ENOENT)), std::string::npos) << what; + } +} //INSTANTIATE_TEST_SUITE_P( // Remote_GH_API_TLS_WithStringConfig, ReadonlyClientTest,