Keep client certificates required after setting a verify callback

tls::set_verify_callback() set the verify mode anew, so on a server that
required client certificates it made them optional: OpenSSL and wolfSSL
dropped SSL_VERIFY_FAIL_IF_NO_PEER_CERT, and Mbed TLS went from REQUIRED
to OPTIONAL. A client without a certificate got through. Keep what the
server requires.

httplib.h

@@ -20485,11 +20485,11 @@ inline bool set_verify_callback(ctx_t ctx, VerifyCallback callback) {
   delete static_cast<VerifyCallback *>(SSL_CTX_get_ex_data(ssl_ctx, index));
   SSL_CTX_set_ex_data(ssl_ctx, index, stored);
 
-  if (stored) {
+  // Keeps the other mode bits, so a server that requires a client
-    SSL_CTX_set_verify(ssl_ctx, SSL_VERIFY_PEER, impl::openssl_verify_callback);
+  // certificate goes on requiring one.
-  } else {
+  SSL_CTX_set_verify(ssl_ctx,
-    SSL_CTX_set_verify(ssl_ctx, SSL_VERIFY_PEER, nullptr);
+                     SSL_CTX_get_verify_mode(ssl_ctx) | SSL_VERIFY_PEER,
-  }
+                     stored ? impl::openssl_verify_callback : nullptr);
   return true;
 }
 
@@ -21875,9 +21875,9 @@ inline bool set_verify_callback(ctx_t ctx, VerifyCallback callback) {
   // create_session() hands each session its own copy to run.
   mbed_ctx->verify_callback = std::move(callback);
 
-  if (mbed_ctx->verify_callback) {
+  // OPTIONAL runs the callback even when verification is disabled, as
-    // Set OPTIONAL mode to ensure callback is called even when verification
+  // SSL_VERIFY_PEER does on OpenSSL, unless a client certificate is required.
-    // is disabled (matching OpenSSL behavior where SSL_VERIFY_PEER is set)
+  if (mbed_ctx->verify_callback && !mbed_ctx->verify_client) {
     mbedtls_ssl_conf_authmode(&mbed_ctx->conf, MBEDTLS_SSL_VERIFY_OPTIONAL);
   }
   return true;
@@ -23035,18 +23035,9 @@ inline bool set_verify_callback(ctx_t ctx, VerifyCallback callback) {
 
   wctx->verify_callback = std::move(callback);
 
-  if (wctx->verify_callback) {
+  if (wctx->verify_callback) { impl::disable_server_resumption(wctx); }
-    impl::disable_server_resumption(wctx);
+  // A server that requires a client certificate goes on requiring one.
-    wolfSSL_CTX_set_verify(wctx->ctx, SSL_VERIFY_PEER,
+  set_verify_client(ctx, wctx->verify_client);
-                           impl::wolfssl_verify_callback);
-  } else {
-    wolfSSL_CTX_set_verify(
-        wctx->ctx,
-        wctx->verify_client
-            ? (SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT)
-            : SSL_VERIFY_NONE,
-        nullptr);
-  }
   return true;
 }
 

test/test.cc

@@ -67,8 +67,9 @@ inline std::string u8_to_string(const char8_t *s) {
 namespace httplib {
 namespace tls {
 // Declared here for the split build, where the TLS abstraction declarations
-// live below the split border; the definition is linked from httplib.cc.
+// live below the split border; the definitions are linked from httplib.cc.
 ca_store_t create_ca_store(const char *pem, size_t len);
+bool set_verify_callback(ctx_t ctx, VerifyCallback callback);
 } // namespace tls
 } // namespace httplib
 #endif
@@ -4903,6 +4904,35 @@ TEST(SSLClientServerTest, UpdateCertsPemReplacesClientCA) {
   EXPECT_FALSE(get());
 }
 
+TEST(SSLClientServerTest, VerifyCallbackKeepsClientCertificateRequired) {
+  SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE, CLIENT_CA_CERT_FILE);
+  ASSERT_TRUE(svr.is_valid());
+  tls::set_verify_callback(svr.tls_context(), [](const tls::VerifyContext &v) {
+    return v.preverify_ok;
+  });
+  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();
+
+  SSLClient without_cert(HOST, PORT);
+  without_cert.enable_server_certificate_verification(false);
+  EXPECT_FALSE(without_cert.Get("/test"));
+
+  SSLClient with_cert(HOST, PORT, CLIENT_CERT_FILE, CLIENT_PRIVATE_KEY_FILE);
+  with_cert.enable_server_certificate_verification(false);
+  auto res = with_cert.Get("/test");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ("ok", res->body);
+}
+
 #ifdef CPPHTTPLIB_OPENSSL_SUPPORT
 // A client that resumes its session gets through, but not once the client CA
 // has changed, as a resumed session does not show the certificate again.