This is an automated email from the ASF dual-hosted git repository.
maskit 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 94c5b42057 Cleanup SSLSNIConfig (#11027)
94c5b42057 is described below
commit 94c5b42057f87118b467527066cecab406b7ca3f
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Thu Feb 1 09:33:04 2024 -0700
Cleanup SSLSNIConfig (#11027)
SNIConfig::test_client_action is a static function and there's almost no
reason to have it in SSLSNIConfig.
Having it in TLSSNISupport which executes SNI actions makes more sense.
---
include/iocore/net/SSLSNIConfig.h | 3 --
include/iocore/net/TLSSNISupport.h | 1 +
src/iocore/net/SSLSNIConfig.cc | 19 --------
src/iocore/net/TLSSNISupport.cc | 19 ++++++++
src/proxy/http/HttpSM.cc | 89 +++++++++++++++++++-------------------
5 files changed, 65 insertions(+), 66 deletions(-)
diff --git a/include/iocore/net/SSLSNIConfig.h
b/include/iocore/net/SSLSNIConfig.h
index e6832f4bc4..5e4b7c6190 100644
--- a/include/iocore/net/SSLSNIConfig.h
+++ b/include/iocore/net/SSLSNIConfig.h
@@ -143,9 +143,6 @@ public:
*/
static void set_on_reconfigure_callback(std::function<void()> cb);
- static bool test_client_action(const char *servername, uint16_t
dest_incoming_port, const IpEndpoint &ep,
- int &enforcement_policy);
-
private:
static int _configid;
static std::function<void()> on_reconfigure;
diff --git a/include/iocore/net/TLSSNISupport.h
b/include/iocore/net/TLSSNISupport.h
index c13bd6f63e..a4a3de350c 100644
--- a/include/iocore/net/TLSSNISupport.h
+++ b/include/iocore/net/TLSSNISupport.h
@@ -55,6 +55,7 @@ public:
void on_servername(SSL *ssl, int *al, void *arg);
const char *get_sni_server_name() const;
+ bool would_have_actions_for(const char *servername, IpEndpoint remote, int
&enforcement_policy);
struct HintsFromSNI {
std::optional<uint32_t> http2_buffer_water_mark;
diff --git a/src/iocore/net/SSLSNIConfig.cc b/src/iocore/net/SSLSNIConfig.cc
index ac616c2876..357abd9f16 100644
--- a/src/iocore/net/SSLSNIConfig.cc
+++ b/src/iocore/net/SSLSNIConfig.cc
@@ -385,22 +385,3 @@
SNIConfig::set_on_reconfigure_callback(std::function<void()> cb)
{
SNIConfig::on_reconfigure = cb;
}
-
-// See if any of the client-side actions would trigger for this combination of
servername and client IP
-// host_sni_policy is an in/out parameter. It starts with the global policy
from the records.yaml
-// setting proxy.config.http.host_sni_policy and is possibly overridden if the
sni policy
-// contains a host_sni_policy entry
-bool
-SNIConfig::test_client_action(const char *servername, in_port_t
dest_incoming_port, const IpEndpoint &ep, int &host_sni_policy)
-{
- bool retval = false;
- SNIConfig::scoped_config params;
-
- auto const &actions = params->get(servername, dest_incoming_port);
- if (actions.first) {
- for (auto &&item : *actions.first) {
- retval |= item->TestClientSNIAction(servername, ep, host_sni_policy);
- }
- }
- return retval;
-}
diff --git a/src/iocore/net/TLSSNISupport.cc b/src/iocore/net/TLSSNISupport.cc
index 9cb14315db..1bffa9678f 100644
--- a/src/iocore/net/TLSSNISupport.cc
+++ b/src/iocore/net/TLSSNISupport.cc
@@ -167,3 +167,22 @@ TLSSNISupport::_set_sni_server_name(std::string_view name)
_sni_server_name.reset(n);
}
}
+
+// See if any of the client-side actions would trigger for this combination of
servername and client IP
+// host_sni_policy is an in/out parameter. It starts with the global policy
from the records.yaml
+// setting proxy.config.http.host_sni_policy and is possibly overridden if the
sni policy
+// contains a host_sni_policy entry
+bool
+TLSSNISupport::would_have_actions_for(const char *servername, IpEndpoint
remote, int &enforcement_policy)
+{
+ bool retval = false;
+ SNIConfig::scoped_config params;
+
+ auto const &actions = params->get(servername, this->_get_local_port());
+ if (actions.first) {
+ for (auto &&item : *actions.first) {
+ retval |= item->TestClientSNIAction(servername, remote,
enforcement_policy);
+ }
+ }
+ return retval;
+}
diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc
index b3b2aa4ca1..2fbacdacdf 100644
--- a/src/proxy/http/HttpSM.cc
+++ b/src/proxy/http/HttpSM.cc
@@ -46,7 +46,6 @@
#include "proxy/Transform.h"
#include "../../iocore/net/P_SSLConfig.h"
#include "iocore/net/ConnectionTracker.h"
-#include "iocore/net/SSLSNIConfig.h"
#include "iocore/net/TLSALPNSupport.h"
#include "iocore/net/TLSBasicSupport.h"
#include "iocore/net/TLSSNISupport.h"
@@ -4271,51 +4270,53 @@ HttpSM::check_sni_host()
// Issue warning or mark the transaction to be terminated as necessary
int host_len;
const char *host_name = t_state.hdr_info.client_request.host_get(&host_len);
- if (host_name && host_len) {
- if (_ua.get_txn()->support_sni()) {
- int host_sni_policy = t_state.http_config_param->http_host_sni_policy;
- NetVConnection *netvc = _ua.get_txn()->get_netvc();
- if (netvc) {
- IpEndpoint ip = netvc->get_remote_endpoint();
- if (SNIConfig::test_client_action(std::string{host_name,
static_cast<size_t>(host_len)}.c_str(), netvc->get_local_port(),
- ip, host_sni_policy) &&
- host_sni_policy > 0) {
- // In a SNI/Host mismatch where the Host would have triggered SNI
policy, mark the transaction
- // to be considered for rejection after the remap phase passes.
Gives the opportunity to conf_remap
- // override the policy.:w
- //
- //
- // to be rejected
- // in the end_remap logic
- const char *sni_value = netvc->get_server_name();
- const char *action_value = host_sni_policy == 2 ? "terminate" :
"continue";
- if (!sni_value || sni_value[0] == '\0') { // No SNI
- Warning("No SNI for TLS request with hostname %.*s action=%s",
host_len, host_name, action_value);
- SMDbg(dbg_ctl_ssl_sni, "No SNI for TLS request with hostname %.*s
action=%s", host_len, host_name, action_value);
- if (host_sni_policy == 2) {
- swoc::bwprint(error_bw_buffer, "No SNI for TLS request:
connecting to {} for host='{}', returning a 403",
- t_state.client_info.dst_addr,
std::string_view{host_name, static_cast<size_t>(host_len)});
- Log::error("%s", error_bw_buffer.c_str());
- this->t_state.client_connection_enabled = false;
- }
- } else if (strncasecmp(host_name, sni_value, host_len) != 0) { //
Name mismatch
- Warning("SNI/hostname mismatch sni=%s host=%.*s action=%s",
sni_value, host_len, host_name, action_value);
- SMDbg(dbg_ctl_ssl_sni, "SNI/hostname mismatch sni=%s host=%.*s
action=%s", sni_value, host_len, host_name,
- action_value);
- if (host_sni_policy == 2) {
- swoc::bwprint(error_bw_buffer, "SNI/hostname mismatch:
connecting to {} for host='{}' sni='{}', returning a 403",
- t_state.client_info.dst_addr,
std::string_view{host_name, static_cast<size_t>(host_len)}, sni_value);
- Log::error("%s", error_bw_buffer.c_str());
- this->t_state.client_connection_enabled = false;
- }
- } else {
- SMDbg(dbg_ctl_ssl_sni, "SNI/hostname successfully match sni=%s
host=%.*s", sni_value, host_len, host_name);
- }
- } else {
- SMDbg(dbg_ctl_ssl_sni, "No SNI/hostname check configured for
host=%.*s", host_len, host_name);
- }
+
+ if (host_name == nullptr || host_len == 0) {
+ return;
+ }
+
+ auto *netvc = _ua.get_txn()->get_netvc();
+ if (netvc == nullptr) {
+ return;
+ }
+
+ auto *snis = netvc->get_service<TLSSNISupport>();
+ if (snis == nullptr) {
+ return;
+ }
+
+ int host_sni_policy = t_state.http_config_param->http_host_sni_policy;
+ if (snis->would_have_actions_for(std::string{host_name,
static_cast<size_t>(host_len)}.c_str(), netvc->get_remote_endpoint(),
+ host_sni_policy) &&
+ host_sni_policy > 0) {
+ // In a SNI/Host mismatch where the Host would have triggered SNI policy,
mark the transaction
+ // to be considered for rejection after the remap phase passes. Gives the
opportunity to conf_remap
+ // override the policy to be rejected in the end_remap logic
+ const char *sni_value = netvc->get_server_name();
+ const char *action_value = host_sni_policy == 2 ? "terminate" : "continue";
+ if (!sni_value || sni_value[0] == '\0') { // No SNI
+ Warning("No SNI for TLS request with hostname %.*s action=%s", host_len,
host_name, action_value);
+ SMDbg(dbg_ctl_ssl_sni, "No SNI for TLS request with hostname %.*s
action=%s", host_len, host_name, action_value);
+ if (host_sni_policy == 2) {
+ swoc::bwprint(error_bw_buffer, "No SNI for TLS request: connecting to
{} for host='{}', returning a 403",
+ t_state.client_info.dst_addr,
std::string_view{host_name, static_cast<size_t>(host_len)});
+ Log::error("%s", error_bw_buffer.c_str());
+ this->t_state.client_connection_enabled = false;
+ }
+ } else if (strncasecmp(host_name, sni_value, host_len) != 0) { // Name
mismatch
+ Warning("SNI/hostname mismatch sni=%s host=%.*s action=%s", sni_value,
host_len, host_name, action_value);
+ SMDbg(dbg_ctl_ssl_sni, "SNI/hostname mismatch sni=%s host=%.*s
action=%s", sni_value, host_len, host_name, action_value);
+ if (host_sni_policy == 2) {
+ swoc::bwprint(error_bw_buffer, "SNI/hostname mismatch: connecting to
{} for host='{}' sni='{}', returning a 403",
+ t_state.client_info.dst_addr,
std::string_view{host_name, static_cast<size_t>(host_len)}, sni_value);
+ Log::error("%s", error_bw_buffer.c_str());
+ this->t_state.client_connection_enabled = false;
}
+ } else {
+ SMDbg(dbg_ctl_ssl_sni, "SNI/hostname successfully match sni=%s
host=%.*s", sni_value, host_len, host_name);
}
+ } else {
+ SMDbg(dbg_ctl_ssl_sni, "No SNI/hostname check configured for host=%.*s",
host_len, host_name);
}
}
