SERVER-57601: OCSPFetcher must verify that the SSLConnectionContext that owns SSLManagerOpenSSL is still valid

6 files changed, 111 insertions, 7 deletions

diff --git a/src/mongo/transport/transport_layer_asio.cpp b/src/mongo/transport/transport_layer_asio.cpp
index 205a3066949..34c75d0b144 100644
--- a/src/mongo/transport/transport_layer_asio.cpp
+++ b/src/mongo/transport/transport_layer_asio.cpp
@@ -1228,7 +1228,10 @@ SSLParams::SSLModes TransportLayerASIO::_sslMode() const {
 
 Status TransportLayerASIO::rotateCertificates(std::shared_ptr<SSLManagerInterface> manager,
                                               bool asyncOCSPStaple) {
-
+    if (manager && manager->isTransient()) {
+        return Status(ErrorCodes::InternalError,
+                      "Should not rotate transient SSL manager's certificates");
+    }
     auto contextOrStatus = _createSSLContext(manager, _sslMode(), asyncOCSPStaple);
     if (!contextOrStatus.isOK()) {
         return contextOrStatus.getStatus();
@@ -1257,6 +1260,8 @@ TransportLayerASIO::_createSSLContext(std::shared_ptr<SSLManagerInterface>& mana
             return status;
         }
 
+        std::weak_ptr<const SSLConnectionContext> weakContextPtr = newSSLContext;
+        manager->registerOwnedBySSLContext(weakContextPtr);
         auto resp = newSSLContext->manager->stapleOCSPResponse(
             newSSLContext->ingress->native_handle(), asyncOCSPStaple);
 
@@ -1292,9 +1297,7 @@ TransportLayerASIO::createTransientSSLContext(const TransientSSLParams& transien
                       "SSLManagerCoordinator is not initialized");
     }
     auto manager = coordinator->createTransientSSLManager(transientSSLParams);
-    if (!manager) {
-        return Status(ErrorCodes::InvalidSSLConfiguration, "TransportLayerASIO has no SSL manager");
-    }
+    invariant(manager);
 
     return _createSSLContext(manager, _sslMode(), true /* asyncOCSPStaple */);
 }
diff --git a/src/mongo/util/net/ssl_manager.h b/src/mongo/util/net/ssl_manager.h
index 567011971b5..f651821e64c 100644
--- a/src/mongo/util/net/ssl_manager.h
+++ b/src/mongo/util/net/ssl_manager.h
@@ -80,6 +80,10 @@ namespace mongo {
 struct SSLParams;
 struct TransientSSLParams;
 
+namespace transport {
+struct SSLConnectionContext;
+}  // namespace transport
+
 #if MONGO_CONFIG_SSL_PROVIDER == MONGO_CONFIG_SSL_PROVIDER_OPENSSL
 typedef SSL_CTX* SSLContextType;
 typedef SSL* SSLConnectionType;
@@ -322,6 +326,12 @@ public:
                                   ConnectionDirection direction) = 0;
 
     /**
+     * Registers this SSL context as the owner of this manager.
+     */
+    virtual void registerOwnedBySSLContext(
+        std::weak_ptr<const transport::SSLConnectionContext> ownedByContext) = 0;
+
+    /**
      * Fetches a peer certificate and validates it if it exists. If validation fails, but weak
      * validation is enabled, the `subjectName` will be empty. If validation fails, and invalid
      * certificates are not allowed, a non-OK status will be returned. If validation is successful,
diff --git a/src/mongo/util/net/ssl_manager_apple.cpp b/src/mongo/util/net/ssl_manager_apple.cpp
index eb6d132f1d3..aba7cbb39ce 100644
--- a/src/mongo/util/net/ssl_manager_apple.cpp
+++ b/src/mongo/util/net/ssl_manager_apple.cpp
@@ -44,6 +44,7 @@
 #include "mongo/crypto/sha256_block.h"
 #include "mongo/logv2/log.h"
 #include "mongo/platform/random.h"
+#include "mongo/transport/ssl_connection_context.h"
 #include "mongo/util/base64.h"
 #include "mongo/util/concurrency/mutex.h"
 #include "mongo/util/fail_point.h"
@@ -69,6 +70,8 @@ extern "C" SecIdentityRef SecIdentityCreate(CFAllocatorRef, SecCertificateRef, S
 
 namespace mongo {
 
+using transport::SSLConnectionContext;
+
 namespace {
 
 // This failpoint is a no-op on OSX.
@@ -1275,6 +1278,9 @@ public:
                           const SSLParams& params,
                           ConnectionDirection direction) final;
 
+    void registerOwnedBySSLContext(
+        std::weak_ptr<const transport::SSLConnectionContext> ownedByContext) final;
+
     SSLConnectionInterface* connect(Socket* socket) final;
     SSLConnectionInterface* accept(Socket* socket, const char* initialBytes, int len) final;
 
@@ -1324,6 +1330,10 @@ private:
     CFUniquePtr<::CFArrayRef> _serverCA;
 
     SSLConfiguration _sslConfiguration;
+
+    // Weak pointer to verify that this manager is still owned by this context.
+    // Will be used if stapling is implemented.
+    synchronized_value<std::weak_ptr<const SSLConnectionContext>> _ownedByContext;
 };
 
 SSLManagerApple::SSLManagerApple(const SSLParams& params, bool isServer)
@@ -1474,6 +1484,11 @@ Status SSLManagerApple::initSSLContext(asio::ssl::apple::Context* context,
         params.sslCertificateSelector, params.sslPEMKeyFile, params.sslPEMKeyPassword);
 }
 
+void SSLManagerApple::registerOwnedBySSLContext(
+    std::weak_ptr<const transport::SSLConnectionContext> ownedByContext) {
+    _ownedByContext = ownedByContext;
+}
+
 SSLConnectionInterface* SSLManagerApple::connect(Socket* socket) {
     return new SSLConnectionApple(
         &_clientCtx, socket, ::kSSLClientSide, socket->remoteAddr().hostOrIp());
diff --git a/src/mongo/util/net/ssl_manager_openssl.cpp b/src/mongo/util/net/ssl_manager_openssl.cpp
index bbf5315abe3..d21cb8be8fd 100644
--- a/src/mongo/util/net/ssl_manager_openssl.cpp
+++ b/src/mongo/util/net/ssl_manager_openssl.cpp
@@ -52,6 +52,7 @@
 #include "mongo/logv2/log.h"
 #include "mongo/platform/atomic_word.h"
 #include "mongo/transport/session.h"
+#include "mongo/transport/ssl_connection_context.h"
 #include "mongo/util/assert_util.h"
 #include "mongo/util/concurrency/mutex.h"
 #include "mongo/util/debug_util.h"
@@ -101,6 +102,8 @@ using namespace fmt::literals;
 
 namespace mongo {
 
+using transport::SSLConnectionContext;
+
 namespace {
 
 MONGO_FAIL_POINT_DEFINE(disableStapling);
@@ -1108,6 +1111,10 @@ private:
 
     void doPeriodicJob();
 
+    bool _isShutdownConditionLocked(WithLock);
+
+    void _shutdownLocked(WithLock);
+
 private:
     // Hack
     // OpenSSL 1.1.0 = Since OpenSSL is ref counted underneath, we should use SSL_CTX_up_ref.
@@ -1121,6 +1128,10 @@ private:
     // object is alive.
     SSLManagerOpenSSL* _manager;
 
+    // Weak pointer to verify that the context owning the manager above is still valid
+    // and the manager it owns still matches the manager.
+    std::weak_ptr<const SSLConnectionContext> _ownedByContext;
+
     Mutex _staplingMutex = MONGO_MAKE_LATCH("OCSPStaplingJobRunner::_mutex");
     PeriodicRunner::JobAnchor _ocspStaplingAnchor;
     bool _shutdown{false};
@@ -1144,6 +1155,12 @@ public:
                           const SSLParams& params,
                           ConnectionDirection direction) final;
 
+    void registerOwnedBySSLContext(std::weak_ptr<const SSLConnectionContext> ownedByContext) final;
+
+    std::weak_ptr<const SSLConnectionContext> getSSLContextOwner() {
+        return *_ownedByContext;
+    }
+
     SSLConnectionInterface* connect(Socket* socket) final;
 
     SSLConnectionInterface* accept(Socket* socket, const char* initialBytes, int len) final;
@@ -1207,6 +1224,9 @@ private:
     // with TransientSSLParams::targetedClusterConnectionString.
     const std::optional<TransientSSLParams> _transientSSLParams;
 
+    // Weak pointer to verify that this manager is still owned by this context.
+    synchronized_value<std::weak_ptr<const SSLConnectionContext>> _ownedByContext;
+
     Mutex _sharedResponseMutex = MONGO_MAKE_LATCH("OCSPStaplingJobRunner::_sharedResponseMutex");
     std::shared_ptr<OCSPStaplingContext> _ocspStaplingContext;
 
@@ -1987,6 +2007,9 @@ Status OCSPFetcher::start(SSL_CTX* context, bool asyncOCSPStaple) {
     // the OCSPFetcher
     _ssl = UniqueSSL(SSL_new(context));
     _context = context;
+    _ownedByContext = _manager->getSSLContextOwner();
+    // Verify the consistent link between manager and the context that owns it.
+    invariant(_ownedByContext.lock()->manager.get() == _manager);
 
     auto certificateHolder = getCertificateForContext(_context);
     _cert = std::get<X509*>(certificateHolder);
@@ -2050,7 +2073,7 @@ void OCSPFetcher::startPeriodicJob(StatusWith<Milliseconds> swDurationInitial) {
         return;
     }
 
-    if (_shutdown) {
+    if (_isShutdownConditionLocked(lock)) {
         return;
     }
 
@@ -2074,13 +2097,40 @@ void OCSPFetcher::doPeriodicJob() {
 
             stdx::lock_guard<Latch> lock(this->_staplingMutex);
 
-            if (_shutdown) {
+            if (_isShutdownConditionLocked(lock)) {
                 return;
             }
 
             this->_ocspStaplingAnchor.setPeriod(getPeriodForStapleJob(swDuration));
         });
 }
+
+bool OCSPFetcher::_isShutdownConditionLocked(WithLock lock) {
+    if (_shutdown) {
+        return true;
+    }
+
+    const auto lockedContext = _ownedByContext.lock();
+    const auto lockedContextFromManager = _manager->getSSLContextOwner().lock();
+    if (!lockedContext) {
+        LOGV2(5760101,
+              "SSLConnectionContext that should own the manager for the active OCSPFetcher is "
+              "deleted");
+        _shutdownLocked(lock);
+        return true;
+    }
+
+    if (lockedContext.get() != lockedContextFromManager.get()) {
+        LOGV2_WARNING(5760102,
+                      "SSLConnectionContext that should own the manager for the active OCSPFetcher "
+                      "doesn't match");
+        _shutdownLocked(lock);
+        return true;
+    }
+
+    return false;
+}
+
 #if OPENSSL_VERSION_NUMBER < 0x10100000L
 void sslContextGetOtherCerts(SSL_CTX* ctx, STACK_OF(X509) * *sk) {
     SSL_CTX_get_extra_chain_certs(ctx, sk);
@@ -2130,7 +2180,7 @@ Future<Milliseconds> OCSPFetcher::fetchAndStaple(Promise<void>* promise) {
             [this, promise](StatusWith<OCSPFetchResponse> swResponse) mutable -> Milliseconds {
                 stdx::lock_guard<Latch> lock(this->_staplingMutex);
 
-                if (_shutdown) {
+                if (_isShutdownConditionLocked(lock)) {
                     return kOCSPUnknownStatusRefreshRate;
                 }
 
@@ -2149,7 +2199,10 @@ Future<Milliseconds> OCSPFetcher::fetchAndStaple(Promise<void>* promise) {
 
 void OCSPFetcher::shutdown() {
     stdx::lock_guard<Mutex> lock(_staplingMutex);
+    _shutdownLocked(lock);
+}
 
+void OCSPFetcher::_shutdownLocked(WithLock) {
     _shutdown = true;
 
     if (_ocspStaplingAnchor.isValid()) {
@@ -2386,6 +2439,11 @@ Status SSLManagerOpenSSL::initSSLContext(SSL_CTX* context,
     return Status::OK();
 }
 
+void SSLManagerOpenSSL::registerOwnedBySSLContext(
+    std::weak_ptr<const SSLConnectionContext> ownedByContext) {
+    _ownedByContext = ownedByContext;
+}
+
 bool SSLManagerOpenSSL::_initSynchronousSSLContext(UniqueSSLContext* contextPtr,
                                                    const SSLParams& params,
                                                    ConnectionDirection direction) {
diff --git a/src/mongo/util/net/ssl_manager_test.cpp b/src/mongo/util/net/ssl_manager_test.cpp
index 2fc17e8f336..72902a32106 100644
--- a/src/mongo/util/net/ssl_manager_test.cpp
+++ b/src/mongo/util/net/ssl_manager_test.cpp
@@ -622,6 +622,9 @@ TEST(SSLManager, TransientSSLParams) {
     ASSERT_TRUE(swContext.getValue()->manager->isTransient());
     ASSERT_EQ(transientSSLParams.targetedClusterConnectionString.toString(),
               swContext.getValue()->manager->getTargetedClusterConnectionString());
+
+    // Cannot rotate certs on transient manager.
+    ASSERT_NOT_OK(tla.rotateCertificates(swContext.getValue()->manager, true));
 }
 
 #endif  // MONGO_CONFIG_SSL_PROVIDER == MONGO_CONFIG_SSL_PROVIDER_OPENSSL
diff --git a/src/mongo/util/net/ssl_manager_windows.cpp b/src/mongo/util/net/ssl_manager_windows.cpp
index 4bfb4141a52..4e9e6666faa 100644
--- a/src/mongo/util/net/ssl_manager_windows.cpp
+++ b/src/mongo/util/net/ssl_manager_windows.cpp
@@ -49,6 +49,7 @@
 #include "mongo/db/server_options.h"
 #include "mongo/logv2/log.h"
 #include "mongo/platform/atomic_word.h"
+#include "mongo/transport/ssl_connection_context.h"
 #include "mongo/util/concurrency/mutex.h"
 #include "mongo/util/debug_util.h"
 #include "mongo/util/exit.h"
@@ -69,6 +70,8 @@
 
 namespace mongo {
 
+using transport::SSLConnectionContext;
+
 extern SSLManagerCoordinator* theSSLManagerCoordinator;
 
 namespace {
@@ -271,6 +274,9 @@ public:
                           const SSLParams& params,
                           ConnectionDirection direction) final;
 
+    void registerOwnedBySSLContext(
+        std::weak_ptr<const transport::SSLConnectionContext> ownedByContext) final;
+
     SSLConnectionInterface* connect(Socket* socket) final;
 
     SSLConnectionInterface* accept(Socket* socket, const char* initialBytes, int len) final;
@@ -351,6 +357,10 @@ private:
 
     UniqueCertificate _sslCertificate;
     UniqueCertificate _sslClusterCertificate;
+
+    // Weak pointer to verify that this manager is still owned by this context.
+    // Will be used if stapling is implemented.
+    synchronized_value<std::weak_ptr<const SSLConnectionContext>> _ownedByContext;
 };
 
 GlobalInitializerRegisterer sslManagerInitializer(
@@ -1433,6 +1443,11 @@ Status SSLManagerWindows::initSSLContext(SCHANNEL_CRED* cred,
     return Status::OK();
 }
 
+void SSLManagerWindows::registerOwnedBySSLContext(
+    std::weak_ptr<const SSLConnectionContext> ownedByContext) {
+    _ownedByContext = ownedByContext;
+}
+
 SSLConnectionInterface* SSLManagerWindows::connect(Socket* socket) {
     std::unique_ptr<SSLConnectionWindows> sslConn =
         std::make_unique<SSLConnectionWindows>(&_clientCred, socket, nullptr, 0);