add host_override to parent.config and other sni name fixes (#12868)

* add host_override to parent.config and other sni name fixes

* add documentation for host_override in parent.config, update strategies.yaml docs

* add test using pristine_host_hdr setting

---------

Co-authored-by: Brian Olsen <bolsen149@comcast.com>

Author: traeak

doc/admin-guide/files/parent.config.en.rst

@@ -324,6 +324,14 @@ The following list shows the possible actions and their allowed values.
 
   -  ``false`` - The default.  Do not ignore the host status.
 
+.. _parent-config-format-host_override:
+
+``host_override``
+    One of the following values:
+
+    -  ``true`` - Sets the host header of the request using the selected parent, including the SNI name to be used for any upstream TLS connection.  Useful when a parent is another CDN that requires a correct SNI name.
+    -  ``false`` - The default.  Does not change the host header.
+
 .. _parent-config-format-hash-algorithm:
 
 ``hash_algorithm``

doc/admin-guide/files/strategies.yaml.en.rst

@@ -286,7 +286,7 @@ Each **strategy** in the list may using the following parameters:
     (**self** should only be necessary when the local hostname can only be translated to an IP address
     with a DNS lookup.)
 
-- **host_override**: A boolean value that will set the host header to the selected parent rather than the original host. **true** sets host header to selected parent. **false** (default) leaves the host header untouched.
+- **host_override**: A boolean value that will set the host header to the selected parent rather than the original host, including the SNI name for any attempted upstream TLS connection. **true** sets host header to selected parent. **false** (default) leaves the host header untouched.
 
 Example:
 ::

include/proxy/ParentSelection.h

@@ -167,6 +167,7 @@ class ParentRecord : public ControlBase
   int                             max_unavailable_server_retries     = 1;
   int                             secondary_mode                     = 1;
   bool                            ignore_self_detect                 = false;
+  bool                            host_override                      = false;
   ParentHashAlgorithm             consistent_hash_algorithm          = ParentHashAlgorithm::SIPHASH24;
   uint64_t                        consistent_hash_seed0              = 0;
   uint64_t                        consistent_hash_seed1              = 0;
@@ -242,6 +243,20 @@ struct ParentResult {
     return is_api_result() ? ParentRetry_t::NONE : rec->parent_retry;
   }
 
+  bool
+  host_override() const
+  {
+    if (is_api_result()) {
+      return false;
+    }
+
+    if (!is_some()) {
+      return false;
+    }
+
+    return rec->host_override;
+  }
+
   unsigned
   max_retries(ParentRetry_t method) const
   {

include/proxy/http/remap/NextHopSelectionStrategy.h

@@ -27,7 +27,6 @@
 #include <utility>
 
 #include "ts/apidefs.h"
-#include "proxy/ParentSelection.h"
 #include "proxy/http/HttpTransact.h"
 
 #ifndef _NH_UNIT_TESTS_
@@ -199,8 +198,6 @@ class NextHopSelectionStrategy
                            const time_t now = 0);
   bool         nextHopExists(TSHttpTxn txnp, void *ih = nullptr);
 
-  void setHostHeader(TSHttpTxn txnp, const char *hostname);
-
   virtual ParentRetry_t responseIsRetryable(int64_t sm_id, HttpTransact::CurrentInfo &current_info, HTTPStatus response_code);
 
   void retryComplete(TSHttpTxn txn, const char *hostname, const int port);

src/proxy/ParentSelection.cc

@@ -797,6 +797,13 @@ ParentRecord::Init(matcher_line *line_info)
         ignore_self_detect = false;
       }
       used = true;
