This is an automated email from the ASF dual-hosted git repository. amc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push: new 7651796f64 Always load initial SSL configuration. (#9580) 7651796f64 is described below commit 7651796f643ae17e70c23115d46063e145d97289 Author: Alan M. Carroll <a...@apache.org> AuthorDate: Sat Apr 8 19:28:57 2023 -0500 Always load initial SSL configuration. (#9580) Issue 9533 - bad certificate disables all TLS on startup. Closes #9553 --- include/tscpp/util/ts_errata.h | 3 +++ iocore/net/P_SSLUtils.h | 4 +++- iocore/net/SSLConfig.cc | 26 +++++++++++++++----------- iocore/net/SSLUtils.cc | 22 ++++++++++++---------- src/traffic_server/CMakeLists.txt | 3 ++- 5 files changed, 35 insertions(+), 23 deletions(-) diff --git a/include/tscpp/util/ts_errata.h b/include/tscpp/util/ts_errata.h index 1f50079522..42bdf11b74 100644 --- a/include/tscpp/util/ts_errata.h +++ b/include/tscpp/util/ts_errata.h @@ -39,3 +39,6 @@ static constexpr swoc::Errata::Severity ERRATA_EMERGENCY{DL_Emergency}; static constexpr std::array<swoc::TextView, 9> Severity_Names{ {"Diag", "Debug", "Status", "Note", "Warn", "Error", "Fatal", "Alert", "Emergency"} }; + +// Temporary string for formatting. +inline thread_local std::string bw_dbg; diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h index 5fc1a6f3e3..934a0c30bc 100644 --- a/iocore/net/P_SSLUtils.h +++ b/iocore/net/P_SSLUtils.h @@ -29,6 +29,8 @@ #endif #include <openssl/ssl.h> +#include "tscpp/util/ts_errata.h" + #include "tscore/ink_config.h" #include "tscore/Diags.h" #include "records/I_RecCore.h" @@ -77,7 +79,7 @@ public: SSLMultiCertConfigLoader(const SSLConfigParams *p) : _params(p) {} virtual ~SSLMultiCertConfigLoader(){}; - bool load(SSLCertLookup *lookup); + swoc::Errata load(SSLCertLookup *lookup); virtual SSL_CTX *default_server_ssl_ctx(); diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index 8154ecd448..3de6cf6f9a 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -610,27 +610,31 @@ SSLCertificateConfig::reconfigure() ink_hrtime_sleep(HRTIME_SECONDS(secs)); } - SSLMultiCertConfigLoader loader(params); - if (!loader.load(lookup)) { + auto errata = SSLMultiCertConfigLoader(params).load(lookup); + if (!lookup->is_valid || (errata.has_severity() && errata.severity() >= ERRATA_ERROR)) { retStatus = false; } - if (!lookup->is_valid) { - retStatus = false; - } - - // If there are errors in the certificate configs and we had wanted to exit on error - // we won't want to reset the config - if (retStatus) { + // If the load succeeded, load it. If there is no current configuration, load even a broken + // config so that a bad initial load doesn't completely disable TLS. + if (retStatus || configid == 0) { configid = configProcessor.set(configid, lookup); } else { delete lookup; } + if (!errata.empty()) { + errata.assign_annotation_glue_text("\n "); + errata.assign_severity_glue_text(" -> \n "); + bwprint(bw_dbg, "\n{}", errata); + } else { + bw_dbg = ""; + } + if (retStatus) { - Note("%s finished loading", params->configFilePath); + Note("%s finished loading%s", params->configFilePath, bw_dbg.c_str()); } else { - Error("%s failed to load", params->configFilePath); + Error("%s failed to load%s", params->configFilePath, bw_dbg.c_str()); } return retStatus; diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index aa4e561b59..c92d5d3252 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -19,6 +19,9 @@ limitations under the License. */ +#include "swoc/Errata.h" +#include "swoc/bwf_std.h" + #include "P_SSLUtils.h" #include "tscpp/util/TextView.h" @@ -1955,7 +1958,7 @@ ssl_extract_certificate(const matcher_line *line_info, SSLMultiCertConfigParams return true; } -bool +swoc::Errata SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) { const SSLConfigParams *params = this->_params; @@ -1975,11 +1978,10 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) switch (ec.value()) { case ENOENT: // missing config file is an acceptable runtime state - Warning("Cannot open SSL certificate configuration from %s - %s", params->configFilePath, strerror(ec.value())); - return true; + return swoc::Errata(ERRATA_WARN, "Cannot open SSL certificate configuration \"{}\" - {}", params->configFilePath, ec); default: - Error("Failed to read SSL certificate configuration from %s - %s", params->configFilePath, strerror(ec.value())); - return false; + return swoc::Errata(ERRATA_ERROR, "Failed to read SSL certificate configuration from \"{}\" - {}", params->configFilePath, + ec); } } @@ -1990,6 +1992,7 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) ElevateAccess elevate_access(elevate_setting ? ElevateAccess::FILE_PRIVILEGE : 0); line = tokLine(content.data(), &tok_state); + swoc::Errata errata(ERRATA_NOTE); while (line != nullptr) { line_num++; @@ -2011,10 +2014,10 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) // 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)) { - return false; + errata.note(ERRATA_ERROR, "Failed to load certificate on line {}", line_num); } } else { - Warning("No ssl_cert_name specified and no tunnel action set"); + errata.note(ERRATA_WARN, "No ssl_cert_name specified and no tunnel action set on line {}", line_num); } } } @@ -2030,12 +2033,11 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup) shared_SSLMultiCertConfigParams sslMultiCertSettings(new SSLMultiCertConfigParams); sslMultiCertSettings->addr = ats_strdup("*"); if (!this->_store_ssl_ctx(lookup, sslMultiCertSettings)) { - Error("failed set default context"); - return false; + errata.note(ERRATA_ERROR, "failed set default context"); } } - return true; + return errata; } // Release SSL_CTX and the associated data. This works for both diff --git a/src/traffic_server/CMakeLists.txt b/src/traffic_server/CMakeLists.txt index 5c3839215c..b0c1462706 100644 --- a/src/traffic_server/CMakeLists.txt +++ b/src/traffic_server/CMakeLists.txt @@ -62,4 +62,5 @@ target_link_libraries(traffic_server if (TS_USE_LINUX_IO_URING) target_link_libraries(traffic_server inkuring uring) -endif (TS_USE_LINUX_IO_URING) \ No newline at end of file +endif (TS_USE_LINUX_IO_URING) +