SERVER-31965 Correctly handle certificates for SRV URIs

The hostname provided by SRV records is a canonicalized FQDN ending
in a '.' character.  X.509 certificates use a canonical hostname
with the trailing '.' removed.  The comparison between these two
forms needs to strip all trailing '.' characters.  This is
considered safe in all cases, as a DNS spoofing attack would still
require forging or obtaining a certificate with a canonicalized name
to make a redirection work.

4 files changed, 140 insertions, 25 deletions

diff --git a/src/mongo/util/net/SConscript b/src/mongo/util/net/SConscript
index a57710a3f1c..4344e2a3a27 100644
--- a/src/mongo/util/net/SConscript
+++ b/src/mongo/util/net/SConscript
@@ -53,6 +53,18 @@ env.CppUnitTest(
     ],
 )
 
+
+env.CppUnitTest(
+    target='ssl_manager_test',
+    source=[
+        'ssl_manager_test.cpp',
+    ],
+    LIBDEPS=[
+        '$BUILD_DIR/mongo/base',
+        'network',
+    ],
+)
+
 env.CppIntegrationTest(
     target='op_msg_integration_test',
     source=[
diff --git a/src/mongo/util/net/ssl_manager.cpp b/src/mongo/util/net/ssl_manager.cpp
index 6299e1336af..919c73a4239 100644
--- a/src/mongo/util/net/ssl_manager.cpp
+++ b/src/mongo/util/net/ssl_manager.cpp
@@ -142,8 +142,10 @@ public:
         return Status::OK();
     }
 } openSSLCipherConfig;
+}  // namespace mongo
 
 #ifdef MONGO_CONFIG_SSL