+    } else if (strcasecmp(label, "host_override") == 0) {
+      if (strcasecmp(val, "true") == 0) {
+        host_override = true;
+      } else {
+        host_override = false;
+      }
+      used = true;
     } else if (strcasecmp(label, "hash_algorithm") == 0) {
       consistent_hash_algorithm = parseHashAlgorithm(val);
       used                      = true;

src/proxy/http/HttpSM.cc

@@ -5337,11 +5337,11 @@ HttpSM::get_outbound_sni() const
     // By default the host header field value is used for the SNI.
     zret = t_state.hdr_info.server_request.host_get();
   } else if (_ua.get_txn() && policy == "server_name"_tv) {
-    const char *server_name = snis->get_sni_server_name();
-    if (server_name[0] == '\0') {
+    const char *const server_name = snis->get_sni_server_name();
+    if (nullptr == server_name || server_name[0] == '\0') {
       zret.assign(nullptr, swoc::TextView::npos);
     } else {
-      zret.assign(snis->get_sni_server_name(), swoc::TextView::npos);
+      zret.assign(server_name, swoc::TextView::npos);
     }
   } else if (policy.front() == '@') { // guaranteed non-empty from previous clause
     zret = policy.remove_prefix(1);

src/proxy/http/HttpTransact.cc

@@ -139,19 +139,51 @@ bypass_ok(HttpTransact::State *s)
   return false;
 }
 
+// wrapper to choose between a remap next hop strategy or use parent.config
+// remap next hop strategy is preferred
+inline static bool
+host_override(HttpTransact::State *s)
+{
+  if (s->response_action.handled) { // should be handled by the plugin
+    return false;
+  } else if (nullptr != s->next_hop_strategy) {
+    // remap strategies do not support the TSHttpTxnParentProxySet API.
+    return s->next_hop_strategy->host_override;
+  } else if (nullptr != s->parent_params) {
+    return s->parent_result.host_override();
+  }
+  return false;
+}
+
+// wrapper to choose between a remap next hop strategy or use parent.config
+// remap next hop strategy is preferred
+inline static bool
+is_some(HttpTransact::State *s)
+{
+  if (s->response_action.handled) {
+    return true;
+  } else if (nullptr != s->next_hop_strategy) {
+    // remap strategies do not support the TSHttpTxnParentProxySet API.
+    return s->parent_result.result == ParentResultType::SPECIFIED;
+  } else if (nullptr != s->parent_params) {
+    return s->parent_result.is_some();
+  }
+  return false;
+}
+
 // wrapper to choose between a remap next hop strategy or use parent.config
 // remap next hop strategy is preferred
 inline static bool
 is_api_result(HttpTransact::State *s)
 {
-  bool r = false;
+  bool res = false;
   if (nullptr != s->next_hop_strategy) {
     // remap strategies do not support the TSHttpTxnParentProxySet API.
-    r = false;
+    res = false;
   } else if (nullptr != s->parent_params) {
-    r = s->parent_result.is_api_result();
+    res = s->parent_result.is_api_result();
   }
-  return r;
+  return res;
 }
 
 // wrapper to get the max_retries.
@@ -207,6 +239,8 @@ retry_type(HttpTransact::State *s)
 inline static void
 findParent(HttpTransact::State *s)
 {
+  TxnDbg(dbg_ctl_http_trans, "findParent");
+
   Metrics::Counter::increment(http_rsb.parent_count);
   if (s->response_action.handled) {
     s->parent_result.hostname = s->response_action.action.hostname;
@@ -295,6 +329,8 @@ parentExists(HttpTransact::State *s)
 inline static void
 nextParent(HttpTransact::State *s)
 {
+  TxnDbg(dbg_ctl_http_trans, "nextParent");
+
   TxnDbg(dbg_ctl_parent_down, "connection to parent %s failed, conn_state: %s, request to origin: %s", s->parent_result.hostname,
          HttpDebugNames::get_server_state_name(s->current.state), s->request_data.get_host());
   Metrics::Counter::increment(http_rsb.parent_count);
@@ -599,10 +635,11 @@ find_server_and_update_current_info(HttpTransact::State *s)
     } else {
       findParent(s);
     }
-    if (!s->parent_result.is_some() || is_api_result(s) || parent_is_proxy(s)) {
+    if (!is_some(s) || is_api_result(s) || parent_is_proxy(s)) {
       TxnDbg(dbg_ctl_http_trans, "request not cacheable, so bypass parent");
       s->parent_result.result = ParentResultType::DIRECT;
     }
+
   } else if (s->txn_conf->uncacheable_requests_bypass_parent && s->txn_conf->no_dns_forward_to_parent == 0 &&
              !HttpTransact::is_request_cache_lookupable(s)) {
     // request not lookupable and cacheable, so bypass parent if the parent is not an origin server.
@@ -616,10 +653,12 @@ find_server_and_update_current_info(HttpTransact::State *s)
     } else {
       findParent(s);
     }
-    if (!s->parent_result.is_some() || is_api_result(s) || parent_is_proxy(s)) {
+
+    if (!is_some(s) || is_api_result(s) || parent_is_proxy(s)) {
       TxnDbg(dbg_ctl_http_trans, "request not cacheable, so bypass parent");
       s->parent_result.result = ParentResultType::DIRECT;
     }
+
   } else {
     switch (s->parent_result.result) {
     case ParentResultType::UNDEFINED:
@@ -660,14 +699,32 @@ find_server_and_update_current_info(HttpTransact::State *s)
   }
 
   switch (s->parent_result.result) {
-  case ParentResultType::SPECIFIED:
-    s->parent_info.name = s->arena.str_store(s->parent_result.hostname, strlen(s->parent_result.hostname));
+  case ParentResultType::SPECIFIED: {
+    char const *const hostname = s->parent_result.hostname;
+
+    if (nullptr != hostname) {
+      s->parent_info.name = s->arena.str_store(hostname, strlen(hostname));
+
+      // if host header override option enabled
+      if (host_override(s)) {
+        TxnDbg(dbg_ctl_http_trans, "overriding host header with parent %s", hostname);
+        if (!s->hdr_info.server_request.valid()) {
+          s->hdr_info.client_request.value_set(static_cast<std::string_view>(MIME_FIELD_HOST), hostname);
+          s->hdr_info.client_request.mark_target_dirty();
+        } else {
+          s->hdr_info.server_request.value_set(static_cast<std::string_view>(MIME_FIELD_HOST), hostname);
+          s->hdr_info.server_request.mark_target_dirty();
+        }
+      }
+    }
+
     update_current_info(&s->current, &s->parent_info, ResolveInfo::PARENT_PROXY, false);
     update_dns_info(&s->dns_info, &s->current);
     ink_assert(s->dns_info.looking_up == ResolveInfo::PARENT_PROXY);
     s->next_hop_scheme = URL_WKSIDX_HTTP;
 
     return ResolveInfo::PARENT_PROXY;
+  }
   case ParentResultType::FAIL:
     // No more parents - need to return an error message
     s->current.request_to = ResolveInfo::HOST_NONE;

src/proxy/http/remap/NextHopConsistentHash.cc

@@ -547,8 +547,4 @@ NextHopConsistentHash::findNextHop(TSHttpTxn txnp, void * /* ih ATS_UNUSED */, t
     NH_Dbg(NH_DBG_CTL, "[%" PRIu64 "] result.result: %s set hostname null port 0 retry false", sm_id,
            ParentResultStr[static_cast<int>(result.result)]);
   }
-
-  setHostHeader(txnp, result.hostname);
-
-  return;
 }

src/proxy/http/remap/NextHopRoundRobin.cc

@@ -170,7 +170,6 @@ NextHopRoundRobin::findNextHop(TSHttpTxn txnp, void * /* ih ATS_UNUSED */, time_
       result->last_parent = cur_hst_index;
       result->last_group  = cur_grp_index;
       result->retry       = parentRetry;
-      setHostHeader(txnp, result->hostname);
       ink_assert(result->hostname != nullptr);
       ink_assert(result->port != 0);
       NH_Dbg(NH_DBG_CTL, "[%" PRIu64 "] Chosen parent = %s.%d", sm_id, result->hostname, result->port);

src/proxy/http/remap/NextHopSelectionStrategy.cc

@@ -265,16 +265,6 @@ NextHopSelectionStrategy::markNextHop(TSHttpTxn txnp, const char *hostname, cons
   return passive_health.markNextHop(txnp, hostname, port, status, ih, now);
 }
 
-void
-NextHopSelectionStrategy::setHostHeader(TSHttpTxn txnp, const char *hostname)
-{
-  if (host_override && nullptr != hostname) {
-    HttpSM *sm = reinterpret_cast<HttpSM *>(txnp);
-    sm->t_state.hdr_info.client_request.value_set(static_cast<std::string_view>(MIME_FIELD_HOST), hostname);
-    NH_Dbg(NH_DBG_CTL, "[%" PRIu64 "] overriding host header with parent %s", sm->sm_id, hostname);
-  }
-}
-
 bool
 NextHopSelectionStrategy::nextHopExists(TSHttpTxn txnp, void * /* ih ATS_UNUSED */)
 {