From fb45f02d0af10e6aad26468e99370489079f782e Mon Sep 17 00:00:00 2001 From: Wang Xiaofeng Date: Sun, 25 Oct 2026 23:26:53 +0800 Subject: [PATCH 1/3] Release resources used by SSL diagnostic output --- src/brpc/details/ssl_helper.cpp | 8 ++-- test/brpc_ssl_unittest.cpp | 69 +++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/src/brpc/details/ssl_helper.cpp b/src/brpc/details/ssl_helper.cpp index 25ee4c6a83..8517ff61fc 100644 --- a/src/brpc/details/ssl_helper.cpp +++ b/src/brpc/details/ssl_helper.cpp @@ -953,14 +953,15 @@ void Print(std::ostream& os, SSL* ssl, const char* sep) { << "protocol=" << SSL_get_version(ssl) << sep << "verify=" << (SSL_get_verify_mode(ssl) & SSL_VERIFY_PEER ? "success" : "none"); - X509* cert = SSL_get_peer_certificate(ssl); + std::unique_ptr cert( + SSL_get_peer_certificate(ssl), X509_free); if (cert) { os << sep << "peer_certificate={"; const std::string new_sep = GetNextLevelSeparator(sep); if (sep[0] == '\n') { os << new_sep; } - Print(os, cert, new_sep.c_str()); + Print(os, cert.get(), new_sep.c_str()); if (sep[0] == '\n') { os << sep; } @@ -969,7 +970,8 @@ void Print(std::ostream& os, SSL* ssl, const char* sep) { } void Print(std::ostream& os, X509* cert, const char* sep) { - BIO* buf = BIO_new(BIO_s_mem()); + std::unique_ptr bio(BIO_new(BIO_s_mem())); + BIO* buf = bio.get(); if (buf == nullptr) { return; } diff --git a/test/brpc_ssl_unittest.cpp b/test/brpc_ssl_unittest.cpp index 6825709e91..4ae77a4b4e 100644 --- a/test/brpc_ssl_unittest.cpp +++ b/test/brpc_ssl_unittest.cpp @@ -20,6 +20,8 @@ // Date: Sun Jul 13 15:04:18 CST 2014 #include +#include +#include #include #include #include @@ -88,6 +90,73 @@ class SSLTest : public ::testing::Test{ virtual void TearDown() {}; }; +static void MarkCertificateFreed(void*, void* ptr, CRYPTO_EX_DATA*, + int, long, void*) { + if (ptr != nullptr) { + *static_cast(ptr) = true; + } +} + +TEST_F(SSLTest, diagnostic_print_releases_peer_certificate) { + bool certificate_freed = false; + std::unique_ptr server_ctx( + SSL_CTX_new(SSLv23_method()), SSL_CTX_free); + std::unique_ptr client_ctx( + SSL_CTX_new(SSLv23_method()), SSL_CTX_free); + ASSERT_TRUE(server_ctx); + ASSERT_TRUE(client_ctx); + ASSERT_EQ(1, SSL_CTX_use_certificate_chain_file(server_ctx.get(), "cert1.crt")); + ASSERT_EQ(1, SSL_CTX_use_PrivateKey_file( + server_ctx.get(), "cert1.key", SSL_FILETYPE_PEM)); + SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_OFF); + SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_OFF); + std::unique_ptr server( + SSL_new(server_ctx.get()), SSL_free); + std::unique_ptr client( + SSL_new(client_ctx.get()), SSL_free); + ASSERT_TRUE(server); + ASSERT_TRUE(client); + BIO* server_bio = nullptr; + BIO* client_bio = nullptr; + ASSERT_EQ(1, BIO_new_bio_pair(&server_bio, 0, &client_bio, 0)); + SSL_set_bio(server.get(), server_bio, server_bio); + SSL_set_bio(client.get(), client_bio, client_bio); + SSL_set_accept_state(server.get()); + SSL_set_connect_state(client.get()); + for (int i = 0; i < 100 && + (!SSL_is_init_finished(client.get()) || + !SSL_is_init_finished(server.get())); ++i) { + for (SSL* ssl : {client.get(), server.get()}) { + const int rc = SSL_do_handshake(ssl); + if (rc != 1) { + const int error = SSL_get_error(ssl, rc); + ASSERT_TRUE(error == SSL_ERROR_WANT_READ || + error == SSL_ERROR_WANT_WRITE) << error; + } + } + } + ASSERT_TRUE(SSL_is_init_finished(client.get())); + ASSERT_TRUE(SSL_is_init_finished(server.get())); + std::unique_ptr peer( + SSL_get_peer_certificate(client.get()), X509_free); + ASSERT_TRUE(peer); + const int index = X509_get_ex_new_index( + 0, nullptr, nullptr, nullptr, MarkCertificateFreed); + ASSERT_GE(index, 0); + ASSERT_EQ(1, X509_set_ex_data(peer.get(), index, &certificate_freed)); + for (int i = 0; i < 10; ++i) { + std::ostringstream os; + brpc::Print(os, client.get(), "\n"); + EXPECT_NE(std::string::npos, os.str().find("peer_certificate={")); + EXPECT_NE(std::string::npos, os.str().find("subject=")); + } + EXPECT_FALSE(certificate_freed); + peer.reset(); + client.reset(); + client_ctx.reset(); + EXPECT_TRUE(certificate_freed); +} + void* RunClosure(void* arg) { google::protobuf::Closure* done = (google::protobuf::Closure*)arg; done->Run(); From e92ce7a7ca5334e9b68e0634dcd30996a0e3e7cd Mon Sep 17 00:00:00 2001 From: Wang Xiaofeng Date: Mon, 26 Oct 2026 22:04:58 +0800 Subject: [PATCH 2/3] Guard OpenSSL diagnostic regression for MesaLink builds --- test/brpc_ssl_unittest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/brpc_ssl_unittest.cpp b/test/brpc_ssl_unittest.cpp index 4ae77a4b4e..3852be1d3a 100644 --- a/test/brpc_ssl_unittest.cpp +++ b/test/brpc_ssl_unittest.cpp @@ -90,6 +90,7 @@ class SSLTest : public ::testing::Test{ virtual void TearDown() {}; }; +#ifndef USE_MESALINK static void MarkCertificateFreed(void*, void* ptr, CRYPTO_EX_DATA*, int, long, void*) { if (ptr != nullptr) { @@ -156,6 +157,7 @@ TEST_F(SSLTest, diagnostic_print_releases_peer_certificate) { client_ctx.reset(); EXPECT_TRUE(certificate_freed); } +#endif // USE_MESALINK void* RunClosure(void* arg) { google::protobuf::Closure* done = (google::protobuf::Closure*)arg; From 97b168e2cff1ed4b64bdc61c7e097fca5026e037 Mon Sep 17 00:00:00 2001 From: Wang Xiaofeng Date: Mon, 26 Oct 2026 23:09:35 +0800 Subject: [PATCH 3/3] Refine SSL diagnostic ownership checks and reuse test index --- src/brpc/details/ssl_helper.cpp | 4 ++-- test/brpc_ssl_unittest.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/brpc/details/ssl_helper.cpp b/src/brpc/details/ssl_helper.cpp index 8517ff61fc..4408de74af 100644 --- a/src/brpc/details/ssl_helper.cpp +++ b/src/brpc/details/ssl_helper.cpp @@ -971,10 +971,10 @@ void Print(std::ostream& os, SSL* ssl, const char* sep) { void Print(std::ostream& os, X509* cert, const char* sep) { std::unique_ptr bio(BIO_new(BIO_s_mem())); - BIO* buf = bio.get(); - if (buf == nullptr) { + if (!bio) { return; } + BIO* buf = bio.get(); BIO_printf(buf, "subject="); X509_NAME_print(buf, X509_get_subject_name(cert), 0); BIO_printf(buf, "%sstart_date=", sep); diff --git a/test/brpc_ssl_unittest.cpp b/test/brpc_ssl_unittest.cpp index 3852be1d3a..3d61b45088 100644 --- a/test/brpc_ssl_unittest.cpp +++ b/test/brpc_ssl_unittest.cpp @@ -141,7 +141,7 @@ TEST_F(SSLTest, diagnostic_print_releases_peer_certificate) { std::unique_ptr peer( SSL_get_peer_certificate(client.get()), X509_free); ASSERT_TRUE(peer); - const int index = X509_get_ex_new_index( + static const int index = X509_get_ex_new_index( 0, nullptr, nullptr, nullptr, MarkCertificateFreed); ASSERT_GE(index, 0); ASSERT_EQ(1, X509_set_ex_data(peer.get(), index, &certificate_freed));