Fix an error on SSL config reload (plus some cleanup).

doc/developer-guide/api/functions/TSSslSecret.en.rst

@@ -11,6 +11,7 @@
    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    implied.  See the License for the specific language governing
    permissions and limitations under the License.
+
 .. include:: /common.defs
 
 .. default-domain:: c
@@ -48,12 +49,13 @@ Synopsis
 
     #include <ts/ts.h>
 
-.. function:: TSReturnCode TSSslSecretGet(const char * secret_name, int secret_name_length, const char ** secret_data_return, int * secret_data_len)
+.. function:: TSAllocatedVarLenData TSSslSecretGet(const char * secret_name, int secret_name_length)
 
 Description
 ===========
 
-:func:`TSSslSecretGet` fetches the named secret from the current secret map. TS_ERROR is returned if there is no entry for the secret.
+:func:`TSSslSecretGet` fetches the named secret from the current secret map.  If there is no secret with the
+given name, the data pointer will be null, and the size will be zero.
 
 TSSslSecretUpdate
 *****************

doc/developer-guide/api/functions/TSTypes.en.rst

@@ -56,6 +56,18 @@ more widely. Those are described on this page.
 
 .. type:: TSAction
 
+.. type:: TSAllocatedVarLenData
+
+   Valiable lenght data stored in memory allocated with :func:`TSmalloc`.
+
+   .. member:: char * data
+
+      Pointer to first byte of sequence of bytes containing data.  Must be freed by plugin with :func:`TSfree`.
+
+   .. member:: size_t size
+
+      Number of bytes in sequence pointed to by data.
+
 .. type:: TSCacheKey
 
 .. type:: TSConfig

include/ts/apidefs.h.in

@@ -1460,6 +1460,12 @@ namespace ts
 }
 #endif
 
+/* Variable-length buffer, with data returned by a TS API call.  Data must be freed by plugin with TSfree(). */
+typedef struct tsapi_allocated_var_len_data {
+  char *data;
+  size_t size;
+} TSAllocatedVarLenData;
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

include/ts/ts.h

