Reject an EOF without close_notify in the Mbed TLS and wolfSSL read (#2649)

Mbed TLS and wolfSSL read a TLS EOF without close_notify as a clean end, so a response delimited by the connection close could be cut short and still come back complete. Both reads now end cleanly only on a close_notify, as on OpenSSL.

httplib.h

@@ -21045,6 +21045,11 @@ inline ErrorCode map_mbedtls_error(int ret, int &out_errno,
     out_errno = errno;
     return ErrorCode::SyscallError;
   }
+  if (ret == MBEDTLS_ERR_SSL_CONN_EOF) {
+    // EOF without close_notify
+    out_errno = 0;
+    return ErrorCode::SyscallError;
+  }
   if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) {
     // Unlike OpenSSL/wolfSSL, Mbed TLS folds the CN/SAN identity check into
     // the handshake's chain verification (see set_sni()); a mismatch there
@@ -21132,7 +21137,7 @@ inline int mbedtls_net_recv_cb(void *ctx, unsigned char *buf, size_t len) {
     return MBEDTLS_ERR_NET_RECV_FAILED;
   }
 #endif
-  if (ret == 0) { return MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY; }
+  // EOF is MBEDTLS_ERR_SSL_CONN_EOF; only a close_notify is a clean end
   return static_cast<int>(ret);
 }
 
@@ -21788,10 +21793,8 @@ inline ssize_t read(session_t session, void *buf, size_t len, TlsError &err) {
     return static_cast<ssize_t>(ret);
   }
 
-  if (ret == 0) {
-    err.code = ErrorCode::PeerClosed;
-    return 0;
-  }
+  // 0 means EOF without close_notify, which is an error as on OpenSSL
+  if (ret == 0) { ret = MBEDTLS_ERR_SSL_CONN_EOF; }
 
   err.code = impl::map_mbedtls_error(ret, err.sys_errno, 0);
   err.backend_code = static_cast<uint64_t>(-ret);
