diff options
author | Gregory Wlodarek <gregory.wlodarek@mongodb.com> | 2021-02-11 19:07:03 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-02-11 19:42:32 +0000 |
commit | 3b4f12abc5d118ea461c4613b7d2475f6c4284cf (patch) | |
tree | 56d5231f0657a0a854d7e183d8b3754e4d64179b /src/mongo/util | |
parent | 330a7b661f2d2f49b638f63508af1b4a2974534a (diff) | |
download | mongo-3b4f12abc5d118ea461c4613b7d2475f6c4284cf.tar.gz |
Revert "SERVER-54328: Refactor creation of transient SSLConnectionContext to own its own instance of SSLManagerInterface"
This reverts commit 8e1cd3402cc0c27d1332ac78a93919bd17d3d556.
Diffstat (limited to 'src/mongo/util')
-rw-r--r-- | src/mongo/util/net/ssl_manager.cpp | 6 | ||||
-rw-r--r-- | src/mongo/util/net/ssl_manager.h | 36 | ||||
-rw-r--r-- | src/mongo/util/net/ssl_manager_apple.cpp | 16 | ||||
-rw-r--r-- | src/mongo/util/net/ssl_manager_openssl.cpp | 74 | ||||
-rw-r--r-- | src/mongo/util/net/ssl_manager_test.cpp | 49 | ||||
-rw-r--r-- | src/mongo/util/net/ssl_manager_windows.cpp | 17 |
6 files changed, 64 insertions, 134 deletions
diff --git a/src/mongo/util/net/ssl_manager.cpp b/src/mongo/util/net/ssl_manager.cpp index 31a858c8910..d80d882fe87 100644 --- a/src/mongo/util/net/ssl_manager.cpp +++ b/src/mongo/util/net/ssl_manager.cpp @@ -330,12 +330,6 @@ SSLManagerCoordinator* SSLManagerCoordinator::get() { return theSSLManagerCoordinator; } -std::shared_ptr<SSLManagerInterface> SSLManagerCoordinator::createTransientSSLManager( - const TransientSSLParams& transientSSLParams) const { - return SSLManagerInterface::create( - sslGlobalParams, transientSSLParams, false /* isSSLServer */); -} - std::shared_ptr<SSLManagerInterface> SSLManagerCoordinator::getSSLManager() { return *_manager; } diff --git a/src/mongo/util/net/ssl_manager.h b/src/mongo/util/net/ssl_manager.h index 567011971b5..b7cfd8b8099 100644 --- a/src/mongo/util/net/ssl_manager.h +++ b/src/mongo/util/net/ssl_manager.h @@ -208,16 +208,7 @@ class SSLManagerInterface : public Decorable<SSLManagerInterface> { public: /** * Creates an instance of SSLManagerInterface. - * Note: if 'transientSSLParams' is set, this will create a transient instance of the manager, - * otherwise, normally, this will be a global instance. - */ - static std::shared_ptr<SSLManagerInterface> create( - const SSLParams& params, - const std::optional<TransientSSLParams>& transientSSLParams, - bool isServer); - - /** - * Creates an instance of SSLManagerInterface without transient SSL params. + * Note: as we normally have one instance of the manager, it cannot take TransientSSLParams. */ static std::shared_ptr<SSLManagerInterface> create(const SSLParams& params, bool isServer); @@ -259,23 +250,6 @@ public: */ virtual const SSLConfiguration& getSSLConfiguration() const = 0; - /** - * @return true if this manager was created with 'transientSSLParams' to authenticate with - * a particular remote cluster. - */ - virtual bool isTransient() const { - return false; - } - - /** - * @return Connection string for the remote cluster if this manager is transient (isTransient() - * == true), otherwise returns empty string. - */ - virtual std::string getTargetedClusterConnectionString() const { - invariant(!isTransient()); - return {}; - } - #if MONGO_CONFIG_SSL_PROVIDER == MONGO_CONFIG_SSL_PROVIDER_OPENSSL /** * Fetches the error text for an error code, in a thread-safe manner. @@ -319,6 +293,7 @@ public: */ virtual Status initSSLContext(SSLContextType context, const SSLParams& params, + const TransientSSLParams& transientParams, ConnectionDirection direction) = 0; /** @@ -375,13 +350,6 @@ public: std::shared_ptr<SSLManagerInterface> getSSLManager(); /** - * Create a transient instance of SSL Manager. - * Ownership of the new manager is passed to the invoker. - */ - std::shared_ptr<SSLManagerInterface> createTransientSSLManager( - const TransientSSLParams& transientSSLParams) const; - - /** * Perform certificate rotation safely. */ void rotate(); diff --git a/src/mongo/util/net/ssl_manager_apple.cpp b/src/mongo/util/net/ssl_manager_apple.cpp index 7fae9426793..0fa197d3b65 100644 --- a/src/mongo/util/net/ssl_manager_apple.cpp +++ b/src/mongo/util/net/ssl_manager_apple.cpp @@ -1250,7 +1250,8 @@ public: Status initSSLContext(asio::ssl::apple::Context* context, const SSLParams& params, - ConnectionDirection direction) final; + const TransientSSLParams& transientParams, + ConnectionDirection direction) override final; SSLConnectionInterface* connect(Socket* socket) final; SSLConnectionInterface* accept(Socket* socket, const char* initialBytes, int len) final; @@ -1309,14 +1310,16 @@ SSLManagerApple::SSLManagerApple(const SSLParams& params, bool isServer) _allowInvalidHostnames(params.sslAllowInvalidHostnames), _suppressNoCertificateWarning(params.suppressNoTLSPeerCertificateWarning) { - uassertStatusOK(initSSLContext(&_clientCtx, params, ConnectionDirection::kOutgoing)); + uassertStatusOK( + initSSLContext(&_clientCtx, params, TransientSSLParams(), ConnectionDirection::kOutgoing)); if (_clientCtx.certs) { _sslConfiguration.clientSubjectName = uassertStatusOK(certificateGetSubject(_clientCtx.certs.get())); } if (isServer) { - uassertStatusOK(initSSLContext(&_serverCtx, params, ConnectionDirection::kIncoming)); + uassertStatusOK(initSSLContext( + &_serverCtx, params, TransientSSLParams(), ConnectionDirection::kIncoming)); if (_serverCtx.certs) { uassertStatusOK( _sslConfiguration.setServerSubjectName(uassertStatusOK(certificateGetSubject( @@ -1390,6 +1393,7 @@ StatusWith<std::pair<::SSLProtocol, ::SSLProtocol>> parseProtocolRange(const SSL Status SSLManagerApple::initSSLContext(asio::ssl::apple::Context* context, const SSLParams& params, + const TransientSSLParams& transientParams, ConnectionDirection direction) { // Protocol Version. const auto swProto = parseProtocolRange(params); @@ -1822,10 +1826,8 @@ bool isSSLServer = false; extern SSLManagerInterface* theSSLManager; extern SSLManagerCoordinator* theSSLManagerCoordinator; -std::shared_ptr<SSLManagerInterface> SSLManagerInterface::create( - const SSLParams& params, - const std::optional<TransientSSLParams>& transientSSLParams, - bool isServer) { +std::shared_ptr<SSLManagerInterface> SSLManagerInterface::create(const SSLParams& params, + bool isServer) { return std::make_shared<SSLManagerApple>(params, isServer); } diff --git a/src/mongo/util/net/ssl_manager_openssl.cpp b/src/mongo/util/net/ssl_manager_openssl.cpp index a786fb9c984..1dfd01c5805 100644 --- a/src/mongo/util/net/ssl_manager_openssl.cpp +++ b/src/mongo/util/net/ssl_manager_openssl.cpp @@ -1129,9 +1129,7 @@ private: class SSLManagerOpenSSL : public SSLManagerInterface, public std::enable_shared_from_this<SSLManagerOpenSSL> { public: - explicit SSLManagerOpenSSL(const SSLParams& params, - const std::optional<TransientSSLParams>& transientSSLParams, - bool isServer); + explicit SSLManagerOpenSSL(const SSLParams& params, bool isServer); ~SSLManagerOpenSSL() { stopJobs(); } @@ -1142,6 +1140,7 @@ public: */ Status initSSLContext(SSL_CTX* context, const SSLParams& params, + const TransientSSLParams& transientParams, ConnectionDirection direction) final; SSLConnectionInterface* connect(Socket* socket) final; @@ -1170,10 +1169,6 @@ public: return _sslConfiguration; } - bool isTransient() const final; - - std::string getTargetedClusterConnectionString() const final; - int SSL_read(SSLConnectionInterface* conn, void* buf, int num) final; int SSL_write(SSLConnectionInterface* conn, const void* buf, int num) final; @@ -1203,9 +1198,6 @@ private: bool _allowInvalidHostnames; bool _suppressNoCertificateWarning; SSLConfiguration _sslConfiguration; - // If set, this manager is an instance providing authentication with remote server specified - // with TransientSSLParams::targetedClusterConnectionString. - const std::optional<TransientSSLParams> _transientSSLParams; Mutex _sharedResponseMutex = MONGO_MAKE_LATCH("OCSPStaplingJobRunner::_sharedResponseMutex"); std::shared_ptr<OCSPStaplingContext> _ocspStaplingContext; @@ -1271,7 +1263,6 @@ private: std::string _prompt; }; - PasswordFetcher _serverPEMPassword; PasswordFetcher _clusterPEMPassword; @@ -1453,17 +1444,9 @@ MONGO_INITIALIZER_WITH_PREREQUISITES(SSLManager, ("SetupOpenSSL", "EndStartupOpt } } -std::shared_ptr<SSLManagerInterface> SSLManagerInterface::create( - const SSLParams& params, - const std::optional<TransientSSLParams>& transientSSLParams, - bool isServer) { - return std::make_shared<SSLManagerOpenSSL>(params, transientSSLParams, isServer); -} - std::shared_ptr<SSLManagerInterface> SSLManagerInterface::create(const SSLParams& params, bool isServer) { - return std::make_shared<SSLManagerOpenSSL>( - params, std::optional<TransientSSLParams>{}, isServer); + return std::make_shared<SSLManagerOpenSSL>(params, isServer); } SSLX509Name getCertificateSubjectX509Name(X509* cert) { @@ -1554,16 +1537,13 @@ SSLConnectionOpenSSL::~SSLConnectionOpenSSL() { } } -SSLManagerOpenSSL::SSLManagerOpenSSL(const SSLParams& params, - const std::optional<TransientSSLParams>& transientSSLParams, - bool isServer) +SSLManagerOpenSSL::SSLManagerOpenSSL(const SSLParams& params, bool isServer) : _serverContext(nullptr), _clientContext(nullptr), _weakValidation(params.sslWeakCertificateValidation), _allowInvalidCertificates(params.sslAllowInvalidCertificates), _allowInvalidHostnames(params.sslAllowInvalidHostnames), _suppressNoCertificateWarning(params.suppressNoTLSPeerCertificateWarning), - _transientSSLParams(transientSSLParams), _fetcher(this), _serverPEMPassword(params.sslPEMKeyPassword, "Enter PEM passphrase"), _clusterPEMPassword(params.sslClusterPassword, "Enter cluster certificate passphrase") { @@ -1571,13 +1551,6 @@ SSLManagerOpenSSL::SSLManagerOpenSSL(const SSLParams& params, uasserted(16768, "ssl initialization problem"); } - if (_transientSSLParams.has_value()) { - // No other initialization is necessary: this is egress connection manager that - // is not using local PEM files. - LOGV2_DEBUG(54090, 1, "Default params are ignored for transient SSL manager"); - return; - } - // pick the certificate for use in outgoing connections, std::string clientPEM; PasswordFetcher* clientPassword; @@ -2158,28 +2131,11 @@ Milliseconds SSLManagerOpenSSL::updateOcspStaplingContextWithResponse( return swResponse.getValue().fetchNewResponseDuration(); } -bool SSLManagerOpenSSL::isTransient() const { - return _transientSSLParams.has_value(); -} - -std::string SSLManagerOpenSSL::getTargetedClusterConnectionString() const { - if (_transientSSLParams.has_value()) { - return (*_transientSSLParams).targetedClusterConnectionString.toString(); - } - return {}; -} Status SSLManagerOpenSSL::initSSLContext(SSL_CTX* context, const SSLParams& params, + const TransientSSLParams& transientParams, ConnectionDirection direction) { - if (isTransient()) { - LOGV2_DEBUG(5270602, - 2, - "Initializing transient egress SSL context", - "targetClusterConnectionString"_attr = - (*_transientSSLParams).targetedClusterConnectionString); - } - // SSL_OP_ALL - Activate all bug workaround options, to support buggy client SSL's. // SSL_OP_NO_SSLv2 - Disable SSL v2 support // SSL_OP_NO_SSLv3 - Disable SSL v3 support @@ -2241,24 +2197,24 @@ Status SSLManagerOpenSSL::initSSLContext(SSL_CTX* context, } - if (direction == ConnectionDirection::kOutgoing && _transientSSLParams) { + if (direction == ConnectionDirection::kOutgoing && + !transientParams.sslClusterPEMPayload.empty()) { // Transient params for outgoing connection have priority over global params. if (!_setupPEMFromMemoryPayload( context, - (*_transientSSLParams).sslClusterPEMPayload, + transientParams.sslClusterPEMPayload, &_clusterPEMPassword, - (*_transientSSLParams).targetedClusterConnectionString.toString())) { + transientParams.targetedClusterConnectionString.toString())) { return Status(ErrorCodes::InvalidSSLConfiguration, str::stream() << "Can not set up transient ssl cluster certificate for " - << (*_transientSSLParams).targetedClusterConnectionString); + << transientParams.targetedClusterConnectionString); } - auto status = - _parseAndValidateCertificateFromMemory((*_transientSSLParams).sslClusterPEMPayload, - &_clusterPEMPassword, - &_sslConfiguration.clientSubjectName, - nullptr); + auto status = _parseAndValidateCertificateFromMemory(transientParams.sslClusterPEMPayload, + &_clusterPEMPassword, + &_sslConfiguration.clientSubjectName, + nullptr); if (!status.isOK()) { return status.withContext("Could not validate transient certificate"); } @@ -2361,7 +2317,7 @@ bool SSLManagerOpenSSL::_initSynchronousSSLContext(UniqueSSLContext* contextPtr, ConnectionDirection direction) { *contextPtr = UniqueSSLContext(SSL_CTX_new(SSLv23_method())); - uassertStatusOK(initSSLContext(contextPtr->get(), params, direction)); + uassertStatusOK(initSSLContext(contextPtr->get(), params, TransientSSLParams(), direction)); // If renegotiation is needed, don't return from recv() or send() until it's successful. // Note: this is for blocking sockets only. diff --git a/src/mongo/util/net/ssl_manager_test.cpp b/src/mongo/util/net/ssl_manager_test.cpp index 183c5e2cae8..3f84c03855c 100644 --- a/src/mongo/util/net/ssl_manager_test.cpp +++ b/src/mongo/util/net/ssl_manager_test.cpp @@ -516,11 +516,6 @@ TEST(SSLManager, InitContextFromFileShouldFail) { // We force the initialization to fail by omitting this param. params.sslCAFile = "jstests/libs/ca.pem"; params.sslClusterFile = "jstests/libs/client.pem"; -#if MONGO_CONFIG_SSL_PROVIDER == MONGO_CONFIG_SSL_PROVIDER_OPENSSL - ASSERT_THROWS_CODE([¶ms] { SSLManagerInterface::create(params, true /* isSSLServer */); }(), - DBException, - ErrorCodes::InvalidSSLConfiguration); -#endif } TEST(SSLManager, RotateClusterCertificatesFromFile) { @@ -557,8 +552,10 @@ TEST(SSLManager, InitContextFromFile) { SSLManagerInterface::create(params, false /* isSSLServer */); auto egress = std::make_unique<asio::ssl::context>(asio::ssl::context::sslv23); - uassertStatusOK(manager->initSSLContext( - egress->native_handle(), params, SSLManagerInterface::ConnectionDirection::kOutgoing)); + uassertStatusOK(manager->initSSLContext(egress->native_handle(), + params, + TransientSSLParams(), + SSLManagerInterface::ConnectionDirection::kOutgoing)); } TEST(SSLManager, InitContextFromMemory) { @@ -570,15 +567,16 @@ TEST(SSLManager, InitContextFromMemory) { transientParams.sslClusterPEMPayload = loadFile("jstests/libs/client.pem"); std::shared_ptr<SSLManagerInterface> manager = - SSLManagerInterface::create(params, transientParams, false /* isSSLServer */); + SSLManagerInterface::create(params, false /* isSSLServer */); auto egress = std::make_unique<asio::ssl::context>(asio::ssl::context::sslv23); - uassertStatusOK(manager->initSSLContext( - egress->native_handle(), params, SSLManagerInterface::ConnectionDirection::kOutgoing)); + uassertStatusOK(manager->initSSLContext(egress->native_handle(), + params, + transientParams, + SSLManagerInterface::ConnectionDirection::kOutgoing)); } -// Tests when 'is server' param to managed interface creation is set, it is ignored. -TEST(SSLManager, IgnoreInitServerSideContextFromMemory) { +TEST(SSLManager, InitServerSideContextFromMemory) { SSLParams params; params.sslMode.store(::mongo::sslGlobalParams.SSLMode_requireSSL); params.sslPEMKeyFile = "jstests/libs/server.pem"; @@ -588,11 +586,13 @@ TEST(SSLManager, IgnoreInitServerSideContextFromMemory) { transientParams.sslClusterPEMPayload = loadFile("jstests/libs/client.pem"); std::shared_ptr<SSLManagerInterface> manager = - SSLManagerInterface::create(params, transientParams, true /* isSSLServer */); + SSLManagerInterface::create(params, true /* isSSLServer */); auto egress = std::make_unique<asio::ssl::context>(asio::ssl::context::sslv23); - uassertStatusOK(manager->initSSLContext( - egress->native_handle(), params, SSLManagerInterface::ConnectionDirection::kOutgoing)); + uassertStatusOK(manager->initSSLContext(egress->native_handle(), + params, + transientParams, + SSLManagerInterface::ConnectionDirection::kOutgoing)); } TEST(SSLManager, TransientSSLParams) { @@ -601,6 +601,9 @@ TEST(SSLManager, TransientSSLParams) { params.sslCAFile = "jstests/libs/ca.pem"; params.sslClusterFile = "jstests/libs/client.pem"; + std::shared_ptr<SSLManagerInterface> manager = + SSLManagerInterface::create(params, false /* isSSLServer */); + ServiceEntryPointUtil sepu; auto options = [] { @@ -615,13 +618,17 @@ TEST(SSLManager, TransientSSLParams) { transientSSLParams.sslClusterPEMPayload = loadFile("jstests/libs/client.pem"); transientSSLParams.targetedClusterConnectionString = ConnectionString::forLocal(); - auto swContext = tla.createTransientSSLContext(transientSSLParams); - uassertStatusOK(swContext.getStatus()); + auto result = tla.createTransientSSLContext(transientSSLParams); + + // This will fail because we need to rotate certificates first to + // initialize the default SSL context inside TransportLayerASIO. + ASSERT_NOT_OK(result.getStatus()); + + // Init the transport properly. + uassertStatusOK(tla.rotateCertificates(manager, false /* asyncOCSPStaple */)); - // Check that the manager owned by the transient context is also transient. - ASSERT_TRUE(swContext.getValue()->manager->isTransient()); - ASSERT_EQ(transientSSLParams.targetedClusterConnectionString.toString(), - swContext.getValue()->manager->getTargetedClusterConnectionString()); + result = tla.createTransientSSLContext(transientSSLParams); + uassertStatusOK(result.getStatus()); } #endif diff --git a/src/mongo/util/net/ssl_manager_windows.cpp b/src/mongo/util/net/ssl_manager_windows.cpp index c7fad877e3f..6c912ce6d08 100644 --- a/src/mongo/util/net/ssl_manager_windows.cpp +++ b/src/mongo/util/net/ssl_manager_windows.cpp @@ -269,7 +269,8 @@ public: */ Status initSSLContext(SCHANNEL_CRED* cred, const SSLParams& params, - ConnectionDirection direction) final; + const TransientSSLParams& transientParams, + ConnectionDirection direction) override final; SSLConnectionInterface* connect(Socket* socket) final; @@ -392,10 +393,8 @@ SSLConnectionWindows::~SSLConnectionWindows() {} // Global variable indicating if this is a server or a client instance bool isSSLServer = false; -std::shared_ptr<SSLManagerInterface> SSLManagerInterface::create( - const SSLParams& params, - const std::optional<TransientSSLParams>& transientSSLParams, - bool isServer) { +std::shared_ptr<SSLManagerInterface> SSLManagerInterface::create(const SSLParams& params, + bool isServer) { return std::make_shared<SSLManagerWindows>(params, isServer); } @@ -417,7 +416,8 @@ SSLManagerWindows::SSLManagerWindows(const SSLParams& params, bool isServer) uassertStatusOK(_loadCertificates(params)); - uassertStatusOK(initSSLContext(&_clientCred, params, ConnectionDirection::kOutgoing)); + uassertStatusOK( + initSSLContext(&_clientCred, params, TransientSSLParams(), ConnectionDirection::kOutgoing)); // Certificates may not have been loaded. This typically occurs in unit tests. if (_clientCertificates[0] != nullptr) { @@ -427,7 +427,8 @@ SSLManagerWindows::SSLManagerWindows(const SSLParams& params, bool isServer) // SSL server specific initialization if (isServer) { - uassertStatusOK(initSSLContext(&_serverCred, params, ConnectionDirection::kIncoming)); + uassertStatusOK(initSSLContext( + &_serverCred, params, TransientSSLParams(), ConnectionDirection::kIncoming)); if (_serverCertificates[0] != nullptr) { SSLX509Name subjectName; @@ -1344,6 +1345,7 @@ Status SSLManagerWindows::_loadCertificates(const SSLParams& params) { Status SSLManagerWindows::initSSLContext(SCHANNEL_CRED* cred, const SSLParams& params, + const TransientSSLParams& transientParams, ConnectionDirection direction) { memset(cred, 0, sizeof(*cred)); @@ -1438,6 +1440,7 @@ SSLConnectionInterface* SSLManagerWindows::accept(Socket* socket, void SSLManagerWindows::_handshake(SSLConnectionWindows* conn, bool client) { initSSLContext(conn->_cred, getSSLGlobalParams(), + TransientSSLParams(), client ? SSLManagerInterface::ConnectionDirection::kOutgoing : SSLManagerInterface::ConnectionDirection::kIncoming); |