Explorar o código

Fix TLS chain verification bypass for IP hosts on Mbed TLS and wolfSSL

For connections to IP-literal hosts with server certificate
verification enabled, the Mbed TLS and wolfSSL backends downgraded the
verification mode before the handshake because no hostname could be
bound for in-handshake checks:

- SSLClient skipped certificate chain validation entirely; only the
  post-handshake identity check (IP SAN match) remained, so any
  untrusted certificate carrying a matching IP SAN was accepted
- The WebSocket client skipped verification altogether on Mbed TLS,
  accepting any certificate

Keep the verification mode enabled for IP hosts and verify the
certificate identity post-handshake via tls::verify_hostname(), which
supports IP SANs on all backends. The WebSocket path now performs the
same post-handshake identity check as SSLClient. On Mbed TLS, sessions
explicitly opt out of in-handshake hostname verification (mandatory
since Mbed TLS 3.6.4) and the post-handshake check covers identity
instead; DNS hosts still bind the hostname during the handshake. Also
stop sending SNI for IP hosts on Mbed TLS and wolfSSL (RFC 6066).
yhirose hai 2 meses
pai
achega
fa981cedae
Modificáronse 2 ficheiros con 99 adicións e 20 borrados
  1. 33 16
      httplib.h
  2. 66 4
      test/test.cc

+ 33 - 16
httplib.h