@@ -22398,6 +22401,8 @@ inline ErrorCode map_wolfssl_error(WOLFSSL *ssl, int ssl_error,
   case SSL_ERROR_WANT_WRITE: return ErrorCode::WantWrite;
   case SSL_ERROR_ZERO_RETURN: return ErrorCode::PeerClosed;
   case SSL_ERROR_SYSCALL: out_errno = errno; return ErrorCode::SyscallError;
+  // EOF without close_notify
+  case SOCKET_PEER_CLOSED_E: out_errno = 0; return ErrorCode::SyscallError;
   default:
     if (ssl) {
       // wolfSSL stores the low-level error code as a negative value.
@@ -22968,13 +22973,10 @@ inline ssize_t read(session_t session, void *buf, size_t len, TlsError &err) {
     return static_cast<ssize_t>(ret);
   }
 
-  if (ret == 0) {
-    err.code = ErrorCode::PeerClosed;
-    return 0;
-  }
-
+  // 0 is also returned for EOF without close_notify; get_error tells them apart
   int ssl_error = wolfSSL_get_error(wsession->ssl, ret);
   err.code = impl::map_wolfssl_error(wsession->ssl, ssl_error, err.sys_errno);
+  if (err.code == ErrorCode::PeerClosed) { return 0; }
   err.backend_code = static_cast<uint64_t>(ssl_error);
   impl::wolfssl_last_error() = err.backend_code;
   return -1;

test/test.cc

@@ -72,6 +72,17 @@ namespace tls {
 ca_store_t create_ca_store(const char *pem, size_t len);
 bool set_verify_callback(ctx_t ctx, VerifyCallback callback);
 std::vector<std::string> get_ca_names(ctx_t ctx);
+ctx_t create_server_context();
+void free_context(ctx_t ctx);
+bool set_server_cert_file(ctx_t ctx, const char *cert_path,
+                          const char *key_path, const char *password);
+session_t create_session(ctx_t ctx, socket_t sock);
+void free_session(session_t session);
+bool accept_nonblocking(session_t session, socket_t sock, time_t timeout_sec,
+                        time_t timeout_usec, TlsError *err);
+ssize_t read(session_t session, void *buf, size_t len, TlsError &err);
+ssize_t write(session_t session, const void *buf, size_t len, TlsError &err);
+void shutdown(session_t session, bool graceful);
 #ifdef CPPHTTPLIB_USE_CERTS_FROM_MACOSX_KEYCHAIN
 namespace impl {
 SecTrustSettingsResult
@@ -24067,6 +24078,115 @@ TEST(StreamBodyEndTest, BodyEndingAtConnectionClose) {
   });
 }
 
+#ifdef CPPHTTPLIB_SSL_ENABLED
+// Serves `response` to one request, then closes with or without a close_notify
+template <typename Fn>
+static void with_single_tls_response(const std::string &response,
+                                     bool close_notify, Fn fn) {
+#ifndef _WIN32
+  signal(SIGPIPE, SIG_IGN);
+#endif
+
+  std::promise<int> port_promise;
+  auto port_future = port_promise.get_future();
+  auto server_thread = std::thread([&port_promise, response, close_notify] {
+    auto srv = ::socket(AF_INET, SOCK_STREAM, 0);
+    default_socket_options(srv);
+
+    sockaddr_in addr{};
+    addr.sin_family = AF_INET;
+    addr.sin_port = htons(0);
+    ::inet_pton(AF_INET, "127.0.0.1", &addr.sin_addr);
+    if (::bind(srv, reinterpret_cast<sockaddr *>(&addr), sizeof(addr)) != 0 ||
+        ::listen(srv, 1) != 0) {
+      port_promise.set_value(-1);
+      detail::close_socket(srv);
+      return;
+    }
+    socklen_t addr_len = sizeof(addr);
+    ::getsockname(srv, reinterpret_cast<sockaddr *>(&addr), &addr_len);
+    port_promise.set_value(static_cast<int>(ntohs(addr.sin_port)));
+
+    auto cli = ::accept(srv, nullptr, nullptr);
+    if (cli != INVALID_SOCKET) {
+      auto ctx = tls::create_server_context();
+      tls::set_server_cert_file(ctx, SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE,
+                                nullptr);
+      auto session = tls::create_session(ctx, cli);
+      tls::TlsError err;
+      if (tls::accept_nonblocking(session, cli, 5, 0, &err)) {
+        // The request head first, so that closing the socket sends a FIN
+        // rather than an RST for the unread bytes.
+        std::string req;
+        char buf[256];
+        while (req.find("\r\n\r\n") == std::string::npos) {
+          auto n = tls::read(session, buf, sizeof(buf), err);
+          if (n <= 0) { break; }
+          req.append(buf, static_cast<size_t>(n));
+        }
+        size_t sent = 0;
+        while (sent < response.size()) {
+          auto n = tls::write(session, response.data() + sent,
+                              response.size() - sent, err);
+          if (n <= 0) { break; }
+          sent += static_cast<size_t>(n);
+        }
+        if (close_notify) { tls::shutdown(session, true); }
+      }
+      tls::free_session(session);
+      detail::close_socket(cli);
+      tls::free_context(ctx);
+    }
+    detail::close_socket(srv);
+  });
+  auto se = detail::scope_exit([&] { server_thread.join(); });
+
+  auto port = port_future.get();
+  ASSERT_GT(port, 0);
+  SSLClient cli("127.0.0.1", port);
+  cli.enable_server_certificate_verification(false);
+  fn(cli);
+}
+
+// RFC 8446 6.1: a close-delimited body is complete only after a close_notify
+TEST(SSLClientTest, UnframedBodyEndsOnlyAtCloseNotify) {
+  const std::string close_delimited = "HTTP/1.1 200 OK\r\n"
+                                      "Connection: close\r\n"
+                                      "\r\n"
+                                      "hello";
+  with_single_tls_response(close_delimited, true, [](SSLClient &cli) {
+    auto res = cli.Get("/");
+    ASSERT_TRUE(res) << to_string(res.error());
+    EXPECT_EQ("hello", res->body);
+  });
+
+  with_single_tls_response(close_delimited, false, [](SSLClient &cli) {
+    auto res = cli.Get("/");
+    ASSERT_FALSE(res);
+    EXPECT_EQ(Error::Read, res.error());
+  });
+
+  with_single_tls_response(close_delimited, false, [](SSLClient &cli) {
+    auto handle = cli.open_stream("GET", "/");
+    ASSERT_TRUE(handle.is_valid()) << to_string(handle.error);
+    read_all(handle);
+    EXPECT_TRUE(handle.has_read_error());
+  });
+
+  // A framed body ends where its framing says, with or without the alert
+  const std::string framed = "HTTP/1.1 200 OK\r\n"
+                             "Content-Length: 5\r\n"
+                             "Connection: close\r\n"
+                             "\r\n"
+                             "hello";
+  with_single_tls_response(framed, false, [](SSLClient &cli) {
+    auto res = cli.Get("/");
+    ASSERT_TRUE(res) << to_string(res.error());
+    EXPECT_EQ("hello", res->body);
+  });
+}
+#endif
+
 TEST(InterimResponseTest, SkippedWithTheirHeaderFields) {
   const char *responses[] = {
       "HTTP/1.1 103 Early Hints\r\n"