[trafficserver] 02/03: Fix stale pointer due to SSL config reload (#7148)

[zw...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit a536f494c6a54aa6c8836ce998ac6584ef0d1ecd
Author: Susan Hinrichs <sh...@yahoo-inc.com>
AuthorDate: Mon Aug 31 13:57:11 2020 -0500

    Fix stale pointer due to SSL config reload (#7148)
    
    (cherry picked from commit 157be26c9872d55a0fde91da1986c69eb89cb5b8)
---
 iocore/net/I_NetVConnection.h     | 24 ++++++++++++++++++++----
 iocore/net/P_UnixNetVConnection.h |  1 -
 iocore/net/SSLNetVConnection.cc   |  2 +-
 proxy/http/HttpSM.cc              |  2 +-
 proxy/http/HttpSessionManager.cc  |  2 +-
 5 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/iocore/net/I_NetVConnection.h b/iocore/net/I_NetVConnection.h
index 725f99f..ca7c3ab 100644
--- a/iocore/net/I_NetVConnection.h
+++ b/iocore/net/I_NetVConnection.h
@@ -199,7 +199,7 @@ struct NetVCOptions {
   /**
    * Client certificate to use in response to OS's certificate request
    */
-  const char *ssl_client_cert_name = nullptr;
+  ats_scoped_str ssl_client_cert_name;
   /*
    * File containing private key matching certificate
    */
@@ -254,6 +254,17 @@ struct NetVCOptions {
   }
 
   self &
+  set_ssl_client_cert_name(const char *name)
+  {
+    if (name) {
+      ssl_client_cert_name = ats_strdup(name);
+    } else {
+      ssl_client_cert_name = nullptr;
+    }
+    return *this;
+  }
+
+  self &
   set_ssl_servername(const char *name)
   {
     if (name) {
@@ -292,9 +303,10 @@ struct NetVCOptions {
        * memcpy removes the extra reference to that's copy of the string
        * Removing the release will eventually cause a double free crash
        */
-      sni_servername = nullptr; // release any current name.
-      ssl_servername = nullptr;
-      sni_hostname   = nullptr;
+      sni_servername       = nullptr; // release any current name.
+      ssl_servername       = nullptr;
+      sni_hostname         = nullptr;
+      ssl_client_cert_name = nullptr;
       memcpy(static_cast<void *>(this), &that, sizeof(self));
       if (that.sni_servername) {
         sni_servername.release(); // otherwise we'll free the source string.
@@ -308,6 +320,10 @@ struct NetVCOptions {
         sni_hostname.release(); // otherwise we'll free the source string.
         this->sni_hostname = ats_strdup(that.sni_hostname);
       }
+      if (that.ssl_client_cert_name) {
+        this->ssl_client_cert_name.release(); // otherwise we'll free the source string.
+        this->ssl_client_cert_name = ats_strdup(that.ssl_client_cert_name);
+      }
     }
     return *this;
   }
diff --git a/iocore/net/P_UnixNetVConnection.h b/iocore/net/P_UnixNetVConnection.h
index 8b7cb2d..cd94178 100644
--- a/iocore/net/P_UnixNetVConnection.h
+++ b/iocore/net/P_UnixNetVConnection.h
@@ -72,7 +72,6 @@ NetVCOptions::reset()
   sni_hostname                = nullptr;
   ssl_client_cert_name        = nullptr;
   ssl_client_private_key_name = nullptr;
-  ssl_client_ca_cert_name     = nullptr;
 }
 
 inline void
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 2f641e9..59f6f27 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -1068,7 +1068,7 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
 
       // First Look to see if there are override parameters
       if (options.ssl_client_cert_name) {
-        std::string certFilePath = Layout::get()->relative_to(params->clientCertPathOnly, options.ssl_client_cert_name);
+        std::string certFilePath = Layout::get()->relative_to(params->clientCertPathOnly, options.ssl_client_cert_name.get());
         std::string keyFilePath;
         if (options.ssl_client_private_key_name) {
           keyFilePath = Layout::get()->relative_to(params->clientKeyPathOnly, options.ssl_client_private_key_name);
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 698f9c1..84dc4f4 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -5142,7 +5142,7 @@ HttpSM::do_http_server_open(bool raw)
     opt.f_tcp_fastopen = (t_state.txn_conf->sock_option_flag_out & NetVCOptions::SOCK_OPT_TCP_FAST_OPEN);
   }
 
-  opt.ssl_client_cert_name        = t_state.txn_conf->ssl_client_cert_filename;
+  opt.set_ssl_client_cert_name(t_state.txn_conf->ssl_client_cert_filename);
   opt.ssl_client_private_key_name = t_state.txn_conf->ssl_client_private_key_filename;
   opt.ssl_client_ca_cert_name     = t_state.txn_conf->ssl_client_ca_cert_filename;
 
diff --git a/proxy/http/HttpSessionManager.cc b/proxy/http/HttpSessionManager.cc
index ac2fed4..8952a61 100644
--- a/proxy/http/HttpSessionManager.cc
+++ b/proxy/http/HttpSessionManager.cc
@@ -126,7 +126,7 @@ ServerSessionPool::validate_cert(HttpSM *sm, NetVConnection *netvc)
   // a new connection.
   //
   if (sm->t_state.scheme == URL_WKSIDX_HTTPS) {
-    const char *session_cert       = netvc->options.ssl_client_cert_name;
+    const char *session_cert       = netvc->options.ssl_client_cert_name.get();
     std::string_view proposed_cert = sm->get_outbound_cert();
     Debug("http_ss", "validate_cert proposed_cert=%.*s, cert=%s", static_cast<int>(proposed_cert.size()), proposed_cert.data(),
           session_cert);