Backport #12998 - Parallel SSL cert load

doc/admin-guide/files/records.yaml.en.rst

@@ -4079,6 +4079,17 @@ SSL Termination
    :file:`ssl_multicert.config` file successfully load.  If false (``0``), SSL certificate
    load failures will not prevent |TS| from starting.
 
+.. ts:cv:: CONFIG proxy.config.ssl.server.multicert.concurrency INT 1
+
+   Controls parallelism when loading :file:`ssl_multicert.config`.
+   A value of ``0`` automatically selects one thread per CPU core.
+   A value of ``1`` (the default) means single-threaded loading.
+   Values greater than ``1`` use that many threads.
+
+   On initial startup (before any traffic is flowing), the loader will use
+   ``max(hardware_concurrency, configured)`` threads since there is no
+   traffic to compete with for CPU.
+
 .. ts:cv:: CONFIG proxy.config.ssl.server.cert.path STRING /config
 
    The location of the SSL certificates and chains used for accepting

include/iocore/net/SSLMultiCertConfigLoader.h

@@ -31,6 +31,9 @@
 
 #include <string>
 #include <set>
+#include <mutex>
+#include <thread>
+#include <tuple>
 #include <vector>
 
 struct SSLConfigParams;
@@ -51,7 +54,7 @@ class SSLMultiCertConfigLoader
   SSLMultiCertConfigLoader(const SSLConfigParams *p) : _params(p) {}
   virtual ~SSLMultiCertConfigLoader(){};
 
-  swoc::Errata load(SSLCertLookup *lookup);
+  swoc::Errata load(SSLCertLookup *lookup, bool firstLoad = false);
 
   virtual SSL_CTX *default_server_ssl_ctx();
 
@@ -88,6 +91,13 @@ class SSLMultiCertConfigLoader
   virtual bool          _store_ssl_ctx(SSLCertLookup *lookup, const shared_SSLMultiCertConfigParams &ssl_multi_cert_params);
   bool _prep_ssl_ctx(const shared_SSLMultiCertConfigParams &sslMultCertSettings, SSLMultiCertConfigLoader::CertLoadData &data,
                      std::set<std::string> &common_names, std::unordered_map<int, std::set<std::string>> &unique_names);
+
+  using SSLConfigLines = std::vector<std::tuple<char *, unsigned>>;
+  void _load_lines(SSLCertLookup *lookup, SSLConfigLines::const_iterator begin, SSLConfigLines::const_iterator end,
+                   swoc::Errata &errata);
+
+  std::mutex _loader_mutex;
+
   virtual void _set_handshake_callbacks(SSL_CTX *ctx);
   virtual bool _setup_session_cache(SSL_CTX *ctx);
   virtual bool _setup_dialog(SSL_CTX *ctx, const SSLMultiCertConfigParams *sslMultCertSettings);

src/iocore/net/P_SSLConfig.h

@@ -72,6 +72,7 @@ struct SSLConfigParams : public ConfigInfo {
   char *cipherSuite;
   char *client_cipherSuite;
   int   configExitOnLoadError;
+  int   configLoadConcurrency;
   int   clientCertLevel;
   int   verify_depth;
   int   ssl_origin_session_cache;

src/iocore/net/SSLConfig.cc

@@ -48,6 +48,7 @@
 #include <array>
 #include <cstring>
 #include <cmath>
+#include <thread>
 #include <unordered_map>
 
 int                SSLConfig::config_index                                = 0;
@@ -216,6 +217,7 @@ SSLConfigParams::reset()
   ssl_session_cache_timeout            = 0;
   ssl_session_cache_auto_clear         = 1;
   configExitOnLoadError                = 1;
+  configLoadConcurrency                = 1;
   clientCertExitOnLoadError            = 0;
 }
 
@@ -523,6 +525,10 @@ SSLConfigParams::initialize()
 
   configFilePath        = ats_stringdup(RecConfigReadConfigPath("proxy.config.ssl.server.multicert.filename"));
   configExitOnLoadError = RecGetRecordInt("proxy.config.ssl.server.multicert.exit_on_load_fail").value_or(0);
+  configLoadConcurrency = RecGetRecordInt("proxy.config.ssl.server.multicert.concurrency").value_or(0);
+  if (configLoadConcurrency == 0) {
+    configLoadConcurrency = std::thread::hardware_concurrency();
+  }
 
   {
     auto rec_str{RecGetRecordStringAlloc("proxy.config.ssl.server.private_key.path")};
@@ -778,7 +784,7 @@ SSLCertificateConfig::reconfigure()
     ink_hrtime_sleep(HRTIME_SECONDS(secs));
   }
 
