From 700f869d5ed0ba0018226f78a9610cb33acb32a9 Mon Sep 17 00:00:00 2001 From: Zhengxi Li Date: Fri, 7 Apr 2023 02:34:47 +0000 Subject: [PATCH 1/5] Add support for tunnel route config with match group and variable --- doc/admin-guide/files/sni.yaml.en.rst | 2 ++ iocore/net/P_SNIActionPerformer.h | 36 +++++++++++++++-------- tests/gold_tests/tls/tls_tunnel.test.py | 39 +++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/doc/admin-guide/files/sni.yaml.en.rst b/doc/admin-guide/files/sni.yaml.en.rst index 78fd0df1177..538fd8e1bf1 100644 --- a/doc/admin-guide/files/sni.yaml.en.rst +++ b/doc/admin-guide/files/sni.yaml.en.rst @@ -176,6 +176,8 @@ tunnel_route Inbound Destination as an FQDN and port, separated b Protocol ` for more information on Proxy Protocol and how it is configured for |TS|. + Note that the match group number can be used in combination with the ``{inbound_local_port}`` and ``{proxy_protocol_port}`` literal strings. + For each of these tunnel targets, unless the port is explicitly specified in the target (e.g., if the port is derived from the Proxy Protocol header), the port must be specified in the :ts:cv:`proxy.config.http.connect_ports` configuration in order for diff --git a/iocore/net/P_SNIActionPerformer.h b/iocore/net/P_SNIActionPerformer.h index 3ef1cd3f14e..a15d7fe3378 100644 --- a/iocore/net/P_SNIActionPerformer.h +++ b/iocore/net/P_SNIActionPerformer.h @@ -95,7 +95,6 @@ class TunnelDestination : public ActionItem // ID of the configured variable. This will be used to know which function // should be called when processing the tunnel destination. enum OpId : int32_t { - DEFAULT = -1, // No specific variable set. MATCH_GROUPS, // Deal with configured groups. MAP_WITH_RECV_PORT, // Use port from inbound local MAP_WITH_PROXY_PROTOCOL_PORT, // Use port from the proxy protocol @@ -109,12 +108,18 @@ class TunnelDestination : public ActionItem const std::vector &alpn) : destination(dest), type(type), tunnel_prewarm(prewarm), alpn_ids(alpn) { - if (destination.find_first_of('$') != std::string::npos) { - fnArrIndex = OpId::MATCH_GROUPS; - } else if (var_start_pos = destination.find(MAP_WITH_RECV_PORT_STR); var_start_pos != std::string::npos) { - fnArrIndex = OpId::MAP_WITH_RECV_PORT; + // Check for port variable specification. Note that only one of these can be + // present in the config. Also note that this is checked before the match + // group so that the corresponding function can be applied before the match + // group expansion(when the var_start_pos is still accurate). + if (var_start_pos = destination.find(MAP_WITH_RECV_PORT_STR); var_start_pos != std::string::npos) { + fnArrIndexes.push_back(OpId::MAP_WITH_RECV_PORT); } else if (var_start_pos = destination.find(MAP_WITH_PROXY_PROTOCOL_PORT_STR); var_start_pos != std::string::npos) { - fnArrIndex = OpId::MAP_WITH_PROXY_PROTOCOL_PORT; + fnArrIndexes.push_back(OpId::MAP_WITH_PROXY_PROTOCOL_PORT); + } + // Check for match groups as well. + if (destination.find_first_of('$') != std::string::npos) { + fnArrIndexes.push_back(OpId::MATCH_GROUPS); } } ~TunnelDestination() override {} @@ -126,13 +131,17 @@ class TunnelDestination : public ActionItem SSLNetVConnection *ssl_netvc = dynamic_cast(snis); const char *servername = snis->get_sni_server_name(); if (ssl_netvc) { - if (fnArrIndex == OpId::DEFAULT) { + if (fnArrIndexes.empty()) { ssl_netvc->set_tunnel_destination(destination, type, !TLSTunnelSupport::PORT_IS_DYNAMIC, tunnel_prewarm); Debug("ssl_sni", "Destination now is [%s], fqdn [%s]", destination.c_str(), servername); } else { - // Dispatch to the correct tunnel destination port function. - bool port_is_dynamic = false; - const auto &fixed_dst = fix_destination[fnArrIndex](destination, var_start_pos, ctx, ssl_netvc, port_is_dynamic); + bool port_is_dynamic = false; + auto fixed_dst{destination}; + // Apply mapping functions to get the final destination. + for (auto fnArrIndex : fnArrIndexes) { + // Dispatch to the correct tunnel destination port function. + fixed_dst = fix_destination[fnArrIndex](fixed_dst, var_start_pos, ctx, ssl_netvc, port_is_dynamic); + } ssl_netvc->set_tunnel_destination(fixed_dst, type, port_is_dynamic, tunnel_prewarm); Debug("ssl_sni", "Destination now is [%s], configured [%s], fqdn [%s]", fixed_dst.c_str(), destination.c_str(), servername); } @@ -232,8 +241,11 @@ class TunnelDestination : public ActionItem YamlSNIConfig::TunnelPreWarm tunnel_prewarm = YamlSNIConfig::TunnelPreWarm::UNSET; const std::vector &alpn_ids; - OpId fnArrIndex{OpId::DEFAULT}; /// On creation, we decide which function needs to be called, set the index and then we - /// call it with the relevant data + // The indexes of the mapping functions that need to be called. On + // creation, we decide which functions need to be called, add the + // coressponding indexes and then we call those functions with the relevant + // data. + std::vector fnArrIndexes; /// tunnel_route destination callback array. static std::array Date: Mon, 10 Apr 2023 16:22:37 +0000 Subject: [PATCH 2/5] Updated comment format --- iocore/net/P_SNIActionPerformer.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/iocore/net/P_SNIActionPerformer.h b/iocore/net/P_SNIActionPerformer.h index a15d7fe3378..0a887989187 100644 --- a/iocore/net/P_SNIActionPerformer.h +++ b/iocore/net/P_SNIActionPerformer.h @@ -241,10 +241,10 @@ class TunnelDestination : public ActionItem YamlSNIConfig::TunnelPreWarm tunnel_prewarm = YamlSNIConfig::TunnelPreWarm::UNSET; const std::vector &alpn_ids; - // The indexes of the mapping functions that need to be called. On - // creation, we decide which functions need to be called, add the - // coressponding indexes and then we call those functions with the relevant - // data. + /** The indexes of the mapping functions that need to be called. On + creation, we decide which functions need to be called, add the coressponding + indexes and then we call those functions with the relevant data. + */ std::vector fnArrIndexes; /// tunnel_route destination callback array. From d3142a0d272c21fc73dca30f03ededad0c3b2efd Mon Sep 17 00:00:00 2001 From: Zhengxi Li Date: Fri, 14 Apr 2023 17:30:21 +0000 Subject: [PATCH 3/5] Added an error message when both port variables are used --- doc/admin-guide/files/sni.yaml.en.rst | 2 +- iocore/net/P_SNIActionPerformer.h | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/doc/admin-guide/files/sni.yaml.en.rst b/doc/admin-guide/files/sni.yaml.en.rst index 538fd8e1bf1..18d5bdc98fb 100644 --- a/doc/admin-guide/files/sni.yaml.en.rst +++ b/doc/admin-guide/files/sni.yaml.en.rst @@ -176,7 +176,7 @@ tunnel_route Inbound Destination as an FQDN and port, separated b Protocol ` for more information on Proxy Protocol and how it is configured for |TS|. - Note that the match group number can be used in combination with the ``{inbound_local_port}`` and ``{proxy_protocol_port}`` literal strings. + Note that only one of the ``{inbound_local_port}`` and ``{proxy_protocol_port}`` literal strings can be specified. The match group number can be used in combination with either one of those. For each of these tunnel targets, unless the port is explicitly specified in the target (e.g., if the port is derived from the Proxy Protocol header), the port must be diff --git a/iocore/net/P_SNIActionPerformer.h b/iocore/net/P_SNIActionPerformer.h index 0a887989187..3324d57c48e 100644 --- a/iocore/net/P_SNIActionPerformer.h +++ b/iocore/net/P_SNIActionPerformer.h @@ -41,6 +41,7 @@ #include "tscore/ink_inet.h" +#include #include class ControlH2 : public ActionItem @@ -108,14 +109,20 @@ class TunnelDestination : public ActionItem const std::vector &alpn) : destination(dest), type(type), tunnel_prewarm(prewarm), alpn_ids(alpn) { - // Check for port variable specification. Note that only one of these can be - // present in the config. Also note that this is checked before the match - // group so that the corresponding function can be applied before the match - // group expansion(when the var_start_pos is still accurate). - if (var_start_pos = destination.find(MAP_WITH_RECV_PORT_STR); var_start_pos != std::string::npos) { - fnArrIndexes.push_back(OpId::MAP_WITH_RECV_PORT); - } else if (var_start_pos = destination.find(MAP_WITH_PROXY_PROTOCOL_PORT_STR); var_start_pos != std::string::npos) { - fnArrIndexes.push_back(OpId::MAP_WITH_PROXY_PROTOCOL_PORT); + // Check for port variable specification. Note that this is checked before + // the match group so that the corresponding function can be applied before + // the match group expansion(when the var_start_pos is still accurate). + auto recv_port_start_pos = destination.find(MAP_WITH_RECV_PORT_STR); + auto pp_port_start_pos = destination.find(MAP_WITH_PROXY_PROTOCOL_PORT_STR); + bool has_recv_port_var = recv_port_start_pos != std::string::npos; + bool has_pp_port_var = pp_port_start_pos != std::string::npos; + if (has_recv_port_var && has_pp_port_var) { + Error("Invalid destination \"%.*s\" in SNI configuration - Only one port variable can be specified.", + static_cast(destination.size()), destination.data()); + } else if (has_recv_port_var || has_pp_port_var) { + // Specified only one of the port variables. + fnArrIndexes.push_back(has_recv_port_var ? OpId::MAP_WITH_RECV_PORT : OpId::MAP_WITH_PROXY_PROTOCOL_PORT); + var_start_pos = has_recv_port_var ? recv_port_start_pos : pp_port_start_pos; } // Check for match groups as well. if (destination.find_first_of('$') != std::string::npos) { From 6d2cef0000e63263c2ef854533853c5ad5bc8e68 Mon Sep 17 00:00:00 2001 From: Zhengxi Li Date: Fri, 14 Apr 2023 18:48:12 +0000 Subject: [PATCH 4/5] remove unnecessary header --- iocore/net/P_SNIActionPerformer.h | 1 - 1 file changed, 1 deletion(-) diff --git a/iocore/net/P_SNIActionPerformer.h b/iocore/net/P_SNIActionPerformer.h index 3324d57c48e..f0547856b5b 100644 --- a/iocore/net/P_SNIActionPerformer.h +++ b/iocore/net/P_SNIActionPerformer.h @@ -41,7 +41,6 @@ #include "tscore/ink_inet.h" -#include #include class ControlH2 : public ActionItem From 0c064a795708e00019881eae8750665267ca35c7 Mon Sep 17 00:00:00 2001 From: Zhengxi Li Date: Fri, 14 Apr 2023 19:18:39 +0000 Subject: [PATCH 5/5] improved readability --- iocore/net/P_SNIActionPerformer.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/iocore/net/P_SNIActionPerformer.h b/iocore/net/P_SNIActionPerformer.h index f0547856b5b..4d30026ad9f 100644 --- a/iocore/net/P_SNIActionPerformer.h +++ b/iocore/net/P_SNIActionPerformer.h @@ -118,10 +118,12 @@ class TunnelDestination : public ActionItem if (has_recv_port_var && has_pp_port_var) { Error("Invalid destination \"%.*s\" in SNI configuration - Only one port variable can be specified.", static_cast(destination.size()), destination.data()); - } else if (has_recv_port_var || has_pp_port_var) { - // Specified only one of the port variables. - fnArrIndexes.push_back(has_recv_port_var ? OpId::MAP_WITH_RECV_PORT : OpId::MAP_WITH_PROXY_PROTOCOL_PORT); - var_start_pos = has_recv_port_var ? recv_port_start_pos : pp_port_start_pos; + } else if (has_recv_port_var) { + fnArrIndexes.push_back(OpId::MAP_WITH_RECV_PORT); + var_start_pos = recv_port_start_pos; + } else if (has_pp_port_var) { + fnArrIndexes.push_back(OpId::MAP_WITH_PROXY_PROTOCOL_PORT); + var_start_pos = pp_port_start_pos; } // Check for match groups as well. if (destination.find_first_of('$') != std::string::npos) {