SERVER-34888 Do not store subject name without validation

4 files changed, 42 insertions, 9 deletions

diff --git a/src/mongo/db/commands/authentication_commands.cpp b/src/mongo/db/commands/authentication_commands.cpp
index 8c6f414661c..ffe3fff78ee 100644
--- a/src/mongo/db/commands/authentication_commands.cpp
+++ b/src/mongo/db/commands/authentication_commands.cpp
@@ -81,6 +81,9 @@ Status _authenticateX509(OperationContext* opCtx, const UserName& user, const BS
     Client* client = Client::getCurrent();
     AuthorizationSession* authorizationSession = AuthorizationSession::get(client);
     auto clientName = SSLPeerInfo::forSession(client->session()).subjectName;
+    uassert(ErrorCodes::AuthenticationFailed,
+            "No verified subject name available from client",
+            !clientName.empty());
 
     if (!getSSLManager()->getSSLConfiguration().hasCA) {
         return Status(ErrorCodes::AuthenticationFailed,
@@ -235,11 +238,13 @@ bool CmdAuthenticate::run(OperationContext* opCtx,
     }
     UserName user;
     auto& sslPeerInfo = SSLPeerInfo::forSession(opCtx->getClient()->session());
+
     if (mechanism == kX509AuthMechanism && !cmdObj.hasField("user")) {
         user = UserName(sslPeerInfo.subjectName, dbname);
     } else {
         user = UserName(cmdObj.getStringField("user"), dbname);
     }
+    uassert(ErrorCodes::AuthenticationFailed, "No user name provided", !user.getUser().empty());
 
     if (getTestCommandsEnabled() && user.getDB() == "admin" &&
         user.getUser() == internalSecurity.user->getName().getUser()) {
diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp
index 1742e7b5197..5c6be867de3 100644
--- a/src/mongo/db/db.cpp
+++ b/src/mongo/db/db.cpp
@@ -49,6 +49,7 @@
 #include "mongo/db/audit.h"
 #include "mongo/db/auth/authorization_manager.h"
 #include "mongo/db/auth/authorization_manager_global.h"
+#include "mongo/db/auth/sasl_options.h"
 #include "mongo/db/catalog/collection.h"
 #include "mongo/db/catalog/create_collection.h"
 #include "mongo/db/catalog/database.h"
@@ -167,6 +168,7 @@
 #include "mongo/util/quick_exit.h"
 #include "mongo/util/ramlog.h"
 #include "mongo/util/scopeguard.h"
+#include "mongo/util/sequence_util.h"
 #include "mongo/util/signal_handlers.h"
 #include "mongo/util/stacktrace.h"
 #include "mongo/util/startup_test.h"
@@ -174,6 +176,10 @@
 #include "mongo/util/time_support.h"
 #include "mongo/util/version.h"
 
+#ifdef MONGO_CONFIG_SSL
+#include "mongo/util/net/ssl_options.h"
+#endif
+
 #if !defined(_WIN32)
 #include <sys/file.h>
 #endif
@@ -367,6 +373,18 @@ ExitCode _initAndListen(int listenPort) {
 
     logMongodStartupWarnings(storageGlobalParams, serverGlobalParams, serviceContext);
 
+#if MONGO_CONFIG_SSL
+    if (sslGlobalParams.sslAllowInvalidCertificates &&
+        ((serverGlobalParams.clusterAuthMode.load() == ServerGlobalParams::ClusterAuthMode_x509) ||
+         sequenceContains(saslGlobalParams.authenticationMechanisms, "MONGODB-X509"))) {
+        log() << "** WARNING: While invalid X509 certificates may be used to" << startupWarningsLog;
+        log() << "**          connect to this server, they will not be considered"
+              << startupWarningsLog;
+        log() << "**          permissible for authentication." << startupWarningsLog;
+        log() << startupWarningsLog;
+    }
+#endif
+
     {
         std::stringstream ss;
         ss << endl;
diff --git a/src/mongo/util/net/ssl_manager_openssl.cpp b/src/mongo/util/net/ssl_manager_openssl.cpp
index 0d9c7acc14e..20abd7b6421 100644
--- a/src/mongo/util/net/ssl_manager_openssl.cpp
+++ b/src/mongo/util/net/ssl_manager_openssl.cpp
@@ -1235,6 +1235,7 @@ StatusWith<boost::optional<SSLPeerInfo>> SSLManagerOpenSSL::parseAndValidatePeer
         if (_allowInvalidCertificates) {
             warning() << "SSL peer certificate validation failed: "
                       << X509_verify_cert_error_string(result);
+            return {boost::none};
         } else {
             str::stream msg;
             msg << "SSL peer certificate validation failed: "
diff --git a/src/mongo/util/net/ssl_manager_windows.cpp b/src/mongo/util/net/ssl_manager_windows.cpp
index 0722f59dfd5..14ed8c9e68f 100644
--- a/src/mongo/util/net/ssl_manager_windows.cpp
+++ b/src/mongo/util/net/ssl_manager_windows.cpp
@@ -1511,7 +1511,8 @@ Status validatePeerCertificate(const std::string& remoteHost,
                                PCCERT_CONTEXT cert,
                                HCERTCHAINENGINE certChainEngine,
                                bool allowInvalidCertificates,
-                               bool allowInvalidHostnames) {
+                               bool allowInvalidHostnames,
+                               std::string* peerSubjectName) {
     CERT_CHAIN_PARA certChainPara;
     memset(&certChainPara, 0, sizeof(certChainPara));
     certChainPara.cbSize = sizeof(CERT_CHAIN_PARA);
@@ -1584,6 +1585,13 @@ Status validatePeerCertificate(const std::string& remoteHost,
                                     << errnoWithDescription(gle));
     }
 
+    auto swSubjectName = getCertificateSubjectName(cert);
+    if (!swSubjectName.isOK()) {
+        return swSubjectName.getStatus();
+    }
+    invariant(peerSubjectName);
+    *peerSubjectName = swSubjectName.getValue();
+
     // This means the certificate chain is not valid.
     // Ignore CRYPT_E_NO_REVOCATION_CHECK since most CAs lack revocation information especially test
     // certificates
@@ -1600,7 +1608,7 @@ Status validatePeerCertificate(const std::string& remoteHost,
                 }
             };
 
-            certificateNames << ", Subject Name: " << getCertificateSubjectName(cert);
+            certificateNames << ", Subject Name: " << *peerSubjectName;
 
             str::stream msg;
             msg << "The server certificate does not match the host name. Hostname: " << remoteHost
@@ -1611,6 +1619,7 @@ Status validatePeerCertificate(const std::string& remoteHost,
                           << integerToHex(certChainPolicyStatus.dwError)
                           << "): " << errnoWithDescription(certChainPolicyStatus.dwError);
                 warning() << msg.ss.str();
+                *peerSubjectName = "";
                 return Status::OK();
             } else if (allowInvalidHostnames) {
                 warning() << msg.ss.str();
@@ -1658,13 +1667,15 @@ StatusWith<boost::optional<SSLPeerInfo>> SSLManagerWindows::parseAndValidatePeer
     }
 
     UniqueCertificate certHolder(cert);
+    std::string peerSubjectName;
 
     // Validate against the local machine store first since it is easier to manage programmatically.
     Status validateCertMachine = validatePeerCertificate(remoteHost,
                                                          certHolder.get(),
                                                          _chainEngineMachine,
                                                          _allowInvalidCertificates,
-                                                         _allowInvalidHostnames);
+                                                         _allowInvalidHostnames,
+                                                         &peerSubjectName);
     if (!validateCertMachine.isOK()) {
         // Validate against the current user store since this is easier for unprivileged users to
         // manage.
@@ -1672,20 +1683,18 @@ StatusWith<boost::optional<SSLPeerInfo>> SSLManagerWindows::parseAndValidatePeer
                                                           certHolder.get(),
                                                           _chainEngineUser,
                                                           _allowInvalidCertificates,
-                                                          _allowInvalidHostnames);
+                                                          _allowInvalidHostnames,
+                                                          &peerSubjectName);
         if (!validateCertUser.isOK()) {
             // Return the local machine status
             return validateCertMachine;
         }
     }
 
-    auto swCert = getCertificateSubjectName(cert);
-
-    if (!swCert.isOK()) {
-        return swCert.getStatus();
+    if (peerSubjectName.empty()) {
+        return {boost::none};
     }
 
-    std::string peerSubjectName = swCert.getValue();
     LOG(2) << "Accepted TLS connection from peer: " << peerSubjectName;
 
     // On the server side, parse the certificate for roles