diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 6fbbde8f3c0..dc6000a667a 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -3463,6 +3463,13 @@ 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 0 + + If set to a non-zero value, allow the loading and reloading of + :file:`ssl_multicert.config` to be parallelized. The value specifies the + concurrency, i.e. how many threads will process the file in parallel. A + value of ``-1`` means to create one thread for each core. + .. ts:cv:: CONFIG proxy.config.ssl.server.cert.path STRING /config The location of the SSL certificates and chains used for accepting diff --git a/iocore/net/P_SSLConfig.h b/iocore/net/P_SSLConfig.h index 0ecab2de7dc..bf3a192a969 100644 --- a/iocore/net/P_SSLConfig.h +++ b/iocore/net/P_SSLConfig.h @@ -76,6 +76,7 @@ struct SSLConfigParams : public ConfigInfo { char *cipherSuite; char *client_cipherSuite; int configExitOnLoadError; + int configLoadConcurrency; int clientCertLevel; int verify_depth; int ssl_origin_session_cache; diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h index 1d876946441..194d1cf9ffd 100644 --- a/iocore/net/P_SSLUtils.h +++ b/iocore/net/P_SSLUtils.h @@ -60,10 +60,10 @@ class SSLMultiCertConfigLoader struct CertLoadData { std::vector cert_names_list, key_list, ca_list, ocsp_list; }; - SSLMultiCertConfigLoader(const SSLConfigParams *p) : _params(p) {} - virtual ~SSLMultiCertConfigLoader(){}; + SSLMultiCertConfigLoader(const SSLConfigParams *p) : _params(p) { ink_mutex_init(&m_mutex); } + virtual ~SSLMultiCertConfigLoader() { ink_mutex_destroy(&m_mutex); }; - bool load(SSLCertLookup *lookup); + bool load(SSLCertLookup *lookup, bool firstLoad); virtual SSL_CTX *default_server_ssl_ctx(); virtual SSL_CTX *init_server_ssl_ctx(CertLoadData const &data, const SSLMultiCertConfigParams *sslMultCertSettings, @@ -89,8 +89,12 @@ class SSLMultiCertConfigLoader std::set &names); private: - virtual const char *_debug_tag() const; + using SSLConfigLines = std::vector>; + bool _store_ssl_ctx(SSLCertLookup *lookup, const shared_SSLMultiCertConfigParams &ssl_multi_cert_params); + void _load_lines(SSLCertLookup *lookup, SSLConfigLines::const_iterator begin, SSLConfigLines::const_iterator end); + + virtual const char *_debug_tag() const; 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); @@ -103,6 +107,8 @@ class SSLMultiCertConfigLoader virtual bool _set_info_callback(SSL_CTX *ctx); virtual bool _set_npn_callback(SSL_CTX *ctx); virtual bool _set_alpn_callback(SSL_CTX *ctx); + + ink_mutex m_mutex; // Mutex around concurrent loading / reloading of multicert configurations }; // Create a new SSL server context fully configured (cert and keys are optional). diff --git a/iocore/net/QUICMultiCertConfigLoader.cc b/iocore/net/QUICMultiCertConfigLoader.cc index 17a80ca0acb..87821b94d95 100644 --- a/iocore/net/QUICMultiCertConfigLoader.cc +++ b/iocore/net/QUICMultiCertConfigLoader.cc @@ -53,7 +53,7 @@ QUICCertConfig::reconfigure() SSLCertLookup *lookup = new SSLCertLookup(); QUICMultiCertConfigLoader loader(params); - loader.load(lookup); + loader.load(lookup, 0 == _config_id); _config_id = configProcessor.set(_config_id, lookup); } diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index 2af43bcdfb4..48ddde6d618 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -33,6 +33,7 @@ #include #include +#include #include "tscore/ink_config.h" #include "tscore/ink_platform.h" @@ -307,6 +308,10 @@ SSLConfigParams::initialize() configFilePath = ats_stringdup(RecConfigReadConfigPath("proxy.config.ssl.server.multicert.filename")); REC_ReadConfigInteger(configExitOnLoadError, "proxy.config.ssl.server.multicert.exit_on_load_fail"); + REC_ReadConfigInteger(configLoadConcurrency, "proxy.config.ssl.server.multicert.concurrency"); + if (configLoadConcurrency < 0) { + configLoadConcurrency = std::thread::hardware_concurrency(); + } REC_ReadConfigStringAlloc(ssl_server_private_key_path, "proxy.config.ssl.server.private_key.path"); set_paths_helper(ssl_server_private_key_path, nullptr, &serverKeyPathOnly, nullptr); @@ -500,7 +505,7 @@ SSLCertificateConfig::reconfigure() } SSLMultiCertConfigLoader loader(params); - loader.load(lookup); + loader.load(lookup, 0 == configid); // The flag indicates if this is a first time load or not if (!lookup->is_valid) { retStatus = false; diff --git a/iocore/net/SSLStats.cc b/iocore/net/SSLStats.cc index aaf3a8f8cc0..79c0f313615 100644 --- a/iocore/net/SSLStats.cc +++ b/iocore/net/SSLStats.cc @@ -257,7 +257,6 @@ SSLInitializeStatistics() RecRegisterRawStat(ssl_rsb, RECT_PROCESS, statName.c_str(), RECD_INT, RECP_NON_PERSISTENT, (int)ssl_cipher_stats_start + index, RecRawStatSyncSum); SSL_CLEAR_DYN_STAT((int)ssl_cipher_stats_start + index); - Debug("ssl", "registering SSL cipher metric '%s'", statName.c_str()); } } diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index 4a190d00b9e..dd2d7055d9f 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -49,6 +49,9 @@ #include #include #include +#include +#include +#include #include #include @@ -1531,7 +1534,7 @@ SSLCreateServerContext(const SSLConfigParams *params, const SSLMultiCertConfigPa } /** - Insert SSLCertContext (SSL_CTX ans options) into SSLCertLookup with key. + Insert SSLCertContext (SSL_CTX and options) into SSLCertLookup with key. Do NOT call SSL_CTX_set_* functions from here. SSL_CTX should be set up by SSLMultiCertConfigLoader::init_server_ssl_ctx(). */ bool @@ -1601,6 +1604,12 @@ SSLMultiCertConfigLoader::_store_single_ssl_ctx(SSLCertLookup *lookup, const sha { bool inserted = false; shared_ssl_ticket_key_block keyblock = nullptr; + + // This may be obsessively large critical section to lock, but there is code written in many places + // which were not intended to be reentrant. The importan / expensive part to avoid locking around is + // the actual loading of the cert, which happens just before this. + ink_mutex_acquire(&m_mutex); + // Load the session ticket key if session tickets are not disabled if (sslMultCertSettings->session_ticket_enabled != 0) { keyblock = shared_ssl_ticket_key_block(ssl_context_enable_tickets(ctx.get(), nullptr), ticket_block_free); @@ -1637,6 +1646,9 @@ SSLMultiCertConfigLoader::_store_single_ssl_ctx(SSLCertLookup *lookup, const sha } } + // Release the mutex now, we should be in the clear. + ink_mutex_release(&m_mutex); + if (inserted) { if (SSLConfigParams::init_ssl_ctx_cb) { SSLConfigParams::init_ssl_ctx_cb(ctx.get(), true); @@ -1721,22 +1733,49 @@ ssl_extract_certificate(const matcher_line *line_info, SSLMultiCertConfigParams return true; } -bool -SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) +// This is a helper function to make it easier to parallelize the configuration parsing. This +// can be called directly over the entire range, or from stl::thread CTOR, with sub-ranges. +void +SSLMultiCertConfigLoader::_load_lines(SSLCertLookup *lookup, SSLConfigLines::const_iterator begin, + SSLConfigLines::const_iterator end) { - const SSLConfigParams *params = this->_params; - - char *tok_state = nullptr; - char *line = nullptr; - unsigned line_num = 0; - matcher_line line_info; - const matcher_tags sslCertTags = {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, false}; + const SSLConfigParams *params = this->_params; + + for (auto it = begin; it != end; ++it) { + auto &&[line, line_num] = *it; + shared_SSLMultiCertConfigParams sslMultiCertSettings = std::make_shared(); + const char *errPtr; + matcher_line line_info; + + errPtr = parseConfigLine(line, &line_info, &sslCertTags); + Debug("ssl", "currently parsing %s", line); + if (errPtr != nullptr) { + RecSignalWarning(REC_SIGNAL_CONFIG_ERROR, "%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) { + this->_store_ssl_ctx(lookup, sslMultiCertSettings); + } else { + Warning("No ssl_cert_name specified and no tunnel action set"); + } + } + } + } +} +bool +SSLMultiCertConfigLoader::load(SSLCertLookup *lookup, bool firstLoad) +{ Note("%s loading ...", ts::filename::SSL_MULTICERT); + Debug("ssl", "%s loading ...", ts::filename::SSL_MULTICERT); + const SSLConfigParams *params = this->_params; std::error_code ec; std::string content{ts::file::load(ts::file::path{params->configFilePath}, ec)}; + if (ec) { switch (ec.value()) { case ENOENT: @@ -1754,6 +1793,13 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) REC_ReadConfigInteger(elevate_setting, "proxy.config.ssl.cert.load_elevated"); ElevateAccess elevate_access(elevate_setting ? ElevateAccess::FILE_PRIVILEGE : 0); + // Store all the lines in the list, slightly clunky but lets us parallelize this if allowed + // ToDo: Maybe change the parser here to use a more C++'ish line parser rather than tokLine()... + char *line = nullptr; + unsigned line_num = 0; + char *tok_state = nullptr; + SSLConfigLines single_lines; + line = tokLine(content.data(), &tok_state); while (line != nullptr) { line_num++; @@ -1764,29 +1810,38 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) } if (*line != '\0' && *line != '#') { - shared_SSLMultiCertConfigParams sslMultiCertSettings = std::make_shared(); - const char *errPtr; - - errPtr = parseConfigLine(line, &line_info, &sslCertTags); - Debug("ssl", "currently parsing %s", line); - if (errPtr != nullptr) { - RecSignalWarning(REC_SIGNAL_CONFIG_ERROR, "%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) { - this->_store_ssl_ctx(lookup, sslMultiCertSettings); - } else { - Warning("No ssl_cert_name specified and no tunnel action set"); - } - } - } + single_lines.push_back(std::make_tuple(line, line_num)); } - line = tokLine(nullptr, &tok_state); } + // Process all the lines if we're not running parallelization on multiple threads + if (params->configLoadConcurrency > 0) { + int num_threads = firstLoad ? std::max(static_cast(std::thread::hardware_concurrency()), params->configLoadConcurrency) : + params->configLoadConcurrency; + std::size_t bucket_size = std::max(1u, static_cast(single_lines.size() / num_threads)); + SSLConfigLines::const_iterator current = std::as_const(single_lines).begin(); + std::list threads; + std::size_t num_lines = single_lines.size(); + + while (num_lines > 0) { + SSLConfigLines::const_iterator last = current + std::min(num_lines, bucket_size); + + num_lines -= std::min(bucket_size, num_lines); + threads.push_back(std::thread(&SSLMultiCertConfigLoader::_load_lines, this, lookup, current, last)); + current = last; + } + + // Wait for all the threads to finish their tasks. + for (std::thread &th : threads) { + if (th.joinable()) + th.join(); + } + ink_assert(num_lines == 0); // Make sure no lines where not processed... + } else { + this->_load_lines(lookup, single_lines.begin(), single_lines.end()); + } + // We *must* have a default context even if it can't possibly work. The default context is used to // bootstrap the SSL handshake so that we can subsequently do the SNI lookup to switch to the real // context. diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index 2d5ed23393e..05dc9e80009 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -1109,7 +1109,9 @@ static const 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_NULL, "[0-1]", RECA_NULL} -, + , + {RECT_CONFIG, "proxy.config.ssl.server.multicert.concurrency", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_NULL, "^[0-9]+$", 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}