@@ -9283,20 +9283,25 @@ inline bool setup_client_tls_session(const std::string &host, tls::ctx_t ctx,
 
   bool is_ip = is_ip_address(host);
 
-#ifdef CPPHTTPLIB_MBEDTLS_SUPPORT
-  if (is_ip && server_certificate_verification) {
-    set_verify_client(ctx, false);
-  } else {
-    set_verify_client(ctx, server_certificate_verification);
-  }
+#if defined(CPPHTTPLIB_MBEDTLS_SUPPORT) || defined(CPPHTTPLIB_WOLFSSL_SUPPORT)
+  // Chain verification happens during the handshake even for IP hosts; the
+  // certificate identity is verified post-handshake via verify_hostname()
+  set_verify_client(ctx, server_certificate_verification);
 #endif
 
   session = create_session(ctx, sock);
   if (!session) { return false; }
 
-  // RFC 6066: SNI must not be set for IP addresses
-  if (!is_ip) { set_sni(session, host.c_str()); }
-  if (server_certificate_verification) { set_hostname(session, host.c_str()); }
+  // RFC 6066: SNI must not be set for IP addresses. On Mbed TLS and wolfSSL
+  // set_hostname also sets SNI, so it must be skipped for IP hosts as well;
+  // their identity is checked post-handshake below instead.
+  if (!is_ip) {
+    if (server_certificate_verification) {
+      set_hostname(session, host.c_str());
+    } else {
+      set_sni(session, host.c_str());
+    }
+  }
 
   if (!connect_nonblocking(session, sock, timeout_sec, timeout_usec, nullptr)) {
     return false;
@@ -9304,6 +9309,14 @@ inline bool setup_client_tls_session(const std::string &host, tls::ctx_t ctx,
 
   if (server_certificate_verification) {
     if (get_verify_result(session) != 0) { return false; }
+
+    // Identity check against the peer certificate, post-handshake for all
+    // backends (same as SSLClient). For IP hosts this is the only identity
+    // verification since no hostname is bound during the handshake.
+    auto server_cert = get_peer_cert(session);
+    if (!server_cert) { return false; }
+    auto cert_guard = detail::scope_exit([&] { free_cert(server_cert); });
+    if (!verify_hostname(server_cert, host.c_str())) { return false; }
   }
 
   return true;
@@ -16200,13 +16213,9 @@ inline bool SSLClient::initialize_ssl(Socket &socket, Error &error) {
 #if defined(CPPHTTPLIB_MBEDTLS_SUPPORT) || defined(CPPHTTPLIB_WOLFSSL_SUPPORT)
   // MbedTLS/wolfSSL need explicit verification mode (OpenSSL uses
   // SSL_VERIFY_NONE by default and performs all verification post-handshake).
-  // For IP addresses with verification enabled, use OPTIONAL mode since
-  // these backends require hostname for strict verification.
-  if (is_ip && server_certificate_verification_) {
-    set_verify_client(ctx_, false);
-  } else {
-    set_verify_client(ctx_, server_certificate_verification_);
-  }
+  // Chain verification happens during the handshake even for IP hosts; the
+  // certificate identity is verified post-handshake via verify_hostname().
+  set_verify_client(ctx_, server_certificate_verification_);
 #endif
 
   // Create TLS session
@@ -18267,6 +18276,14 @@ inline session_t create_session(ctx_t ctx, socket_t sock) {
     return nullptr;
   }
 
+  // Explicitly opt out of in-handshake hostname verification by default;
+  // since Mbed TLS 3.6.4 a client handshake with certificate verification
+  // fails outright when no hostname was set. set_sni() installs the real
+  // hostname for DNS hosts; for IP hosts (where SNI must not be set) the
+  // caller verifies the certificate identity post-handshake via
+  // verify_hostname().
+  mbedtls_ssl_set_hostname(&session->ssl, nullptr);
+
   // Set BIO callbacks
   mbedtls_ssl_set_bio(&session->ssl, &session->sock, impl::mbedtls_net_send_cb,
                       impl::mbedtls_net_recv_cb, nullptr);

+ 66 - 4
test/test.cc

@@ -11774,6 +11774,40 @@ TEST(SSLClientTest, EnableSystemCaCustomCaVerifiesLocalServer) {
   ASSERT_EQ(StatusCode::OK_200, res->status);
 }
 
+// Regression test: for IP hosts the Mbed TLS and wolfSSL backends used to
+// skip chain verification entirely (only the certificate identity was
+// checked), so a server presenting an untrusted certificate with a matching
+// IP SAN was accepted. The chain must be validated for IP hosts too.
+TEST(SSLClientTest, IpHostUntrustedChainFails) {
+  SSLServer svr(SERVER_CERT2_FILE, SERVER_PRIVATE_KEY_FILE);
+  ASSERT_TRUE(svr.is_valid());
+  svr.Get("/test", [&](const Request &, Response &res) {
+    res.set_content("test", "text/plain");
+  });
+
+  thread t = thread([&]() { ASSERT_TRUE(svr.listen("127.0.0.1", PORT)); });
+  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);
+  std::string cert;
+  // A trusted CA that did not sign the server certificate. The server cert
+  // (cert2) carries the matching IP SAN, so only chain validation can reject
+  // this connection.
+  read_file(CLIENT_CA_CERT_FILE, cert);
+  cli.load_ca_cert_store(cert.c_str(), cert.size());
+  cli.enable_server_certificate_verification(true);
+  cli.set_connection_timeout(30);
+
+  auto res = cli.Get("/test");
+  ASSERT_FALSE(res);
+}
+
 // enable_system_ca(false) prevents system CA loading entirely
 TEST(SSLClientTest, DisableSystemCa_Online) {
   SSLClient cli("google.com");
@@ -18553,10 +18587,7 @@ TEST_F(WebSocketSSLIntegrationTest, TextEcho) {
 }
 #endif
 
-// Limited to the OpenSSL backend (like WebSocketSSLIntegrationTest above):
-// Mbed TLS and wolfSSL disable certificate verification for IP hosts in
-// setup_client_tls_session, so the assertions below would not hold there.
-#ifdef CPPHTTPLIB_OPENSSL_SUPPORT
+#ifdef CPPHTTPLIB_SSL_ENABLED
 class WebSocketSSLCATest : public ::testing::Test {
 protected:
   void SetUp() override {
@@ -18633,6 +18664,37 @@ TEST_F(WebSocketSSLCATest, ReconnectWithCustomCaStore) {
   EXPECT_EQ("again", msg);
   client.close();
 }
+
+// Regression test: a certificate with a trusted chain but no identity match
+// for the server IP must be rejected. The Mbed TLS backend used to skip
+// verification entirely for IP hosts, so this connection succeeded there.
+// SERVER_CERT_FILE has no IP SAN (CN only), so trusting it as a CA satisfies
+// chain verification while the identity check must still fail.
+TEST(WebSocketSSLVerifyTest, TrustedChainWrongIdentityFails) {
+  SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE);
+  ASSERT_TRUE(svr.is_valid());
+  svr.WebSocket("/echo", [](const Request &, ws::WebSocket &ws) {
+    std::string msg;
+    while (ws.read(msg)) {
+      ws.send(msg);
+    }
+  });
+  auto port = svr.bind_to_any_port("127.0.0.1");
+  auto t = std::thread([&]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+  });
+  svr.wait_until_ready();
+
+  ws::WebSocketClient client("wss://127.0.0.1:" + std::to_string(port) +
+                             "/echo");
+  std::string cert;
+  read_file(SERVER_CERT_FILE, cert);
+  client.load_ca_cert_store(cert.c_str(), cert.size());
+
+  ASSERT_FALSE(client.connect());
+}
 #endif
 
 #if !defined(_WIN32)