SERVER-55119 startup warning when X.509 certificates have no Subject Alternative Name

6 files changed, 195 insertions, 3 deletions

diff --git a/jstests/libs/server_no_SAN.pem b/jstests/libs/server_no_SAN.pem
new file mode 100644
index 00000000000..5b5e015d5a0
--- /dev/null
+++ b/jstests/libs/server_no_SAN.pem
@@ -0,0 +1,53 @@
+# Autogenerated file, do not edit.
+# Generate using jstests/ssl/x509/mkcert.py --config jstests/ssl/x509/certs.yml server_no_SAN.pem
+#
+# General purpose server certificate with missing SAN.
+-----BEGIN CERTIFICATE-----
+MIIDgjCCAmqgAwIBAgIEW7kt/TANBgkqhkiG9w0BAQsFADB0MQswCQYDVQQGEwJV
+UzERMA8GA1UECAwITmV3IFlvcmsxFjAUBgNVBAcMDU5ldyBZb3JrIENpdHkxEDAO
+BgNVBAoMB01vbmdvREIxDzANBgNVBAsMBktlcm5lbDEXMBUGA1UEAwwOS2VybmVs
+IFRlc3QgQ0EwHhcNMjEwMzE3MTgyODQxWhcNNDEwMzE5MTgyODQxWjCBkTELMAkG
+A1UEBhMCVVMxETAPBgNVBAgMCE5ldyBZb3JrMRYwFAYDVQQHDA1OZXcgWW9yayBD
+aXR5MRAwDgYDVQQKDAdNb25nb0RCMQ8wDQYDVQQLDAZLZXJuZWwxEjAQBgNVBAMM
+CWxvY2FsaG9zdDEgMB4GA1UEDAwXU2VydmVyIG5vIFNBTiBhdHRyaWJ1dGUwggEi
+MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDPO1Jhwl0hXBSdtpJjwhRmCkn1
+1hag6IGLprTEUPxBgrIFwwHhk5b7NAZV4TeMa3tSxs1OvtgW+X1utid5dE7VQEwt
+2C0q1rL0HIyka+RhX/gyzWKJHsXl1V+e5nnTwqDImx20rscDeohKOXA71npCcoOA
+21cEBtJUh7DnI50i0e7pgjup0Y63WVBEz3qf+en4Wdu46kYqWmW6HZOIGppKHnMo
+PIeZjf9cTUlR6jnGKy6p95I9y3pz1eQ0yrU/Tkw3V5J9BE+t+06Umft9O7KEkspj
+0EM6GeVQa7j4tbhsxKZSXE+pmidg/fOcPk3hQQq6mncu420nIDTDZjfzaoz3AgMB
+AAEwDQYJKoZIhvcNAQELBQADggEBAGP0NGcs/jSWO4i17rlTNDeswmA0siiArg52
+1A8VV0elSdf7eOIdUjynKYVDuiWfXj+pkOwDwZs8m9rn8xvqPiMSyQrEGlNXDtev
+fK4088jQ8gJykwEXcaIhcDmZMJ3t04qta1dIYPRlZrVaRspdJ/CM6crjDImVNmUs
+PIIcoEVtdQ5UpWs5ICz9YZ2IaPofpOC2O9AZOFyMeofviSnV8rASUnM5O0ubsHMi
+k9wKa4rJ4mnCrVZeJLf+9mKxTG+82Cummj+rOZMDERlkHqXR5uH7B0DQYoHTrSkk
+ECb0O7VYjEKdV3Y4UThqgTsO84iK/lgn30zjBU+V4o4cLXxhBS4=
+-----END CERTIFICATE-----
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDPO1Jhwl0hXBSd
+tpJjwhRmCkn11hag6IGLprTEUPxBgrIFwwHhk5b7NAZV4TeMa3tSxs1OvtgW+X1u
+tid5dE7VQEwt2C0q1rL0HIyka+RhX/gyzWKJHsXl1V+e5nnTwqDImx20rscDeohK
+OXA71npCcoOA21cEBtJUh7DnI50i0e7pgjup0Y63WVBEz3qf+en4Wdu46kYqWmW6
+HZOIGppKHnMoPIeZjf9cTUlR6jnGKy6p95I9y3pz1eQ0yrU/Tkw3V5J9BE+t+06U
+mft9O7KEkspj0EM6GeVQa7j4tbhsxKZSXE+pmidg/fOcPk3hQQq6mncu420nIDTD
+Zjfzaoz3AgMBAAECggEBAMxgWv0i7SpLX+Gy/2j3LZr9JrgXLjX/WFPcU4cRv9b0
+CJJ6Ik7QeiTAyEbGWTxZfETE4BJ7US5HXBdl+kRkGqNiSD8mZlVLbS4nQeWeqpwG
+RAgGWtmUyePDrgxOjXP1DRELOh7KCGg73lIll7TL78O8oEjjCUxlVeYb9LHgg8aj
+skAWt9OE8/c/P5ySATiPvtl7FpkSWkV4FMElnW1ORJmWQXsub3+HeTHLjEynXyrZ
+q3m47QlnaWbSAFA/UDHG5pLHE1CbmNC7/TaseZuCS4HeUSr/fwh+GJNIwqte5CyR
+pr3qUKhZ/5gQcNqRcMZNFngWMwWGTKehEP3cGmro3AECgYEA6GniBoKPMMqDGky4
+XAQ7QAnU5/b0L2uM0yhX+Hlf/Zels+xJlXzMMTVIgQ2ZjwQW9I/hIW5at7ohXpua
+qthvtvAxrjDZ+2ovlUS58NteapvbWCWr50b9I2R6Vrm1I7GLsBrt/LOhJvzgaLT/
+W2XLEUoRa+wWmT79JJWdeuiA2QECgYEA5EM2YFk3nItFOkVuAPKr+q/UIKX4g/PD
+1AMgozqZfhjMbwfrtfDH4tqxG1jXUV7n9c4OJNFbD3XYA4Sb6AShWSuvev1wBmkm
+iVYsi+ljY2l1O+llGAG7IuQ63zk4pr89SiedisHm4JX9PCbCcHGJJDQJNby9Jn0u
+fdMwcCT0LfcCgYBlZUxm6q7t6mwoHTCRdIck+SUZznPZ/GID/aXjkZB/Ypm4VW4E
++d1b2pM3Ome0LWSWbe8aVrrdTSchz2E7CBI1DbWe+VEgjsMTrFgy7IHUoQqg+k51
+KFNoDX4SOBL+74ax3g3WIcg86jY9eDmv9kkR0e6n1uhFE2X9gAikhqswAQKBgQDT
+upyXpmn1JSIjuP8elfp8X9gOKKVqEBSXdgcyIUr7Mhl+7APyEdP3Uw9w5GllKvlS
+gb2Q3TjwEEk8iibrgk//nIv7M1ZUO/jo7ywG44ezUMDTv9xr9j8VUEpjgHpSAZXi
+UPjLGq0DqVzqDLHTBx1EnZflZpq1NuyG/fwyKbTtZQKBgCeXe6R//lRXbHSr+MNH
+TOAry6B8c9gUfFtR+Zywb+2J+FD5LHPubXa7Tz58uV/9osJ8/XMBwZri5R8CTuKl
+4ulQLn+4dOcU2MCSsAJ57myFz2kLuITWFEk+/6FdDWSGnFZv3Dkna47gaVVoyfou
+gJiCq/jR8PaP/aQMto7a9cNC
+-----END PRIVATE KEY-----
diff --git a/jstests/ssl/x509/certs.yml b/jstests/ssl/x509/certs.yml
index b567c228939..2cee7f9e43f 100644
--- a/jstests/ssl/x509/certs.yml
+++ b/jstests/ssl/x509/certs.yml
@@ -299,6 +299,10 @@ certs:
     subjectAltName:
       DNS: ['localhost', '127.0.0.1', '::1']
 