@@ -1304,8 +1304,10 @@ tsapi TSReturnCode TSSslServerCertUpdate(const char *cert_path, const char *key_
 
 /* Update the transient secret table for SSL_CTX loading */
 tsapi TSReturnCode TSSslSecretSet(const char *secret_name, int secret_name_length, const char *secret_data, int secret_data_len);
-tsapi TSReturnCode TSSslSecretGet(const char *secret_name, int secret_name_length, const char **secret_data_return,
-                                  int *secret_data_len);
+
+/* Returns secret with given name as TSAllocatedVerLenData.  If there is no secret with the given name, data will
+** be null and size will be zero.  Calling code must free by calling TSfree(). */
+tsapi TSAllocatedVarLenData TSSslSecretGet(const char *secret_name, int secret_name_length);
 
 tsapi TSReturnCode TSSslSecretUpdate(const char *secret_name, int secret_name_length);
 

include/tscpp/api/Cleanup.h

@@ -26,6 +26,8 @@
 
 #include <type_traits>
 #include <memory>
+#include <cstring>
+#include <new>
 
 #include <ts/ts.h>
 
@@ -100,6 +102,77 @@ struct TSIOBufferReaderDeleter {
 };
 using TSIOBufferReaderUniqPtr = std::unique_ptr<std::remove_pointer_t<TSIOBufferReader>, TSIOBufferReaderDeleter>;
 
+// Guard for TSAllocatedVarLenData.
+//
+class TSAllocatedVarLenDataGuard
+{
+public:
+  TSAllocatedVarLenDataGuard()
+  {
+    _m.data = nullptr;
+    _m.size = 0;
+  }
+
+  TSAllocatedVarLenDataGuard(TSAllocatedVarLenData const &d) : _m(d) { TSAssert((nullptr == d.data) == (0 == d.size)); }
+
+  operator TSAllocatedVarLenData const &() const { return _m; }
+
+  TSAllocatedVarLenData const &
+  operator()() const
+  {
+    return _m;
+  }
+
+  TSAllocatedVarLenDataGuard(TSAllocatedVarLenDataGuard const &src)
+  {
+    TSAssert((nullptr == src._m.data) == (0 == src._m.size));
+
+    if (src._m.data && src._m.size) {
+      _m.data = static_cast<char *>(TSmalloc(src._m.size));
+      std::memcpy(_m.data, src._m.data, src._m.size);
+      _m.size = src._m.size;
+
+    } else {
+      _m.data = nullptr;
+      _m.size = 0;
+    }
+  }
+
+  TSAllocatedVarLenDataGuard &
+  operator=(TSAllocatedVarLenDataGuard const &src)
+  {
+    TSAssert((nullptr == src._m.data) == (0 == src._m.size));
+
+    this->~TSAllocatedVarLenDataGuard();
+    new (this) TSAllocatedVarLenDataGuard(src);
+    return *this;
+  }
+
+  TSAllocatedVarLenDataGuard(TSAllocatedVarLenDataGuard &&src)
+  {
+    TSAssert((nullptr == src._m.data) == (0 == src._m.size));
+
+    _m          = src;
+    src._m.data = nullptr;
+    src._m.size = 0;
+  }
+
+  TSAllocatedVarLenDataGuard &
+  operator=(TSAllocatedVarLenDataGuard &&src)
+  {
+    TSAssert((nullptr == src._m.data) == (0 == src._m.size));
+
+    this->~TSAllocatedVarLenDataGuard();
+    new (this) TSAllocatedVarLenDataGuard(std::move(src));
+    return *this;
+  }
+
+  ~TSAllocatedVarLenDataGuard() { TSfree(_m.data); }
+
+private:
+  TSAllocatedVarLenData _m;
+};
+
 class TxnAuxDataMgrBase
 {
 protected:

iocore/cache/test/stub.cc

@@ -51,6 +51,13 @@ APIHook::invoke(int, void *) const
   return 0;
 }
 
+int
+APIHook::blocking_invoke(int, void *) const
+{
+  ink_assert(false);
+  return 0;
+}
+
 APIHook *
 APIHook::next() const
 {

iocore/net/P_SSLConfig.h

@@ -30,6 +30,8 @@
  ****************************************************************************/
 #pragma once
 
+#include <atomic>
+
 #include <openssl/rand.h>
 
 #include "tscore/ink_inet.h"
@@ -167,6 +169,11 @@ struct SSLConfigParams : public ConfigInfo {
   void cleanup();
   void reset();
   void SSLConfigInit(IpMap *global);
+
+private:
+  // c_str() of string passed to in-progess call to updateCTX().
+  //
+  mutable std::atomic<char const *> secret_for_updateCTX{nullptr};
 };
 
 /////////////////////////////////////////////////////////////

iocore/net/P_SSLSecret.h

@@ -19,6 +19,8 @@
   limitations under the License.
  */
 
+#pragma once
+
 #include <string>
 #include <string_view>
 #include <mutex>
@@ -28,14 +30,13 @@ class SSLSecret
 {
 public:
   SSLSecret() {}
-  bool getSecret(const std::string &name, std::string_view &data) const;
-  bool setSecret(const std::string &name, const char *data, int data_len);
-  bool getOrLoadSecret(const std::string &name, const std::string &name2, std::string_view &data, std::string_view &data2);
+  std::string getSecret(const std::string &name) const;
+  void setSecret(const std::string &name, std::string_view data);
+  void getOrLoadSecret(const std::string &name1, const std::string &name2, std::string &data, std::string &data2);
 
 private:
-  const std::string *getSecretItem(const std::string &name) const;
-  bool loadSecret(const std::string &name, const std::string &name2, std::string &data_item, std::string &data_item2);
-  bool loadFile(const std::string &name, std::string &data_item);
+  void loadSecret(const std::string &name1, const std::string &name2, std::string &data_item, std::string &data_item2);
+  std::string loadFile(const std::string &name);
 
   std::unordered_map<std::string, std::string> secret_map;
   mutable std::recursive_mutex secret_map_mutex;

iocore/net/SSLConfig.cc

@@ -745,13 +745,32 @@ cleanup_bio(BIO *&biop)
 void
 SSLConfigParams::updateCTX(const std::string &cert_secret_name) const
 {
+  Debug("ssl_config_updateCTX", "Update cert %s, %p", cert_secret_name.c_str(), this);
+
+  // Instances of SSLConfigParams should access by one thread at a time only.  secret_for_updateCTX is accessed
+  // atomically as a fail-safe.
+  //
+  char const *expected = nullptr;
+  if (!secret_for_updateCTX.compare_exchange_strong(expected, cert_secret_name.c_str())) {
+    if (is_debug_tag_set("ssl_config_updateCTX")) {
+      // As a fail-safe, handle it if secret_for_updateCTX doesn't or no longer points to a null-terminated string.
+      //
+      char const *s{expected};
+      for (; *s && (std::size_t(s - expected) < cert_secret_name.size()); ++s) {
+      }
+      Debug("ssl_config_updateCTX", "Update cert, indirect recusive call caused by call for %.*s", int(s - expected), expected);
+    }
+    return;
+  }
+
   // Clear the corresponding client CTXs.  They will be lazy loaded later
-  Debug("ssl", "Update cert %s", cert_secret_name.c_str());
   this->clearCTX(cert_secret_name);
 
   // Update the server cert
   SSLMultiCertConfigLoader loader(this);
   loader.update_ssl_ctx(cert_secret_name);
+
+  secret_for_updateCTX = nullptr;
 }
 
 void
@@ -806,8 +825,8 @@ SSLConfigParams::getCTX(const std::string &client_cert, const std::string &key_f
 
     // Set public and private keys
     if (!client_cert.empty()) {
-      std::string_view secret_data;
-      std::string_view secret_key_data;
+      std::string secret_data;
+      std::string secret_key_data;
 
       // Fetch the client_cert data
       std::string completeSecretPath{Layout::get()->relative_to(this->clientCertPathOnly, client_cert)};