SERVER-46633 Windows TLS implementation may declare hostname mismatch on unrelated error

(cherry picked from commit 06f26fac35c7a3b82effbb9815ad9f7aedaf4dfb)

2 files changed, 49 insertions, 12 deletions

diff --git a/jstests/ssl/mongo_uri_secondaries.js b/jstests/ssl/mongo_uri_secondaries.js
index eec34153b30..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 522004e6c82..aac7044ed87 100644
--- a/src/mongo/util/net/ssl_manager_windows.cpp
+++ b/src/mongo/util/net/ssl_manager_windows.cpp
@@ -39,6 +39,7 @@
 #include <string>
 #include <tuple>
 #include <vector>
+#include <winhttp.h>
 
 #include "mongo/base/init.h"
 #include "mongo/base/initializer_context.h"
@@ -311,6 +312,7 @@ private:
         CERT_CHAIN_ENGINE_CONFIG userConfig;
         UniqueCertChainEngine user;
         UniqueCertStore CAstore;
+        bool hasCRL;
     };
 
     Status _initChainEngines(CAEngine* engine);
@@ -1285,6 +1287,7 @@ Status SSLManagerWindows::_loadCertificates(const SSLParams& params) {
         }
 
         _clientEngine.CAstore = std::move(swChain.getValue());
+        _clientEngine.hasCRL = !params.sslCRLFile.empty();
     }
 
     const auto serverCAFile =
@@ -1296,6 +1299,7 @@ Status SSLManagerWindows::_loadCertificates(const SSLParams& params) {
         }
 
         _serverEngine.CAstore = std::move(swChain.getValue());
+        _serverEngine.hasCRL = !params.sslCRLFile.empty();
     }
 
     if (hasCertificateSelector(params.sslCertificateSelector)) {
@@ -1675,6 +1679,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));
@@ -1757,11 +1762,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();
@@ -1820,7 +1855,9 @@ Status validatePeerCertificate(const std::string& remoteHost,
             return Status(ErrorCodes::SSLHandshakeFailed, msg);
         }
     }
+
     uassertStatusOK(peerSubjectName->normalizeStrings());
+
     return Status::OK();
 }
 
@@ -1900,6 +1937,7 @@ StatusWith<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
@@ -1909,6 +1947,7 @@ StatusWith<SSLPeerInfo> SSLManagerWindows::parseAndValidatePeerCertificate(
                                                           engine->user,
                                                           _allowInvalidCertificates,
                                                           _allowInvalidHostnames,
+                                                          engine->hasCRL,
                                                           &peerSubjectName);
         if (!validateCertUser.isOK()) {
             // Return the local machine status