diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 45dc91c48e..135bf7b500 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -148,7 +148,7 @@ jobs: run: vcpkg install gtest curl zlib brotli zstd - name: Install OpenSSL if: ${{ matrix.config.with_ssl }} - run: choco install openssl --version 3.5.2 # workaround for chocolatey issue with the latest OpenSSL + run: choco install openssl - name: Configure CMake ${{ matrix.config.name }} run: > cmake -B build -S . diff --git a/httplib.h b/httplib.h index 32bd094387..d171e56b6b 100644 --- a/httplib.h +++ b/httplib.h @@ -1954,14 +1954,17 @@ class SSLServer : public Server { void update_certs(X509 *cert, EVP_PKEY *private_key, X509_STORE *client_ca_cert_store = nullptr); + int ssl_last_error() const { return last_ssl_error_; } + private: bool process_and_close_socket(socket_t sock) override; + STACK_OF(X509_NAME) * extract_ca_names_from_x509_store(X509_STORE *store); + SSL_CTX *ctx_; std::mutex ctx_mutex_; -#ifdef CPPHTTPLIB_OPENSSL_SUPPORT + int last_ssl_error_ = 0; -#endif }; class SSLClient final : public ClientImpl { @@ -10716,6 +10719,19 @@ inline SSLServer::SSLServer(const char *cert_path, const char *private_key_path, SSL_CTX_load_verify_locations(ctx_, client_ca_cert_file_path, client_ca_cert_dir_path); + // Set client CA list to be sent to clients during TLS handshake + if (client_ca_cert_file_path) { + auto ca_list = SSL_load_client_CA_file(client_ca_cert_file_path); + if (ca_list != nullptr) { + SSL_CTX_set_client_CA_list(ctx_, ca_list); + } else { + // Failed to load client CA list, but we continue since + // SSL_CTX_load_verify_locations already succeeded and + // certificate verification will still work + last_ssl_error_ = static_cast(ERR_get_error()); + } + } + SSL_CTX_set_verify( ctx_, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr); } @@ -10740,6 +10756,15 @@ inline SSLServer::SSLServer(X509 *cert, EVP_PKEY *private_key, } else if (client_ca_cert_store) { SSL_CTX_set_cert_store(ctx_, client_ca_cert_store); + // Extract CA names from the store and set them as the client CA list + auto ca_list = extract_ca_names_from_x509_store(client_ca_cert_store); + if (ca_list) { + SSL_CTX_set_client_CA_list(ctx_, ca_list); + } else { + // Failed to extract CA names, record the error + last_ssl_error_ = static_cast(ERR_get_error()); + } + SSL_CTX_set_verify( ctx_, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr); } @@ -10820,6 +10845,44 @@ inline bool SSLServer::process_and_close_socket(socket_t sock) { return ret; } +inline STACK_OF(X509_NAME) * SSLServer::extract_ca_names_from_x509_store( + X509_STORE *store) { + if (!store) { return nullptr; } + + auto ca_list = sk_X509_NAME_new_null(); + if (!ca_list) { return nullptr; } + + // Get all objects from the store + auto objs = X509_STORE_get0_objects(store); + if (!objs) { + sk_X509_NAME_free(ca_list); + return nullptr; + } + + // Iterate through objects and extract certificate subject names + for (int i = 0; i < sk_X509_OBJECT_num(objs); i++) { + auto obj = sk_X509_OBJECT_value(objs, i); + if (X509_OBJECT_get_type(obj) == X509_LU_X509) { + auto cert = X509_OBJECT_get0_X509(obj); + if (cert) { + auto subject = X509_get_subject_name(cert); + if (subject) { + auto name_dup = X509_NAME_dup(subject); + if (name_dup) { sk_X509_NAME_push(ca_list, name_dup); } + } + } + } + } + + // If no names were extracted, free the list and return nullptr + if (sk_X509_NAME_num(ca_list) == 0) { + sk_X509_NAME_free(ca_list); + return nullptr; + } + + return ca_list; +} + // SSL HTTP client implementation inline SSLClient::SSLClient(const std::string &host) : SSLClient(host, 443, std::string(), std::string()) {} @@ -11145,6 +11208,11 @@ inline bool SSLClient::initialize_ssl(Socket &socket, Error &error) { return true; } + if (ctx_ == nullptr) { + error = Error::SSLConnection; + last_openssl_error_ = ERR_get_error(); + } + shutdown_socket(socket); close_socket(socket); return false; diff --git a/test/test.cc b/test/test.cc index 3779ca29f6..d6c06194cb 100644 --- a/test/test.cc +++ b/test/test.cc @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -8379,6 +8380,48 @@ TEST(SSLClientTest, ErrorReportingWhenInvalid) { EXPECT_EQ(Error::SSLConnection, res.error()); } +TEST(SSLClientTest, Issue2251_SwappedClientCertAndKey) { + // Test for Issue #2251: SSL error not properly reported when client cert + // and key paths are swapped or mismatched + // This simulates the scenario where user accidentally swaps the cert and key + // files + + // Using client cert file as private key and vice versa (completely wrong) + SSLClient cli("localhost", 8080, "client.key.pem", "client.cert.pem"); + + // Should fail validation due to cert/key mismatch + ASSERT_FALSE(cli.is_valid()); + + // Attempt to make a request should fail with proper error + auto res = cli.Get("/"); + ASSERT_FALSE(res); + EXPECT_EQ(Error::SSLConnection, res.error()); + + // SSL error should be recorded in the Result object (this is the key fix for + // Issue #2251) + auto openssl_error = res.ssl_openssl_error(); + EXPECT_NE(0u, openssl_error); +} + +TEST(SSLClientTest, Issue2251_ClientCertFileNotMatchingKey) { + // Another variant: using valid file paths but with mismatched cert/key pair + // This tests the case where files exist but contain incompatible key material + + // Using client cert with wrong key (cert2 key) + SSLClient cli("localhost", 8080, "client.cert.pem", "key.pem"); + + // Should fail validation + ASSERT_FALSE(cli.is_valid()); + + auto res = cli.Get("/"); + ASSERT_FALSE(res); + // Must report error properly, not appear as success + EXPECT_EQ(Error::SSLConnection, res.error()); + + // OpenSSL error should be captured in Result + EXPECT_NE(0u, res.ssl_openssl_error()); +} + #if 0 TEST(SSLClientTest, SetInterfaceWithINET6) { auto cli = std::make_shared("https://httpbin.org"); @@ -8721,6 +8764,197 @@ TEST(SSLClientServerTest, CustomizeServerSSLCtx) { ASSERT_EQ(StatusCode::OK_200, res->status); } +TEST(SSLClientServerTest, ClientCAListSentToClient) { + SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE, CLIENT_CA_CERT_FILE); + ASSERT_TRUE(svr.is_valid()); + + // Set up a handler to verify client certificate is present + bool client_cert_verified = false; + svr.Get("/test", [&](const Request & /*req*/, Response &res) { + // Verify that client certificate was provided + client_cert_verified = true; + res.set_content("success", "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(); + + // Client with certificate + SSLClient cli(HOST, PORT, CLIENT_CERT_FILE, CLIENT_PRIVATE_KEY_FILE); + cli.enable_server_certificate_verification(false); + cli.set_connection_timeout(30); + + auto res = cli.Get("/test"); + ASSERT_TRUE(res); + ASSERT_EQ(StatusCode::OK_200, res->status); + ASSERT_TRUE(client_cert_verified); + EXPECT_EQ("success", res->body); +} + +TEST(SSLClientServerTest, ClientCAListSetInContext) { + // Test that when client CA cert file is provided, + // SSL_CTX_set_client_CA_list is called and the CA list is properly set + + // Create a server with client authentication + SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE, CLIENT_CA_CERT_FILE); + ASSERT_TRUE(svr.is_valid()); + + // We can't directly access the SSL_CTX from SSLServer to verify, + // but we can test that the server properly requests client certificates + // and accepts valid ones from the specified CA + + bool handler_called = false; + svr.Get("/test", [&](const Request &req, Response &res) { + handler_called = true; + + // Verify that a client certificate was provided + auto peer_cert = SSL_get_peer_certificate(req.ssl); + ASSERT_TRUE(peer_cert != nullptr); + + // Get the issuer name + auto issuer_name = X509_get_issuer_name(peer_cert); + ASSERT_TRUE(issuer_name != nullptr); + + char issuer_buf[256]; + X509_NAME_oneline(issuer_name, issuer_buf, sizeof(issuer_buf)); + + // The client certificate should be issued by our test CA + std::string issuer_str(issuer_buf); + EXPECT_TRUE(issuer_str.find("Root CA Name") != std::string::npos); + + X509_free(peer_cert); + res.set_content("authenticated", "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(); + + // Connect with a client certificate issued by the CA + SSLClient cli(HOST, PORT, CLIENT_CERT_FILE, CLIENT_PRIVATE_KEY_FILE); + cli.enable_server_certificate_verification(false); + cli.set_connection_timeout(30); + + auto res = cli.Get("/test"); + ASSERT_TRUE(res); + ASSERT_EQ(StatusCode::OK_200, res->status); + ASSERT_TRUE(handler_called); + EXPECT_EQ("authenticated", res->body); +} + +TEST(SSLClientServerTest, ClientCAListLoadErrorRecorded) { + // Test 1: Valid CA file - no error should be recorded + { + SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE, + CLIENT_CA_CERT_FILE); + ASSERT_TRUE(svr.is_valid()); + + // With valid setup, last_ssl_error should be 0 + EXPECT_EQ(0, svr.ssl_last_error()); + } + + // Test 2: Invalid CA file content + // When SSL_load_client_CA_file fails, last_ssl_error_ should be set + { + // Create a temporary file with completely invalid content + const char *temp_invalid_ca = "./temp_invalid_ca_for_test.txt"; + { + std::ofstream ofs(temp_invalid_ca); + ofs << "This is not a certificate file at all\n"; + ofs << "Just plain text content\n"; + } + + // Create server with invalid CA file + SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE, temp_invalid_ca); + + // Clean up temporary file + std::remove(temp_invalid_ca); + + // When there's an SSL error (from either SSL_CTX_load_verify_locations + // or SSL_load_client_CA_file), last_ssl_error_ should be non-zero + // Note: SSL_CTX_load_verify_locations typically fails first, + // but our error handling code path is still exercised + if (!svr.is_valid()) { EXPECT_NE(0, svr.ssl_last_error()); } + } +} + +TEST(SSLClientServerTest, ClientCAListFromX509Store) { + // Test SSL server using X509_STORE constructor with client CA certificates + // This test verifies that Phase 2 implementation correctly extracts CA names + // from an X509_STORE and sets them in the SSL context + + // Load the CA certificate into memory + auto bio = BIO_new_file(CLIENT_CA_CERT_FILE, "r"); + ASSERT_NE(nullptr, bio); + + auto ca_cert = PEM_read_bio_X509(bio, nullptr, nullptr, nullptr); + BIO_free(bio); + ASSERT_NE(nullptr, ca_cert); + + // Create an X509_STORE and add the CA certificate + auto store = X509_STORE_new(); + ASSERT_NE(nullptr, store); + ASSERT_EQ(1, X509_STORE_add_cert(store, ca_cert)); + + // Load server certificate and private key + auto cert_bio = BIO_new_file(SERVER_CERT_FILE, "r"); + ASSERT_NE(nullptr, cert_bio); + auto server_cert = PEM_read_bio_X509(cert_bio, nullptr, nullptr, nullptr); + BIO_free(cert_bio); + ASSERT_NE(nullptr, server_cert); + + auto key_bio = BIO_new_file(SERVER_PRIVATE_KEY_FILE, "r"); + ASSERT_NE(nullptr, key_bio); + auto server_key = PEM_read_bio_PrivateKey(key_bio, nullptr, nullptr, nullptr); + BIO_free(key_bio); + ASSERT_NE(nullptr, server_key); + + // Create SSLServer with X509_STORE constructor + // Note: X509_STORE ownership is transferred to SSL_CTX + SSLServer svr(server_cert, server_key, store); + ASSERT_TRUE(svr.is_valid()); + + // No SSL error should be recorded for valid setup + EXPECT_EQ(0, svr.ssl_last_error()); + + // Set up server endpoints + svr.Get("/test-x509store", [&](const Request & /*req*/, Response &res) { + res.set_content("ok", "text/plain"); + }); + + // Start server in a thread + auto server_thread = thread([&]() { svr.listen(HOST, PORT); }); + svr.wait_until_ready(); + + // Connect with client certificate (using constructor with paths) + SSLClient cli(HOST, PORT, CLIENT_CERT_FILE, CLIENT_PRIVATE_KEY_FILE); + cli.enable_server_certificate_verification(false); + + auto res = cli.Get("/test-x509store"); + ASSERT_TRUE(res); + EXPECT_EQ(200, res->status); + EXPECT_EQ("ok", res->body); + + // Clean up + X509_free(server_cert); + EVP_PKEY_free(server_key); + X509_free(ca_cert); + + svr.stop(); + server_thread.join(); +} + // Disabled due to the out-of-memory problem on GitHub Actions Workflows TEST(SSLClientServerTest, DISABLED_LargeDataTransfer) {