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)
+

Reply via email to