summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Shuvalov <andrew.shuvalov@mongodb.com>2021-06-18 00:58:06 +0000
committerAndrew Shuvalov <andrew.shuvalov@mongodb.com>2021-06-25 16:09:16 +0000
commita2924c2fa049b664831fcca6bc34d0dfd9cf33dc (patch)
treea88db19da4e65eb499e94de7e40014c0fe6ea58e
parent275643f8603a22ee103ac161aa1ad8f687d78285 (diff)
downloadmongo-a2924c2fa049b664831fcca6bc34d0dfd9cf33dc.tar.gz
SERVER-57601: OCSPFetcher must verify that the SSLConnectionContext that owns SSLManagerOpenSSL is still valid
-rw-r--r--src/mongo/transport/transport_layer_asio.cpp11
-rw-r--r--src/mongo/util/net/ssl_manager.h10
-rw-r--r--src/mongo/util/net/ssl_manager_apple.cpp15
-rw-r--r--src/mongo/util/net/ssl_manager_openssl.cpp64
-rw-r--r--src/mongo/util/net/ssl_manager_test.cpp3
-rw-r--r--src/mongo/util/net/ssl_manager_windows.cpp15
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);