Răsfoiți Sursa

Wait for handshakes before replacing a server's certificates

update_certs_pem() replaced the certificate, key and client CAs inside
the TLS context while handshakes on other threads were still reading
them, and only the creation of a session was serialized with it. A
server rotating its certificates under load could crash.

Count the handshakes in progress, make an update wait until there are
none, and hold off new handshakes while an update is pending so that a
steady stream of them cannot keep it waiting. The wait is bounded by
the handshake timeout.
yhirose 1 zi în urmă
părinte
comite
9c51a0effa

+ 1 - 1
docs-src/pages/en/cookbook/t03-ssl-server.md

@@ -60,7 +60,7 @@ Before a certificate expires, you may want to swap it out without restarting the
 svr.update_certs_pem(new_cert_pem, new_key_pem);
 ```
 
-Existing connections keep using the old cert; new connections use the new one.
+Existing connections keep using the old cert; new connections use the new one. It waits for handshakes in progress to finish, at most the read timeout, and new connections wait until it returns.
 
 ## Generating a test certificate
 

+ 1 - 1
docs-src/pages/ja/cookbook/t03-ssl-server.md

@@ -60,7 +60,7 @@ httplib::SSLServer svr(pem);
 svr.update_certs_pem(new_cert_pem, new_key_pem);
 ```
 
-既存の接続はそのまま、これから確立する接続は新しい証明書で動きます。
+既存の接続はそのまま、これから確立する接続は新しい証明書で動きます。差し替えは進行中のハンドシェイクが終わってから行われ(長くても読み取りタイムアウトまで)、その間に来た接続は差し替えが終わるまで待ちます。
 
 ## 証明書の準備
 

+ 24 - 5
httplib.h

@@ -3373,7 +3373,14 @@ private:
   bool process_and_close_socket(socket_t sock) override;
 
   tls::ctx_t ctx_ = nullptr;
+  // update_certs_pem() replaces the certificates and CAs that handshakes read
+  // from ctx_, so it waits for the handshakes in progress, which their timeout
+  // bounds, and new ones wait while it is pending so that they cannot keep it
+  // waiting.
   std::mutex ctx_mutex_;
+  std::condition_variable ctx_cv_;
+  size_t handshakes_in_progress_ = 0;
+  size_t updates_pending_ = 0;
 
   int last_ssl_error_ = 0;
 };
@@ -18497,11 +18504,12 @@ inline bool SSLServer::is_valid() const {
 inline bool SSLServer::process_and_close_socket(socket_t sock) {
   using namespace tls;
 
-  // Create TLS session with mutex protection
   session_t session = nullptr;
   {
-    std::lock_guard<std::mutex> guard(ctx_mutex_);
+    std::unique_lock<std::mutex> lock(ctx_mutex_);
+    ctx_cv_.wait(lock, [&] { return updates_pending_ == 0; });
     session = create_session(static_cast<ctx_t>(ctx_), sock);
+    if (session) { handshakes_in_progress_++; }
   }
 
   if (!session) {
@@ -18524,8 +18532,13 @@ inline bool SSLServer::process_and_close_socket(socket_t sock) {
 
   // Perform TLS accept handshake with timeout
   TlsError tls_err;
-  if (!accept_nonblocking(session, sock, read_timeout_sec_, read_timeout_usec_,
-                          &tls_err)) {
+  auto accepted = accept_nonblocking(session, sock, read_timeout_sec_,
+                                     read_timeout_usec_, &tls_err);
+  {
+    std::lock_guard<std::mutex> guard(ctx_mutex_);
+    if (--handshakes_in_progress_ == 0) { ctx_cv_.notify_all(); }
+  }
+  if (!accepted) {
 #ifdef CPPHTTPLIB_OPENSSL_SUPPORT
     // Map TlsError to legacy ssl_error for backward compatibility
     if (tls_err.code == ErrorCode::WantRead) {
@@ -18572,7 +18585,13 @@ inline bool SSLServer::update_certs_pem(const char *cert_pem,
                                         const char *client_ca_pem,
                                         const char *password) {
   if (!ctx_) { return false; }
-  std::lock_guard<std::mutex> guard(ctx_mutex_);
+  std::unique_lock<std::mutex> lock(ctx_mutex_);
+  updates_pending_++;
+  auto se = detail::scope_exit([&] {
+    if (--updates_pending_ == 0) { ctx_cv_.notify_all(); }
+  });
+  ctx_cv_.wait(lock, [&] { return handshakes_in_progress_ == 0; });
+
   if (!tls::update_server_cert(ctx_, cert_pem, key_pem, password)) {
     return false;
   }

+ 47 - 0
test/test.cc

@@ -4657,6 +4657,53 @@ TEST(SSLClientServerTest, UpdateCertsPemWithClientAuth) {
   ASSERT_TRUE(handler_called);
   EXPECT_EQ("ok", res->body);
 }
+
+TEST(SSLClientServerTest, UpdateCertsPemWaitsForHandshakeInProgress) {
+  SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE);
+  ASSERT_TRUE(svr.is_valid());
+  svr.Get("/test", [](const Request &, Response &res) {
+    res.set_content("ok", "text/plain");
+  });
+
+  thread t = thread([&]() { ASSERT_TRUE(svr.listen(HOST, PORT)); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  // A connection that sends no ClientHello keeps its handshake in progress,
+  // once the server has had a moment to start it.
+  auto error = Error::Success;
+  auto sock = detail::create_client_socket(
+      HOST, "", PORT, AF_UNSPEC, false, false, nullptr,
+      /*connection_timeout_sec=*/5, 0,
+      /*read_timeout_sec=*/5, 0,
+      /*write_timeout_sec=*/5, 0, std::string(), error);
+  ASSERT_NE(INVALID_SOCKET, sock);
+  std::this_thread::sleep_for(std::chrono::milliseconds(500));
+
+  std::string cert_pem, key_pem;
+  read_file(SERVER_CERT_FILE, cert_pem);
+  read_file(SERVER_PRIVATE_KEY_FILE, key_pem);
+
+  auto update = std::async(std::launch::async, [&]() {
+    return svr.update_certs_pem(cert_pem.c_str(), key_pem.c_str());
+  });
+  EXPECT_EQ(std::future_status::timeout,
+            update.wait_for(std::chrono::milliseconds(200)));
+
+  detail::close_socket(sock);
+  EXPECT_TRUE(update.get());
+
+  SSLClient cli(HOST, PORT);
+  cli.enable_server_certificate_verification(false);
+  auto res = cli.Get("/test");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::OK_200, res->status);
+}
 #endif
 
 TEST(ErrorHandlerTest, ContentLength) {