diff options
| -rw-r--r-- | jstests/ssl/mongo_uri_secondaries.js | 16 | ||||
| -rw-r--r-- | src/mongo/util/net/ssl_manager_windows.cpp | 60 |
2 files changed, 57 insertions, 19 deletions
diff --git a/jstests/ssl/mongo_uri_secondaries.js b/jstests/ssl/mongo_uri_secondaries.js index 6970015cc12..a4ed1eae93c 100644 --- a/jstests/ssl/mongo_uri_secondaries.js +++ b/jstests/ssl/mongo_uri_secondaries.js @@ -61,16 +61,14 @@ const subShellCommandFormatter = function(replSet) { return command; }; -const subShellArgs = [ - "env", - "SSL_CERT_FILE=jstests/libs/trusted-ca.pem", - 'mongo', - '--nodb', - '--eval', - subShellCommandFormatter(rst) -]; +function runWithEnv(args, env) { + const pid = _startMongoProgram({args: args, env: env}); + return waitProgram(pid); +} + +const subShellArgs = ['mongo', '--nodb', '--eval', subShellCommandFormatter(rst)]; -const retVal = _runMongoProgram(...subShellArgs); +const retVal = runWithEnv(subShellArgs, {"SSL_CERT_FILE": "jstests/libs/trusted-ca.pem"}); assert.eq(retVal, 0, 'mongo shell did not succeed with exit code 0'); rst.stopSet(); diff --git a/src/mongo/util/net/ssl_manager_windows.cpp b/src/mongo/util/net/ssl_manager_windows.cpp index ccae93b440f..ec50095fa05 100644 --- a/src/mongo/util/net/ssl_manager_windows.cpp +++ b/src/mongo/util/net/ssl_manager_windows.cpp @@ -40,6 +40,7 @@ #include <string> #include <tuple> #include <vector> +#include <winhttp.h> #include "mongo/base/init.h" #include "mongo/base/initializer_context.h" @@ -310,6 +311,7 @@ private: CERT_CHAIN_ENGINE_CONFIG userConfig; UniqueCertChainEngine user; UniqueCertStore CAstore; + bool hasCRL; }; Status _initChainEngines(CAEngine* engine); @@ -1288,6 +1290,7 @@ Status SSLManagerWindows::_loadCertificates(const SSLParams& params) { } _clientEngine.CAstore = std::move(swChain.getValue()); + _clientEngine.hasCRL = !params.sslCRLFile.empty(); } const auto serverCAFile = @@ -1299,6 +1302,7 @@ Status SSLManagerWindows::_loadCertificates(const SSLParams& params) { } _serverEngine.CAstore = std::move(swChain.getValue()); + _serverEngine.hasCRL = !params.sslCRLFile.empty(); } if (hasCertificateSelector(params.sslCertificateSelector)) { @@ -1684,6 +1688,7 @@ Status validatePeerCertificate(const std::string& remoteHost, HCERTCHAINENGINE certChainEngine, bool allowInvalidCertificates, bool allowInvalidHostnames, + bool hasCRL, SSLX509Name* peerSubjectName) { CERT_CHAIN_PARA certChainPara; memset(&certChainPara, 0, sizeof(certChainPara)); @@ -1780,11 +1785,41 @@ Status validatePeerCertificate(const std::string& remoteHost, // This means the certificate chain is not valid. // Ignore CRYPT_E_NO_REVOCATION_CHECK since most CAs lack revocation information especially test - // certificates + // certificates. Either there needs to be a CRL, a CrlDistributionPoint in the Cert or OCSP and + // user-generated certs lack this information. if (certChainPolicyStatus.dwError != S_OK && certChainPolicyStatus.dwError != CRYPT_E_NO_REVOCATION_CHECK) { - auto swAltNames = getSubjectAlternativeNames(cert); - if (certChainPolicyStatus.dwError == CERT_E_CN_NO_MATCH || allowInvalidCertificates) { + bool onlyCNError = false; + + // Try again to validate if the cert has any other errors besides a CN mismatch + if (certChainPolicyStatus.dwError == CERT_E_CN_NO_MATCH && !allowInvalidCertificates) { + + // We know the CNs do not match, are there any other issues? + sslCertChainPolicy.fdwChecks = SECURITY_FLAG_IGNORE_CERT_CN_INVALID; + + ret = CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, + certChainHolder.get(), + &chain_policy_para, + &certChainPolicyStatus); + + // This means something really went wrong, this should not happen. + if (!ret) { + DWORD gle = GetLastError(); + return Status(ErrorCodes::InvalidSSLConfiguration, + str::stream() << "CertVerifyCertificateChainPolicy2 failed: " + << errnoWithDescription(gle)); + } + + if (certChainPolicyStatus.dwError == S_OK || + certChainPolicyStatus.dwError == CRYPT_E_NO_REVOCATION_CHECK) { + onlyCNError = true; + } + } + + // We need to check if the user has a cert where SANs have ip addresses label as DNS Name + // but only if a CN mismatch is the only error + if (onlyCNError || allowInvalidCertificates) { + auto swAltNames = getSubjectAlternativeNames(cert); auto swCIDRRemoteHost = CIDR::parse(remoteHost); if (swAltNames.isOK() && swCIDRRemoteHost.isOK()) { auto remoteHostCIDR = swCIDRRemoteHost.getValue(); @@ -1821,7 +1856,6 @@ Status validatePeerCertificate(const std::string& remoteHost, msg << "The server certificate does not match the host name. Hostname: " << remoteHost << " does not match " << certificateNames.str(); - if (allowInvalidCertificates) { LOGV2_WARNING(23274, "SSL peer certificate validation failed ({errorCode}): {error}", @@ -1829,12 +1863,14 @@ Status validatePeerCertificate(const std::string& remoteHost, "errorCode"_attr = integerToHex(certChainPolicyStatus.dwError), "error"_attr = errnoWithDescription(certChainPolicyStatus.dwError)); - LOGV2_WARNING(23275, - "The server certificate does not match the host name. Hostname: " - "{remoteHost} does not match {certificateNames}", - "The server certificate does not match the host name", - "remoteHost"_attr = remoteHost, - "certificateNames"_attr = certificateNames.str()); + if (certChainPolicyStatus.dwError == CERT_E_CN_NO_MATCH) { + LOGV2_WARNING(23275, + "The server certificate does not match the host name. Hostname: " + "{remoteHost} does not match {certificateNames}", + "The server certificate does not match the host name", + "remoteHost"_attr = remoteHost, + "certificateNames"_attr = certificateNames.str()); + } *peerSubjectName = SSLX509Name(); return Status::OK(); @@ -1864,7 +1900,9 @@ Status validatePeerCertificate(const std::string& remoteHost, return Status(ErrorCodes::SSLHandshakeFailed, msg); } } + uassertStatusOK(peerSubjectName->normalizeStrings()); + return Status::OK(); } @@ -1955,6 +1993,7 @@ Future<SSLPeerInfo> SSLManagerWindows::parseAndValidatePeerCertificate( engine->machine, _allowInvalidCertificates, _allowInvalidHostnames, + engine->hasCRL, &peerSubjectName); if (!validateCertMachine.isOK()) { // Validate against the current user store since this is easier for unprivileged users to @@ -1964,6 +2003,7 @@ Future<SSLPeerInfo> SSLManagerWindows::parseAndValidatePeerCertificate( engine->user, _allowInvalidCertificates, _allowInvalidHostnames, + engine->hasCRL, &peerSubjectName); if (!validateCertUser.isOK()) { // Return the local machine status |