+namespace mongo {
 namespace {
 
 // If the underlying SSL supports auto-configuration of ECDH parameters, this function will select
@@ -475,11 +477,6 @@ private:
      */
     void _flushNetworkBIO(SSLConnection* conn);
 
-    /*
-     * match a remote host name to an x.509 host name
-     */
-    bool _hostNameMatch(const char* nameToMatch, const char* certHostName);
-
     /**
      * Callbacks for SSL functions.
      */
@@ -1314,22 +1311,6 @@ SSLConnection* SSLManager::accept(Socket* socket, const char* initialBytes, int
     return sslConn.release();
 }
 
-// TODO SERVER-11601 Use NFC Unicode canonicalization
-bool SSLManager::_hostNameMatch(const char* nameToMatch, const char* certHostName) {
-    if (strlen(certHostName) < 2) {
-        return false;
-    }
-
-    // match wildcard DNS names
-    if (certHostName[0] == '*' && certHostName[1] == '.') {
-        // allow name.example.com if the cert is *.example.com, '*' does not match '.'
-        const char* subName = strchr(nameToMatch, '.');
-        return subName && !strcasecmp(certHostName + 1, subName);
-    } else {
-        return !strcasecmp(nameToMatch, certHostName);
-    }
-}
-
 StatusWith<boost::optional<SSLPeerInfo>> SSLManager::parseAndValidatePeerCertificate(
     SSL* conn, const std::string& remoteHost) {
     if (!_sslConfiguration.hasCA && isSSLServer)
@@ -1399,7 +1380,7 @@ StatusWith<boost::optional<SSLPeerInfo>> SSLManager::parseAndValidatePeerCertifi
             const GENERAL_NAME* currentName = sk_GENERAL_NAME_value(sanNames, i);
             if (currentName && currentName->type == GEN_DNS) {
                 char* dnsName = reinterpret_cast<char*>(ASN1_STRING_data(currentName->d.dNSName));
-                if (_hostNameMatch(remoteHost.c_str(), dnsName)) {
+                if (hostNameMatchForX509Certificates(remoteHost, dnsName)) {
                     sanMatch = true;
                     break;
                 }
@@ -1414,7 +1395,7 @@ StatusWith<boost::optional<SSLPeerInfo>> SSLManager::parseAndValidatePeerCertifi
         int cnEnd = peerSubjectName.find(",", cnBegin);
         std::string commonName = peerSubjectName.substr(cnBegin, cnEnd - cnBegin);
 
-        if (_hostNameMatch(remoteHost.c_str(), commonName.c_str())) {
+        if (hostNameMatchForX509Certificates(remoteHost, commonName)) {
             cnMatch = true;
         }
         certificateNames << "CN: " << commonName;
@@ -1619,13 +1600,44 @@ void SSLManager::_handleSSLError(int code, int ret) {
     }
     throw SocketException(SocketException::CONNECT_ERROR, "");
 }
+}  // namespace mongo
+
+// TODO SERVER-11601 Use NFC Unicode canonicalization
+bool mongo::hostNameMatchForX509Certificates(std::string nameToMatch, std::string certHostName) {
+    auto removeFQDNRoot = [](std::string name) -> std::string {
+        if (name.back() == '.') {
+            name.pop_back();
+        }
+        return name;
+    };
+
+    nameToMatch = removeFQDNRoot(std::move(nameToMatch));
+    certHostName = removeFQDNRoot(std::move(certHostName));
+
+    if (certHostName.size() < 2) {
+        return false;
+    }
+
+    // match wildcard DNS names
+    if (certHostName[0] == '*' && certHostName[1] == '.') {
+        // allow name.example.com if the cert is *.example.com, '*' does not match '.'
+        const char* subName = strchr(nameToMatch.c_str(), '.');
+        return subName && !strcasecmp(certHostName.c_str() + 1, subName);
+    } else {
+        return !strcasecmp(nameToMatch.c_str(), certHostName.c_str());
+    }
+}
+
 #else
 
+namespace mongo {
+namespace {
 MONGO_INITIALIZER(SSLManager)(InitializerContext*) {
     // we need a no-op initializer so that we can depend on SSLManager as a prerequisite in
     // non-SSL builds.
     return Status::OK();
 }
+}  // namespace
+}  // namespace mongo
 
 #endif  // #ifdef MONGO_CONFIG_SSL
-}  // namespace mongo
diff --git a/src/mongo/util/net/ssl_manager.h b/src/mongo/util/net/ssl_manager.h
index ef7ad5c403b..002a0dd6be7 100644
--- a/src/mongo/util/net/ssl_manager.h
+++ b/src/mongo/util/net/ssl_manager.h
@@ -199,5 +199,12 @@ extern bool isSSLServer;
  * "EndStartupOptionStorage" as a prerequisite.
  */
 const SSLParams& getSSLGlobalParams();
-}
+
+
+/**
+ * Returns true if the `nameToMatch` is a valid match against the `certHostName` requirement from an
+ * x.509 certificate.  Matches a remote host name to an x.509 host name, including wildcards.
+ */
+bool hostNameMatchForX509Certificates(std::string nameToMatch, std::string certHostName);
+}  // namespace mongo
 #endif  // #ifdef MONGO_CONFIG_SSL
diff --git a/src/mongo/util/net/ssl_manager_test.cpp b/src/mongo/util/net/ssl_manager_test.cpp
new file mode 100644
index 00000000000..35ca49e6d92
--- /dev/null
+++ b/src/mongo/util/net/ssl_manager_test.cpp
@@ -0,0 +1,84 @@
+/**
+ *    Copyright (C) 2017 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kNetwork
+
+#include "mongo/util/net/ssl_manager.h"
+
+#include "mongo/unittest/unittest.h"
+#include "mongo/util/log.h"
+
+#ifdef MONGO_CONFIG_SSL
+
+namespace mongo {
+namespace {
+TEST(SSLManager, matchHostname) {
+    enum Expected : bool { match = true, mismatch = false };
+    const struct {
+        Expected expected;
+        std::string hostname;
+        std::string certName;
+    } tests[] = {
+        // clang-format off
+        // Matches?  |    Hostname and possibly FQDN   |  Certificate name
+        {match,                    "foo.bar.bas" ,           "*.bar.bas."},
+        {mismatch,       "foo.subdomain.bar.bas" ,           "*.bar.bas."},
+        {match,                    "foo.bar.bas.",           "*.bar.bas."},
+        {mismatch,       "foo.subdomain.bar.bas.",           "*.bar.bas."},
+
+        {match,                    "foo.bar.bas" ,           "*.bar.bas"},
+        {mismatch,       "foo.subdomain.bar.bas" ,           "*.bar.bas"},
+        {match,                    "foo.bar.bas.",           "*.bar.bas"},
+        {mismatch,       "foo.subdomain.bar.bas.",           "*.bar.bas"},
+
+        {mismatch,                "foo.evil.bas" ,           "*.bar.bas."},
+        {mismatch,      "foo.subdomain.evil.bas" ,           "*.bar.bas."},
+        {mismatch,                "foo.evil.bas.",           "*.bar.bas."},
+        {mismatch,      "foo.subdomain.evil.bas.",           "*.bar.bas."},
+
+        {mismatch,                "foo.evil.bas" ,           "*.bar.bas"},
+        {mismatch,      "foo.subdomain.evil.bas" ,           "*.bar.bas"},
+        {mismatch,                "foo.evil.bas.",           "*.bar.bas"},
+        {mismatch,      "foo.subdomain.evil.bas.",           "*.bar.bas"},
+        // clang-format on
+    };
+    bool failure = false;
+    for (const auto& test : tests) {
+        if (test.expected != hostNameMatchForX509Certificates(test.hostname, test.certName)) {
+            failure = true;
+            LOG(1) << "Failure for Hostname: " << test.hostname
+                   << " Certificate: " << test.certName;
+        } else {
+            LOG(1) << "Passed for Hostname: " << test.hostname << " Certificate: " << test.certName;
+        }
+    }
+    ASSERT_FALSE(failure);
+}
+}  // namespace
+}  // namespace mongo
+#endif