Просмотр исходного кода

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.
yhirose 2 дней назад
Родитель
Сommit
e121385910
3 измененных файлов с 130 добавлено и 37 удалено
  1. 1 1
      README.md
  2. 52 36
      httplib.h
  3. 77 0
      test/test.cc

+ 1 - 1
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
 

+ 52 - 36
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<size_t> 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<std::mutex> 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<std::mutex> guard(ctx_mutex_);
-  tls::set_verify_callback(ctx_, std::move(verifier));
+  {
+    // Under the lock that creating a session from ctx_ takes
+    std::lock_guard<std::mutex> guard(ctx_mutex_);
+    tls::set_verify_callback(ctx_, std::move(verifier));
+  }
+  tls_settings_changed();
 }
 
 inline void SSLClient::set_session_verifier(
     std::function<SSLVerifierResponse(tls::session_t)> 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() {

+ 77 - 0
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<std::function<void()>> 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;