+- name: 'server_no_SAN.pem'
+  description: General purpose server certificate with missing SAN.
+  Subject: {CN: localhost, title: 'Server no SAN attribute'}
+
 ###
 # Certificates not based on the primary root ca.pem
 ###
diff --git a/src/mongo/util/net/ssl_manager_apple.cpp b/src/mongo/util/net/ssl_manager_apple.cpp
index 71566aee663..1c02c3cd80d 100644
--- a/src/mongo/util/net/ssl_manager_apple.cpp
+++ b/src/mongo/util/net/ssl_manager_apple.cpp
@@ -541,6 +541,29 @@ StatusWith<std::vector<std::string>> extractSubjectAlternateNames(::CFDictionary
     return ret;
 }
 
+StatusWith<::SecCertificateRef> getCertificate(::CFArrayRef certs);
+
+StatusWith<std::vector<std::string>> extractSubjectAlternateNames(::CFArrayRef certs) {
+    auto swCert = getCertificate(certs);
+    if (!swCert.isOK()) {
+        return swCert.getStatus();
+    }
+    CFUniquePtr<::SecCertificateRef> cert(swCert.getValue());
+
+    CFUniquePtr<::CFMutableArrayRef> oids(
+        ::CFArrayCreateMutable(nullptr, 1, &::kCFTypeArrayCallBacks));
+    ::CFArrayAppendValue(oids.get(), ::kSecOIDSubjectAltName);
+
+    ::CFErrorRef err = nullptr;
+    CFUniquePtr<::CFDictionaryRef> cfdict(::SecCertificateCopyValues(cert.get(), oids.get(), &err));
+    CFUniquePtr<::CFErrorRef> cferror(err);
+    if (cferror) {
+        return Status(ErrorCodes::NoSuchKey, "Could not find Subject Alternative Name");
+    }
+
+    return extractSubjectAlternateNames(cfdict.get());
+}
+
 bool isCFDataEqual(::CFDataRef a, ::CFDataRef b) {
     const auto len = ::CFDataGetLength(a);
     if (::CFDataGetLength(b) != len) {
@@ -818,6 +841,27 @@ StatusWith<CFUniquePtr<::CFArrayRef>> loadPEM(const std::string& keyfilepath,
     return std::move(cfcerts);
 }
 
+// Get the root certificate from an array of certs.
+StatusWith<::SecCertificateRef> getCertificate(::CFArrayRef certs) {
+    if (::CFArrayGetCount(certs) <= 0) {
+        return {ErrorCodes::InvalidSSLConfiguration, "No certificates in certificate list"};
+    }
+
+    auto root = ::CFArrayGetValueAtIndex(certs, 0);
+    if (!root || (::CFGetTypeID(root) != ::SecIdentityGetTypeID())) {
+        return {ErrorCodes::InvalidSSLConfiguration, "Root certificate not an identity pair"};
+    }
+
+    ::SecCertificateRef idcert = nullptr;
+    auto status = ::SecIdentityCopyCertificate(cf_cast<::SecIdentityRef>(root), &idcert);
+    if (status != ::errSecSuccess) {
+        return {ErrorCodes::InvalidSSLConfiguration,
+                str::stream() << "Unable to get certificate from identity: "
+                              << stringFromOSStatus(status)};
+    }
+    return idcert;
+}
+
 StatusWith<SSLX509Name> certificateGetSubject(::SecCertificateRef cert, Date_t* expire = nullptr) {
     // Fetch expiry range and full subject name.
     CFUniquePtr<::CFMutableArrayRef> oids(
@@ -1293,8 +1337,16 @@ SSLManagerApple::SSLManagerApple(const SSLParams& params, bool isServer)
             uassertStatusOK(
                 _sslConfiguration.setServerSubjectName(uassertStatusOK(certificateGetSubject(
                     _serverCtx.certs.get(), &_sslConfiguration.serverCertificateExpirationDate))));
-            static auto task =
-                CertificateExpirationMonitor(_sslConfiguration.serverCertificateExpirationDate);
+
+            auto swSans = extractSubjectAlternateNames(_serverCtx.certs.get());
+            const bool hasSan = swSans.isOK() && (0 != swSans.getValue().size());
+            if (!hasSan) {
+                LOGV2_WARNING_OPTIONS(
+                    551191,
+                    {logv2::LogTag::kStartupWarnings},
+                    "Server certificate has no compatible Subject Alternative Name. "
+                    "This may prevent TLS clients from connecting");
+            }
         }
     }
 
diff --git a/src/mongo/util/net/ssl_manager_openssl.cpp b/src/mongo/util/net/ssl_manager_openssl.cpp
index 9e09884315b..cc0a75229fb 100644
--- a/src/mongo/util/net/ssl_manager_openssl.cpp
+++ b/src/mongo/util/net/ssl_manager_openssl.cpp
@@ -1223,6 +1223,7 @@ private:
      * and extract server certificate notAfter date.
      * @param keyFile referencing the PEM file to be read.
      * @param subjectName as a pointer to the subject name variable being set.
+     * @param verifyHasSubjectAlternativeName to generate warning if SAN is absent.
      * @param serverNotAfter a Date_t object pointer that is valued if the
      * date is to be checked (as for a server certificate) and null otherwise.
      * @return bool showing if the function was successful.
@@ -1230,6 +1231,7 @@ private:
     bool _parseAndValidateCertificate(const std::string& keyFile,
                                       PasswordFetcher* keyPassword,
                                       SSLX509Name* subjectName,
+                                      bool verifyHasSubjectAlternativeName,
                                       Date_t* serverNotAfter);
 
 
@@ -1473,7 +1475,7 @@ SSLManagerOpenSSL::SSLManagerOpenSSL(const SSLParams& params, bool isServer)
 
     if (!clientPEM.empty()) {
         if (!_parseAndValidateCertificate(
-                clientPEM, clientPassword, &_sslConfiguration.clientSubjectName, nullptr)) {
+                clientPEM, clientPassword, &_sslConfiguration.clientSubjectName, false, nullptr)) {
             uasserted(16941, "ssl initialization problem");
         }
     }
@@ -1487,6 +1489,7 @@ SSLManagerOpenSSL::SSLManagerOpenSSL(const SSLParams& params, bool isServer)
         if (!_parseAndValidateCertificate(params.sslPEMKeyFile,
                                           &_serverPEMPassword,
                                           &serverSubjectName,
+                                          true,
                                           &_sslConfiguration.serverCertificateExpirationDate)) {
             uasserted(16942, "ssl initialization problem");
         }
@@ -2219,6 +2222,7 @@ bool SSLManagerOpenSSL::_initSynchronousSSLContext(UniqueSSLContext* contextPtr,
 bool SSLManagerOpenSSL::_parseAndValidateCertificate(const std::string& keyFile,
                                                      PasswordFetcher* keyPassword,
                                                      SSLX509Name* subjectName,
+                                                     bool verifyHasSubjectAlternativeName,
                                                      Date_t* serverCertificateExpirationDate) {
     BIO* inBIO = BIO_new(BIO_s_file());
     if (inBIO == nullptr) {
@@ -2252,6 +2256,25 @@ bool SSLManagerOpenSSL::_parseAndValidateCertificate(const std::string& keyFile,
     ON_BLOCK_EXIT([&] { X509_free(x509); });
 
     *subjectName = getCertificateSubjectX509Name(x509);
+
+    if (verifyHasSubjectAlternativeName) {
+        bool hasSan = false;
+        STACK_OF(GENERAL_NAME)* sanNames = static_cast<STACK_OF(GENERAL_NAME)*>(
+            X509_get_ext_d2i(x509, NID_subject_alt_name, nullptr, nullptr));
+        if (nullptr != sanNames) {
+            int sanNamesCount = sk_GENERAL_NAME_num(sanNames);
+            hasSan = (0 != sanNamesCount);
+            sk_GENERAL_NAME_pop_free(sanNames, GENERAL_NAME_free);
+        }
+
+        if (!hasSan) {
+            LOGV2_WARNING_OPTIONS(551190,
+                                  {logv2::LogTag::kStartupWarnings},
+                                  "Server certificate has no compatible Subject Alternative Name. "
+                                  "This may prevent TLS clients from connecting");
+        }
+    }
+
     if (serverCertificateExpirationDate != nullptr) {
         auto notBeforeMillis = convertASN1ToMillis(X509_get_notBefore(x509));
         if (notBeforeMillis == Date_t()) {
diff --git a/src/mongo/util/net/ssl_manager_test.cpp b/src/mongo/util/net/ssl_manager_test.cpp
index 781707b14a9..3e88d5e2e12 100644
--- a/src/mongo/util/net/ssl_manager_test.cpp
+++ b/src/mongo/util/net/ssl_manager_test.cpp
@@ -36,6 +36,8 @@
 #include "mongo/config.h"
 #include "mongo/logv2/log.h"
 #include "mongo/unittest/unittest.h"
+#include "mongo/util/net/ssl/context.hpp"
+#include "mongo/util/net/ssl_options.h"
 
 #if MONGO_CONFIG_SSL_PROVIDER == MONGO_CONFIG_SSL_PROVIDER_OPENSSL
 #include "mongo/util/net/dh_openssl.h"
@@ -415,5 +417,51 @@ TEST(SSLManager, BadDNParsing) {
     }
 }
 
+static bool isSanWarningWritten(const std::vector<std::string>& logLines) {
+    for (const auto& line : logLines) {
+        if (std::string::npos !=
+            line.find("Server certificate has no compatible Subject Alternative Name")) {
+            return true;
+        }
+    }
+    return false;
+}
+
+// This test verifies there is a startup warning if Subject Alternative Name is missing
+TEST(SSLManager, InitContextSanWarning) {
+    SSLParams params;
+    params.sslMode.store(::mongo::sslGlobalParams.SSLMode_requireSSL);
+    params.sslCAFile = "jstests/libs/ca.pem";
+    params.sslPEMKeyFile = "jstests/libs/server_no_SAN.pem";
+
+    startCapturingLogMessages();
+    auto manager = SSLManagerInterface::create(params, true);
+    auto egress = std::make_unique<asio::ssl::context>(asio::ssl::context::sslv23);
+
+    uassertStatusOK(manager->initSSLContext(
+        egress->native_handle(), params, SSLManagerInterface::ConnectionDirection::kIncoming));
+    stopCapturingLogMessages();
+
+    ASSERT_TRUE(isSanWarningWritten(getCapturedTextFormatLogMessages()));
+}
+
+// This test verifies there is no startup warning if Subject Alternative Name is present
+TEST(SSLManager, InitContextNoSanWarning) {
+    SSLParams params;
+    params.sslMode.store(::mongo::sslGlobalParams.SSLMode_requireSSL);
+    params.sslCAFile = "jstests/libs/ca.pem";
+    params.sslPEMKeyFile = "jstests/libs/server.pem";
+
+    startCapturingLogMessages();
+    auto manager = SSLManagerInterface::create(params, true);
+    auto egress = std::make_unique<asio::ssl::context>(asio::ssl::context::sslv23);
+
+    uassertStatusOK(manager->initSSLContext(
+        egress->native_handle(), params, SSLManagerInterface::ConnectionDirection::kIncoming));
+    stopCapturingLogMessages();
+
+    ASSERT_FALSE(isSanWarningWritten(getCapturedTextFormatLogMessages()));
+}
+
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/util/net/ssl_manager_windows.cpp b/src/mongo/util/net/ssl_manager_windows.cpp
index 8f816e16468..ce21ddb0de5 100644
--- a/src/mongo/util/net/ssl_manager_windows.cpp
+++ b/src/mongo/util/net/ssl_manager_windows.cpp
@@ -396,6 +396,8 @@ std::shared_ptr<SSLManagerInterface> SSLManagerInterface::create(const SSLParams
 
 namespace {
 
+StatusWith<std::vector<std::string>> getSubjectAlternativeNames(PCCERT_CONTEXT cert);
+
 SSLManagerWindows::SSLManagerWindows(const SSLParams& params, bool isServer)
     : _weakValidation(params.sslWeakCertificateValidation),
       _allowInvalidCertificates(params.sslAllowInvalidCertificates),
@@ -431,6 +433,16 @@ SSLManagerWindows::SSLManagerWindows(const SSLParams& params, bool isServer)
                                      &subjectName,
                                      &_sslConfiguration.serverCertificateExpirationDate));
             uassertStatusOK(_sslConfiguration.setServerSubjectName(std::move(subjectName)));
+
+            auto swSans = getSubjectAlternativeNames(_serverCertificates[0]);
+            const bool hasSan = swSans.isOK() && (0 != swSans.getValue().size());
+            if (!hasSan) {
+                LOGV2_WARNING_OPTIONS(
+                    551192,
+                    {logv2::LogTag::kStartupWarnings},
+                    "Server certificate has no compatible Subject Alternative Name. "
+                    "This may prevent TLS clients from connecting");
+            }
         }
 
         // Monitor the server certificate's expiration