-  auto errata = SSLMultiCertConfigLoader(params).load(lookup);
+  auto errata = SSLMultiCertConfigLoader(params).load(lookup, configid == 0);
   if (!lookup->is_valid || (errata.has_severity() && errata.severity() >= ERRATA_ERROR)) {
     retStatus = false;
   }

src/iocore/net/SSLUtils.cc

@@ -48,6 +48,9 @@
 #endif
 #include "swoc/swoc_file.h"
 #include "swoc/Errata.h"
+
+#include <thread>
+#include <tuple>
 #include <openssl/asn1.h>
 #include <openssl/bio.h>
 #include <openssl/bn.h>
@@ -1662,11 +1665,20 @@ SSLMultiCertConfigLoader::_store_ssl_ctx(SSLCertLookup *lookup, const shared_SSL
   SSLMultiCertConfigLoader::CertLoadData         data;
 
   if (!this->_prep_ssl_ctx(sslMultCertSettings, data, common_names, unique_names)) {
-    lookup->is_valid = false;
+    {
+      std::lock_guard<std::mutex> lock(_loader_mutex);
+      lookup->is_valid = false;
+    }
     return false;
   }
 
   std::vector<SSLLoadingContext> ctxs = this->init_server_ssl_ctx(data, sslMultCertSettings.get());
+
+  // Serialize all mutations to the shared SSLCertLookup.
+  // The expensive work above (_prep_ssl_ctx + init_server_ssl_ctx) runs
+  // without the lock, allowing parallel cert loading across threads.
+  std::lock_guard<std::mutex> lock(_loader_mutex);
+
   for (const auto &loadingctx : ctxs) {
     if (!sslMultCertSettings ||
         !this->_store_single_ssl_ctx(lookup, sslMultCertSettings, shared_SSL_CTX{loadingctx.ctx, SSL_CTX_free}, loadingctx.ctx_type,
@@ -1916,17 +1928,49 @@ ssl_extract_certificate(const matcher_line *line_info, SSLMultiCertConfigParams
   return true;
 }
 
-swoc::Errata
-SSLMultiCertConfigLoader::load(SSLCertLookup *lookup)
+void
+SSLMultiCertConfigLoader::_load_lines(SSLCertLookup *lookup, SSLConfigLines::const_iterator begin,
+                                      SSLConfigLines::const_iterator end, swoc::Errata &errata)
 {
-  const SSLConfigParams *params = this->_params;
+  const matcher_tags     sslCertTags = {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, false};
+  const SSLConfigParams *params      = this->_params;
+
+  // Each thread needs its own ElevateAccess since POSIX capabilities
+  // are per-thread and don't propagate to spawned threads.
+  uint32_t      elevate_setting = RecGetRecordInt("proxy.config.ssl.cert.load_elevated").value_or(0);
+  ElevateAccess elevate_access(elevate_setting ? ElevateAccess::FILE_PRIVILEGE : 0);
 
-  char        *tok_state = nullptr;
-  char        *line      = nullptr;
-  unsigned     line_num  = 0;
-  matcher_line line_info;
+  for (auto it = begin; it != end; ++it) {
+    auto &[line, line_num] = *it;
 
-  const matcher_tags sslCertTags = {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, false};
+    shared_SSLMultiCertConfigParams sslMultiCertSettings = std::make_shared<SSLMultiCertConfigParams>();
+    matcher_line                    line_info;
+    const char                     *errPtr;
+
+    errPtr = parseConfigLine(line, &line_info, &sslCertTags);
+    Dbg(dbg_ctl_ssl_load, "currently parsing %s at line %d from config file: %s", line, line_num, params->configFilePath);
+    if (errPtr != nullptr) {
+      Warning("%s: discarding %s entry at line %d: %s", __func__, params->configFilePath, line_num, errPtr);
+    } else {
+      if (ssl_extract_certificate(&line_info, sslMultiCertSettings.get())) {
+        if (sslMultiCertSettings->cert || sslMultiCertSettings->opt != SSLCertContextOption::OPT_TUNNEL) {
+          if (!this->_store_ssl_ctx(lookup, sslMultiCertSettings)) {
+            std::lock_guard<std::mutex> lock(_loader_mutex);
+            errata.note(ERRATA_ERROR, "Failed to load certificate on line {}", line_num);
+          }
+        } else {
+          std::lock_guard<std::mutex> lock(_loader_mutex);
+          errata.note(ERRATA_WARN, "No ssl_cert_name specified and no tunnel action set on line {}", line_num);
+        }
+      }
+    }
+  }
+}
+
+swoc::Errata
+SSLMultiCertConfigLoader::load(SSLCertLookup *lookup, bool firstLoad)
+{
+  const SSLConfigParams *params = this->_params;
 
   Note("(%s) %s loading ...", this->_debug_tag(), ts::filename::SSL_MULTICERT);
 
@@ -1935,7 +1979,6 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup)
   if (ec) {
     switch (ec.value()) {
     case ENOENT:
-      // missing config file is an acceptable runtime state
       return swoc::Errata(ERRATA_WARN, "Cannot open SSL certificate configuration \"{}\" - {}", params->configFilePath, ec);
     default:
       return swoc::Errata(ERRATA_ERROR, "Failed to read SSL certificate configuration from \"{}\" - {}", params->configFilePath,
@@ -1949,8 +1992,13 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup)
   elevate_setting          = RecGetRecordInt("proxy.config.ssl.cert.load_elevated").value_or(0);
   ElevateAccess elevate_access(elevate_setting ? ElevateAccess::FILE_PRIVILEGE : 0);
 
+  // Collect all non-empty, non-comment lines for processing
+  char          *tok_state = nullptr;
+  char          *line      = nullptr;
+  unsigned       line_num  = 0;
+  SSLConfigLines config_lines;
+
   line = tokLine(content.data(), &tok_state);
-  swoc::Errata errata(ERRATA_NOTE);
   while (line != nullptr) {
     line_num++;
 
@@ -1960,28 +2008,49 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup)
     }
 
     if (*line != '\0' && *line != '#') {
-      shared_SSLMultiCertConfigParams sslMultiCertSettings = std::make_shared<SSLMultiCertConfigParams>();
-      const char                     *errPtr;
+      config_lines.emplace_back(line, line_num);
+    }
+    line = tokLine(nullptr, &tok_state);
+  }
 
-      errPtr = parseConfigLine(line, &line_info, &sslCertTags);
-      Dbg(dbg_ctl_ssl_load, "currently parsing %s at line %d from config file: %s", line, line_num, params->configFilePath);
-      if (errPtr != nullptr) {
-        Warning("%s: discarding %s entry at line %d: %s", __func__, params->configFilePath, line_num, errPtr);
-      } else {
-        if (ssl_extract_certificate(&line_info, sslMultiCertSettings.get())) {
-          // There must be a certificate specified unless the tunnel action is set
-          if (sslMultiCertSettings->cert || sslMultiCertSettings->opt != SSLCertContextOption::OPT_TUNNEL) {
-            if (!this->_store_ssl_ctx(lookup, sslMultiCertSettings)) {
-              errata.note(ERRATA_ERROR, "Failed to load certificate on line {}", line_num);
-            }
-          } else {
-            errata.note(ERRATA_WARN, "No ssl_cert_name specified and no tunnel action set on line {}", line_num);
-          }
-        }
-      }
+  swoc::Errata errata(ERRATA_NOTE);
+
+  if (params->configLoadConcurrency > 1 && config_lines.size() > 1) {
+    // On first load (no traffic yet), allow more threads for faster startup
+    int num_threads = params->configLoadConcurrency;
+    if (firstLoad) {
+      num_threads = std::max(static_cast<int>(std::thread::hardware_concurrency()), num_threads);
     }
 
-    line = tokLine(nullptr, &tok_state);
+    // Don't spawn more threads than lines
+    num_threads = std::min(num_threads, static_cast<int>(config_lines.size()));
+
+    std::size_t                    bucket_size = config_lines.size() / num_threads;
+    std::size_t                    remainder   = config_lines.size() % num_threads;
+    SSLConfigLines::const_iterator current     = config_lines.cbegin();
+    std::vector<std::thread>       threads;
+
+    Note("(%s) loading %zu certs with %d threads", this->_debug_tag(), config_lines.size(), num_threads);
+
+    for (int t = 0; t < num_threads; ++t) {
+      // Distribute remainder lines across the first threads
+      std::size_t                    this_bucket = bucket_size + (static_cast<std::size_t>(t) < remainder ? 1 : 0);
+      SSLConfigLines::const_iterator end         = current + this_bucket;
+
+      threads.emplace_back(&SSLMultiCertConfigLoader::_load_lines, this, lookup, current, end, std::ref(errata));
+      current = end;
+    }
+
+    for (auto &th : threads) {
+      th.join();
+    }
+
+    Note("(%s) loaded %zu certs in %d threads", this->_debug_tag(), config_lines.size(), num_threads);
+  } else {
+    // Single-threaded path (concurrency == 1 or only 1 line)
+    this->_load_lines(lookup, config_lines.cbegin(), config_lines.cend(), errata);
+
+    Note("(%s) loaded %zu certs (single-threaded)", this->_debug_tag(), config_lines.size());
   }
 
   // We *must* have a default context even if it can't possibly work. The default context is used to

src/records/RecordsConfig.cc

@@ -1174,7 +1174,9 @@ static constexpr RecordElement RecordsConfig[] =
   {RECT_CONFIG, "proxy.config.ssl.server.multicert.filename", RECD_STRING, ts::filename::SSL_MULTICERT, RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
   ,
   {RECT_CONFIG, "proxy.config.ssl.server.multicert.exit_on_load_fail", RECD_INT, "1", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
-,
+  ,
+  {RECT_CONFIG, "proxy.config.ssl.server.multicert.concurrency", RECD_INT, "1", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
+  ,
   {RECT_CONFIG, "proxy.config.ssl.servername.filename", RECD_STRING, ts::filename::SNI, RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
   ,
   {RECT_CONFIG, "proxy.config.ssl.server.ticket_key.filename", RECD_STRING, nullptr, RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}

tests/gold_tests/tls/ssl_multicert_loader.test.py

@@ -109,3 +109,52 @@
 ts2.Disk.traffic_out.Content = Testers.ExcludesExpression(
     'Traffic Server is fully initialized', 'process should fail when invalid certificate specified')
 ts2.Disk.diags_log.Content = Testers.IncludesExpression('EMERGENCY: failed to load SSL certificate file', 'check diags.log"')
+
+##########################################################################
+# Ensure parallel cert loading works correctly with multiple certs
+
+ts3 = Test.MakeATSProcess("ts3", enable_tls=True)
+server3 = Test.MakeOriginServer("server4")
+server3.addResponse("sessionlog.json", request_header, response_header)
+
+ts3.Disk.records_config.update(
+    {
+        'proxy.config.ssl.server.cert.path': f'{ts3.Variables.SSLDir}',
+        'proxy.config.ssl.server.private_key.path': f'{ts3.Variables.SSLDir}',
+        'proxy.config.ssl.server.multicert.concurrency': 4,
+        'proxy.config.diags.debug.enabled': 1,
+        'proxy.config.diags.debug.tags': 'ssl_load',
+    })
+
+ts3.addDefaultSSLFiles()
+
+ts3.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server3.Variables.Port}')
+
+# Add enough cert lines to exercise multiple threads (need > 1 for parallel path,
+# and ideally >= concurrency to actually use all threads)
+ts3.Disk.ssl_multicert_config.AddLines(
+    [
+        'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key',
+        'ssl_cert_name=server.pem ssl_key_name=server.key',
+        'ssl_cert_name=server.pem ssl_key_name=server.key',
+        'ssl_cert_name=server.pem ssl_key_name=server.key',
+        'ssl_cert_name=server.pem ssl_key_name=server.key',
+    ])
+
+tr5 = Test.AddTestRun("Verify parallel cert loading works")
+tr5.Processes.Default.StartBefore(ts3)
+tr5.Processes.Default.StartBefore(server3)
+tr5.StillRunningAfter = ts3
+tr5.StillRunningAfter = server3
+tr5.MakeCurlCommand(
+    f"-q -s -v -k --resolve '{sni_domain}:{ts3.Variables.ssl_port}:127.0.0.1' https://{sni_domain}:{ts3.Variables.ssl_port}",
+    ts=ts3)
+tr5.Processes.Default.ReturnCode = 0
+tr5.Processes.Default.Streams.stdout = Testers.ExcludesExpression("Could Not Connect", "Check response")
+tr5.Processes.Default.Streams.stderr = Testers.IncludesExpression(f"CN={sni_domain}", "Check response")
+
+# Verify the parallel loading code path was actually exercised.
+# With 5 identical cert lines, 4 will fail to insert as duplicates.
+# This confirms all lines were processed (regardless of thread count).
+ts3.Disk.diags_log.Content = Testers.ContainsExpression(
+    'Failed to insert SSL_CTX for certificate', 'duplicate cert insertions confirm all lines were processed')