From adb9069c877cb70bf614375b6aa69d81368aeee9 Mon Sep 17 00:00:00 2001 From: nhada1 Date: Thu, 27 Aug 2026 15:06:18 -0400 Subject: [PATCH] Add nullptr check in logCertVerificationFailure --- src/rmq/rmqio/rmqio_asioresolver.cpp | 56 +++++++++----- src/rmq/rmqio/rmqio_asioresolver.h | 4 + src/tests/rmqio/rmqio_asioresolver.t.cpp | 95 ++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 18 deletions(-) diff --git a/src/rmq/rmqio/rmqio_asioresolver.cpp b/src/rmq/rmqio/rmqio_asioresolver.cpp index e986d054..e42ea3b0 100644 --- a/src/rmq/rmqio/rmqio_asioresolver.cpp +++ b/src/rmq/rmqio/rmqio_asioresolver.cpp @@ -101,23 +101,6 @@ void logTlsConnectionAlert(const SSL* s, int where, int ret) } } -bool logCertVerificationFailure(bool preverified, - boost::asio::ssl::verify_context& ctx) -{ - if (!preverified) { - char subject_name[256]; - X509* cert = X509_STORE_CTX_get_current_cert(ctx.native_handle()); - X509_NAME_oneline(X509_get_subject_name(cert), - subject_name, - sizeof(subject_name) - 1); - - BALL_LOG_ERROR << "Certificate verification failed: [" << subject_name - << "]: "; - } - - return preverified; -} - bsl::string augmentTlsError(const boost::system::error_code& ec) { bsl::string err = ec.message(); @@ -263,7 +246,7 @@ createSecureContext(const bsl::shared_ptr& params) } SSL_CTX_set_info_callback(result->native_handle(), &logTlsConnectionAlert); - result->set_verify_callback(&logCertVerificationFailure); + result->set_verify_callback(&AsioResolver::logCertVerificationFailure); if (fail) { result.reset(); @@ -485,6 +468,43 @@ void AsioResolver::handleConnect( } } +bool AsioResolver::logCertVerificationFailure( + bool preverified, + boost::asio::ssl::verify_context& ctx) +{ + if (preverified) { + return preverified; + } + + X509_STORE_CTX* storeCtx = ctx.native_handle(); + + if (!storeCtx) { + BALL_LOG_ERROR << "Certificate verification failed: no verification " + "context available"; + return preverified; + } + + const int errorCode = X509_STORE_CTX_get_error(storeCtx); + const int errorDepth = X509_STORE_CTX_get_error_depth(storeCtx); + + X509* cert = X509_STORE_CTX_get_current_cert(storeCtx); + X509_NAME* subject = cert ? X509_get_subject_name(cert) : 0; + + char subjectName[256] = "unavailable"; + + if (subject) { + X509_NAME_oneline(subject, subjectName, sizeof(subjectName)); + } + + BALL_LOG_ERROR << "Certificate verification failed: subjectName=" + << subjectName << " errorCode=" << errorCode + << " errorString=" + << X509_verify_cert_error_string(errorCode) + << " depth=" << errorDepth; + + return preverified; +} + void AsioResolver::shuffleResolverResults( AsioResolver::results_type& resolverResults, bool shuffleConnectionEndpoints, diff --git a/src/rmq/rmqio/rmqio_asioresolver.h b/src/rmq/rmqio/rmqio_asioresolver.h index 31855d33..00ffab5a 100644 --- a/src/rmq/rmqio/rmqio_asioresolver.h +++ b/src/rmq/rmqio/rmqio_asioresolver.h @@ -107,6 +107,10 @@ class AsioResolver : public Resolver, const bsl::string& host, const bsl::string& port); + static bool + logCertVerificationFailure(bool preverified, + boost::asio::ssl::verify_context& ctx); + private: explicit AsioResolver(AsioEventLoop& eventloop, bool shuffleConnectionEndpoints); diff --git a/src/tests/rmqio/rmqio_asioresolver.t.cpp b/src/tests/rmqio/rmqio_asioresolver.t.cpp index ad14d5e5..07e2e7e5 100644 --- a/src/tests/rmqio/rmqio_asioresolver.t.cpp +++ b/src/tests/rmqio/rmqio_asioresolver.t.cpp @@ -20,6 +20,8 @@ #include #include +#include + #include #include #include @@ -48,6 +50,52 @@ class ResolverTests : public Test { }; }; +class StoreContextGuard { + public: + StoreContextGuard() + : d_ctx(X509_STORE_CTX_new()) + { + } + + ~StoreContextGuard() + { + if (d_ctx) { + X509_STORE_CTX_free(d_ctx); + } + } + + X509_STORE_CTX* get() { return d_ctx; } + + private: + StoreContextGuard(const StoreContextGuard&); + StoreContextGuard& operator=(const StoreContextGuard&); + + X509_STORE_CTX* d_ctx; +}; + +class CertificateGuard { + public: + CertificateGuard() + : d_cert(X509_new()) + { + } + + ~CertificateGuard() + { + if (d_cert) { + X509_free(d_cert); + } + } + + X509* get() { return d_cert; } + + private: + CertificateGuard(const CertificateGuard&); + CertificateGuard& operator=(const CertificateGuard&); + + X509* d_cert; +}; + } // namespace TEST_F(ResolverTests, Breathing) @@ -56,6 +104,53 @@ TEST_F(ResolverTests, Breathing) bsl::shared_ptr resolver(AsioResolver::create(loop, false)); } +TEST_F(ResolverTests, LogCertVerificationFailureNullCurrentCert) +{ + StoreContextGuard storeCtx; + ASSERT_THAT(storeCtx.get(), NotNull()); + ASSERT_THAT(X509_STORE_CTX_init(storeCtx.get(), 0, 0, 0), Eq(1)); + ASSERT_THAT(X509_STORE_CTX_get_current_cert(storeCtx.get()), IsNull()); + + boost::asio::ssl::verify_context ctx(storeCtx.get()); + + EXPECT_FALSE(AsioResolver::logCertVerificationFailure(false, ctx)); +} + +TEST_F(ResolverTests, LogCertVerificationFailureNullStoreContext) +{ + boost::asio::ssl::verify_context ctx(0); + + EXPECT_FALSE(AsioResolver::logCertVerificationFailure(false, ctx)); +} + +TEST_F(ResolverTests, LogCertVerificationFailureWithSubject) +{ + StoreContextGuard storeCtx; + ASSERT_THAT(storeCtx.get(), NotNull()); + ASSERT_THAT(X509_STORE_CTX_init(storeCtx.get(), 0, 0, 0), Eq(1)); + + CertificateGuard cert; + ASSERT_THAT(cert.get(), NotNull()); + ASSERT_THAT(X509_get_subject_name(cert.get()), NotNull()); + + X509_STORE_CTX_set_current_cert(storeCtx.get(), cert.get()); + + boost::asio::ssl::verify_context ctx(storeCtx.get()); + + EXPECT_FALSE(AsioResolver::logCertVerificationFailure(false, ctx)); +} + +TEST_F(ResolverTests, LogCertVerificationPreverifiedPassesThrough) +{ + StoreContextGuard storeCtx; + ASSERT_THAT(storeCtx.get(), NotNull()); + ASSERT_THAT(X509_STORE_CTX_init(storeCtx.get(), 0, 0, 0), Eq(1)); + + boost::asio::ssl::verify_context ctx(storeCtx.get()); + + EXPECT_TRUE(AsioResolver::logCertVerificationFailure(true, ctx)); +} + TEST_F(ResolverTests, badresolve) { using bdlf::PlaceHolders::_1;