From e121385910f8de6d1e1911b93234d13ff7dae74e Mon Sep 17 00:00:00 2001 From: yhirose Date: Thu, 22 Oct 2026 00:08:44 -0400 Subject: [PATCH] Reconnect after a TLS setting changes on a kept-alive client TLS settings take effect as a connection is set up, so after a client changed one (server certificate or host name verification, a CA store, a verifier) a later request still went over the connection kept alive under the old settings, unverified if verification had just been turned on. Each such setter now bumps a version that the connection records when it is set up, and a connection set up under an older version is closed instead of reused. The setters take no lock, so they cannot wait on a handshake in flight. --- README.md | 2 +- httplib.h | 88 +++++++++++++++++++++++++++++++--------------------- test/test.cc | 77 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 63743322..1011f1e0 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ cli.enable_server_hostname_verification(false); ``` > [!NOTE] -> The CA certificates are loaded when the client makes its first connection. Changing `set_ca_cert_path()` or `enable_system_ca()` after that has no effect, so make the settings before the first request, or use a new client. `set_ca_cert_store()` and `load_ca_cert_store()` take effect at once. `WebSocketClient` works the same way. +> The CA certificates are loaded when the client makes its first connection. Changing `set_ca_cert_path()` or `enable_system_ca()` after that has no effect, so make the settings before the first request, or use a new client. `set_ca_cert_store()` and `load_ca_cert_store()` take effect at once. `WebSocketClient` works the same way. On `Client` and `SSLClient`, a setter that changes a TLS setting also closes a connection kept alive under the old one, so the next request connects again. ### SSL Error Handling diff --git a/httplib.h b/httplib.h index b11a6325..17bcb21d 100644 --- a/httplib.h +++ b/httplib.h @@ -3000,6 +3000,15 @@ protected: void close_socket(Socket &socket); void disconnect(bool gracefully); + // Under socket_mutex_: whether the connection kept alive can take another + // request. One the peer has closed, or one set up under TLS settings changed + // since, is closed instead. + bool socket_reusable(); + + // TLS settings apply as a connection is set up, so a change makes the + // connection kept alive unfit for the next request. + void tls_settings_changed() { tls_settings_version_++; } + std::string make_request_target(const Request &req) const; bool process_request(Stream &strm, Request &req, Response &res, bool close_connection, Error &error); @@ -3026,6 +3035,9 @@ protected: size_t socket_requests_in_flight_ = 0; std::thread::id socket_requests_are_from_thread_ = std::thread::id(); bool socket_should_be_closed_when_request_is_done_ = false; + size_t socket_tls_settings_version_ = 0; + + std::atomic tls_settings_version_{0}; // Hostname to connection target map. The value is an IP literal or another // hostname; only the connection target changes, never the identity. @@ -15546,6 +15558,7 @@ inline bool ClientImpl::create_and_connect_socket(Socket &socket, } inline bool ClientImpl::ensure_socket_connection(Socket &socket, Error &error) { + socket_tls_settings_version_ = tls_settings_version_; return create_and_connect_socket(socket, error); } @@ -15595,6 +15608,28 @@ inline void ClientImpl::disconnect(bool gracefully) { close_socket(socket_); } +inline bool ClientImpl::socket_reusable() { + if (!socket_.is_open()) { return false; } + + auto is_alive = detail::is_socket_alive(socket_.sock); +#ifdef CPPHTTPLIB_SSL_ENABLED + if (is_alive && is_ssl() && tls::is_peer_closed(socket_.ssl, socket_.sock)) { + is_alive = false; + } +#endif + if (!is_alive) { + // Peer seems gone — non-graceful shutdown to avoid SIGPIPE. + disconnect(/*gracefully=*/false); + return false; + } + + if (is_ssl() && socket_tls_settings_version_ != tls_settings_version_) { + disconnect(/*gracefully=*/true); + return false; + } + return true; +} + inline bool ClientImpl::read_response_line(Stream &strm, const Request &req, Response &res, bool skip_100_continue) const { @@ -15672,25 +15707,7 @@ inline bool ClientImpl::send_(Request &req, Response &res, Error &error) { // socket. socket_should_be_closed_when_request_is_done_ = false; - auto is_alive = false; - if (socket_.is_open()) { - is_alive = detail::is_socket_alive(socket_.sock); - -#ifdef CPPHTTPLIB_SSL_ENABLED - if (is_alive && is_ssl()) { - if (tls::is_peer_closed(socket_.ssl, socket_.sock)) { - is_alive = false; - } - } -#endif - - if (!is_alive) { - // Peer seems gone — non-graceful shutdown to avoid SIGPIPE. - disconnect(/*gracefully=*/false); - } - } - - if (!is_alive) { + if (!socket_reusable()) { if (!ensure_socket_connection(socket_, error)) { return false; } { @@ -15853,20 +15870,7 @@ inline ClientImpl::StreamHandle ClientImpl::open_stream_(Request &req) { { std::lock_guard guard(socket_mutex_); - auto is_alive = false; - if (socket_.is_open()) { - is_alive = detail::is_socket_alive(socket_.sock); -#ifdef CPPHTTPLIB_SSL_ENABLED - if (is_alive && is_ssl()) { - if (tls::is_peer_closed(socket_.ssl, socket_.sock)) { - is_alive = false; - } - } -#endif - if (!is_alive) { disconnect(/*gracefully=*/false); } - } - - if (!is_alive) { + if (!socket_reusable()) { if (!ensure_socket_connection(socket_, handle.error)) { handle.response.reset(); return handle; @@ -18083,11 +18087,15 @@ inline void ClientImpl::set_proxy_digest_auth(const std::string &username, } inline void ClientImpl::enable_server_certificate_verification(bool enabled) { + if (server_certificate_verification_ == enabled) { return; } server_certificate_verification_ = enabled; + tls_settings_changed(); } inline void ClientImpl::enable_server_hostname_verification(bool enabled) { + if (server_hostname_verification_ == enabled) { return; } server_hostname_verification_ = enabled; + tls_settings_changed(); } inline void ClientImpl::enable_system_ca(bool enabled) { @@ -19246,25 +19254,32 @@ inline void SSLClient::set_ca_cert_store(tls::ca_store_t ca_cert_store) { tls::free_ca_store(ca_cert_store); } custom_ca_loaded_ = true; + tls_settings_changed(); } inline void SSLClient::set_server_certificate_verifier(tls::VerifyCallback verifier) { if (!ctx_) { return; } server_certificate_verifier_ = verifier; - // Under the lock that creating a session from ctx_ takes - std::lock_guard guard(ctx_mutex_); - tls::set_verify_callback(ctx_, std::move(verifier)); + { + // Under the lock that creating a session from ctx_ takes + std::lock_guard guard(ctx_mutex_); + tls::set_verify_callback(ctx_, std::move(verifier)); + } + tls_settings_changed(); } inline void SSLClient::set_session_verifier( std::function verifier) { session_verifier_ = std::move(verifier); + tls_settings_changed(); } #ifdef CPPHTTPLIB_WINDOWS_AUTOMATIC_ROOT_CERTIFICATES_UPDATE inline void SSLClient::enable_windows_certificate_verification(bool enabled) { + if (enable_windows_cert_verification_ == enabled) { return; } enable_windows_cert_verification_ = enabled; + tls_settings_changed(); } #endif @@ -19279,6 +19294,7 @@ inline void SSLClient::load_ca_cert_store(const char *ca_cert, tls::load_ca_pem(ctx_, ca_cert, size); } custom_ca_loaded_ = true; + tls_settings_changed(); } inline bool SSLClient::load_certs() { diff --git a/test/test.cc b/test/test.cc index f586fcb1..085c42ef 100644 --- a/test/test.cc +++ b/test/test.cc @@ -18343,6 +18343,83 @@ TEST(SSLClientTest, EmptyCustomCaKeepsSystemCasOut) { } #endif +// A TLS setting changed between requests applies to the next one, not just to +// the next connection +TEST(SSLClientTest, TlsSettingChangeClosesKeptAliveConnection) { + SSLServer svr(SERVER_CERT2_FILE, SERVER_PRIVATE_KEY_FILE); + ASSERT_TRUE(svr.is_valid()); + svr.Get("/", [](const Request &req, Response &res) { + res.set_content(std::to_string(req.remote_port), "text/plain"); + }); + auto port = svr.bind_to_any_port("127.0.0.1"); + ASSERT_LT(0, port); + thread t = thread([&] { svr.listen_after_bind(); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + ASSERT_FALSE(svr.is_running()); + }); + svr.wait_until_ready(); + + SSLClient cli("127.0.0.1", port); + cli.set_keep_alive(true); + cli.set_ca_cert_path(CLIENT_CA_CERT_FILE); // does not trust cert2.pem + cli.enable_server_certificate_verification(false); + + // The port the request went out from, so a new one means a new connection + auto client_port = [&] { + auto res = cli.Get("/"); + return res ? res->body : std::string(); + }; + auto last = client_port(); + ASSERT_FALSE(last.empty()); + + // Setting a value it already has keeps the connection + cli.enable_server_certificate_verification(false); + cli.enable_server_hostname_verification(true); + EXPECT_EQ(last, client_port()); + + cli.enable_server_certificate_verification(true); + auto res = cli.Get("/"); + EXPECT_FALSE(res); + EXPECT_EQ(Error::SSLServerVerification, res.error()); + + std::string ca; + read_file(SERVER_CERT2_FILE, ca); + cli.load_ca_cert_store(ca.c_str(), ca.size()); + last = client_port(); + ASSERT_FALSE(last.empty()); + + std::vector> setters = { + [&] { cli.enable_server_hostname_verification(false); }, + [&] { cli.load_ca_cert_store(ca.c_str(), ca.size()); }, + [&] { + cli.set_ca_cert_store(tls::create_ca_store(ca.c_str(), ca.size())); + }, + [&] { + cli.set_server_certificate_verifier( + [](const tls::VerifyContext &) { return true; }); + }, + [&] { + cli.set_session_verifier( + [](tls::session_t) { return SSLVerifierResponse::NoDecisionMade; }); + }, + }; + for (const auto &setter : setters) { + setter(); + auto next = client_port(); + ASSERT_FALSE(next.empty()); + EXPECT_NE(last, next); + last = next; + } + + cli.set_session_verifier( + [](tls::session_t) { return SSLVerifierResponse::CertificateRejected; }); + auto handle = cli.open_stream("GET", "/"); + EXPECT_FALSE(handle.is_valid()); + EXPECT_EQ(Error::SSLServerVerification, handle.error); +} + // A PEM given to create_ca_store() needs no NUL after it TEST(SSLClientTest, CreateCaStoreFromPemWithoutNul) { std::string pem;