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 2d34cd33e0 Cleanup: Remove server name stuff from NetVConnection
(#11745)
2d34cd33e0 is described below
commit 2d34cd33e0b77b0ef59a0fcf675bd713687a4d4b
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Mon Sep 16 16:03:46 2024 -0600
Cleanup: Remove server name stuff from NetVConnection (#11745)
* Cleanup: Remove server name stuff from NetVConnection
* Fix a bug
* Maybe it's nullptr vs empty string
* Another try
* Add another empty string check
---
include/iocore/net/NetVConnection.h | 25 ----------------------
include/iocore/net/TLSSNISupport.h | 5 +++++
src/api/InkAPI.cc | 16 +++++++-------
src/iocore/net/CMakeLists.txt | 10 ++-------
src/iocore/net/P_QUICNetVConnection.h | 2 --
src/iocore/net/P_SSLNetVConnection.h | 20 ------------------
src/iocore/net/P_UnixNetVConnection.h | 6 ------
src/iocore/net/QUICNetVConnection.cc | 12 -----------
src/iocore/net/unit_tests/test_Net.cc | 34 ------------------------------
src/proxy/ProxySession.cc | 6 +++++-
src/proxy/http/HttpSM.cc | 25 +++++++++++++++++-----
src/proxy/http/HttpSessionManager.cc | 39 +++++++++++++++++++++--------------
src/proxy/private/SSLProxySession.cc | 5 +++--
13 files changed, 67 insertions(+), 138 deletions(-)
diff --git a/include/iocore/net/NetVConnection.h
b/include/iocore/net/NetVConnection.h
index ff38f853aa..a7d8bd8886 100644
--- a/include/iocore/net/NetVConnection.h
+++ b/include/iocore/net/NetVConnection.h
@@ -163,15 +163,6 @@ public:
*/
void do_io_shutdown(ShutdownHowTo_t howto) override = 0;
- /**
- Return the server name that is appropriate for the network VC type
- */
- virtual const char *
- get_server_name() const
- {
- return nullptr;
- }
-
////////////////////////////////////////////////////////////
// Set the timeouts associated with this connection. //
// active_timeout is for the total elapsed time of //
@@ -327,22 +318,6 @@ public:
return netvc_context;
}
- /**
- * Returns true if the network protocol
- * supports a client provided SNI value
- */
- virtual bool
- support_sni() const
- {
- return false;
- }
-
- virtual const char *
- get_sni_servername() const
- {
- return nullptr;
- }
-
virtual bool
peer_provided_cert() const
{
diff --git a/include/iocore/net/TLSSNISupport.h
b/include/iocore/net/TLSSNISupport.h
index 2f1277667d..1b35a683cf 100644
--- a/include/iocore/net/TLSSNISupport.h
+++ b/include/iocore/net/TLSSNISupport.h
@@ -61,6 +61,11 @@ public:
void on_client_hello(ClientHello &client_hello);
void on_servername(SSL *ssl, int *al, void *arg);
+ /**
+ * Get the server name in SNI
+ *
+ * @return Either a pointer to the (null-terminated) name fetched from the
TLS object or the empty string.
+ */
const char *get_sni_server_name() const;
bool would_have_actions_for(const char *servername, IpEndpoint
remote, int &enforcement_policy);
diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc
index 089772d22f..6de5a09bab 100644
--- a/src/api/InkAPI.cc
+++ b/src/api/InkAPI.cc
@@ -7928,17 +7928,17 @@ TSVConnFdGet(TSVConn vconnp)
const char *
TSVConnSslSniGet(TSVConn sslp, int *length)
{
- char const *server_name = nullptr;
- NetVConnection *vc = reinterpret_cast<NetVConnection *>(sslp);
-
- if (vc == nullptr) {
+ if (sslp == nullptr) {
return nullptr;
}
- server_name = vc->get_server_name();
-
- if (length) {
- *length = server_name ? strlen(server_name) : 0;
+ char const *server_name = nullptr;
+ NetVConnection *vc = reinterpret_cast<NetVConnection *>(sslp);
+ if (auto snis = vc->get_service<TLSSNISupport>(); snis) {
+ server_name = snis->get_sni_server_name();
+ if (length) {
+ *length = server_name ? strlen(server_name) : 0;
+ }
}
return server_name;
diff --git a/src/iocore/net/CMakeLists.txt b/src/iocore/net/CMakeLists.txt
index c9a5c5147d..7d925af515 100644
--- a/src/iocore/net/CMakeLists.txt
+++ b/src/iocore/net/CMakeLists.txt
@@ -118,14 +118,8 @@ endif()
if(BUILD_TESTING)
add_executable(
- test_net
- libinknet_stub.cc
- NetVCTest.cc
- unit_tests/test_ProxyProtocol.cc
- unit_tests/test_SSLSNIConfig.cc
- unit_tests/test_YamlSNIConfig.cc
- unit_tests/unit_test_main.cc
- unit_tests/test_Net.cc
+ test_net libinknet_stub.cc NetVCTest.cc unit_tests/test_ProxyProtocol.cc
unit_tests/test_SSLSNIConfig.cc
+ unit_tests/test_YamlSNIConfig.cc unit_tests/unit_test_main.cc
)
target_link_libraries(test_net PRIVATE ts::inknet catch2::catch2)
set(LIBINKNET_UNIT_TEST_DIR "${CMAKE_SOURCE_DIR}/src/iocore/net/unit_tests")
diff --git a/src/iocore/net/P_QUICNetVConnection.h
b/src/iocore/net/P_QUICNetVConnection.h
index 644bc6f987..fd1ae29938 100644
--- a/src/iocore/net/P_QUICNetVConnection.h
+++ b/src/iocore/net/P_QUICNetVConnection.h
@@ -110,8 +110,6 @@ public:
// NetVConnection
int populate_protocol(std::string_view *results, int n) const
override;
const char *protocol_contains(std::string_view tag) const override;
- const char *get_server_name() const override;
- bool support_sni() const override;
// QUICConnection
QUICStreamManager *stream_manager() override;
diff --git a/src/iocore/net/P_SSLNetVConnection.h
b/src/iocore/net/P_SSLNetVConnection.h
index fb5150c20a..0f54f75fa0 100644
--- a/src/iocore/net/P_SSLNetVConnection.h
+++ b/src/iocore/net/P_SSLNetVConnection.h
@@ -317,20 +317,6 @@ public:
std::shared_ptr<SSL_SESSION> client_sess = nullptr;
- // The serverName is either a pointer to the (null-terminated) name fetched
from the
- // SSL object or the empty string.
- const char *
- get_server_name() const override
- {
- return get_sni_server_name() ? get_sni_server_name() : "";
- }
-
- bool
- support_sni() const override
- {
- return true;
- }
-
/// Set by asynchronous hooks to request a specific operation.
SslVConnOp hookOpRequested = SSL_HOOK_OP_DEFAULT;
@@ -356,12 +342,6 @@ public:
verify_cert = ctx;
}
- const char *
- get_sni_servername() const override
- {
- return SSL_get_servername(this->ssl, TLSEXT_NAMETYPE_host_name);
- }
-
bool
peer_provided_cert() const override
{
diff --git a/src/iocore/net/P_UnixNetVConnection.h
b/src/iocore/net/P_UnixNetVConnection.h
index f685528d65..a6ce3ab89e 100644
--- a/src/iocore/net/P_UnixNetVConnection.h
+++ b/src/iocore/net/P_UnixNetVConnection.h
@@ -60,12 +60,6 @@ public:
bool get_data(int id, void *data) override;
- const char *
- get_server_name() const override
- {
- return nullptr;
- }
-
void do_io_close(int lerrno = -1) override;
void do_io_shutdown(ShutdownHowTo_t howto) override;
diff --git a/src/iocore/net/QUICNetVConnection.cc
b/src/iocore/net/QUICNetVConnection.cc
index 44b65ebacf..d20330360e 100644
--- a/src/iocore/net/QUICNetVConnection.cc
+++ b/src/iocore/net/QUICNetVConnection.cc
@@ -755,18 +755,6 @@ QUICNetVConnection::protocol_contains(std::string_view
prefix) const
return retval;
}
-const char *
-QUICNetVConnection::get_server_name() const
-{
- return get_sni_server_name();
-}
-
-bool
-QUICNetVConnection::support_sni() const
-{
- return true;
-}
-
QUICConnection *
QUICNetVConnection::get_quic_connection()
{
diff --git a/src/iocore/net/unit_tests/test_Net.cc
b/src/iocore/net/unit_tests/test_Net.cc
deleted file mode 100644
index f1a4d630b3..0000000000
--- a/src/iocore/net/unit_tests/test_Net.cc
+++ /dev/null
@@ -1,34 +0,0 @@
-/** @file
-
- Catch based unit tests for inknet
-
- @section license License
-
- Licensed to the Apache Software Foundation (ASF) under one
- or more contributor license agreements. See the NOTICE file
- distributed with this work for additional information
- regarding copyright ownership. The ASF licenses this file
- to you under the Apache License, Version 2.0 (the
- "License"); you may not use this file except in compliance
- with the License. You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- 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 "iocore/net/NetProcessor.h"
-#include "iocore/net/NetVConnection.h"
-
-#include <catch.hpp>
-
-TEST_CASE("When we allocate a VC, it should not have a server name yet.")
-{
- NetVConnection *vc{netProcessor.allocate_vc(this_ethread())};
- CHECK(nullptr == vc->get_server_name());
- vc->do_io_close();
-}
diff --git a/src/proxy/ProxySession.cc b/src/proxy/ProxySession.cc
index a6516e030f..fd1620b409 100644
--- a/src/proxy/ProxySession.cc
+++ b/src/proxy/ProxySession.cc
@@ -335,7 +335,11 @@ ProxySession::reenable(VIO *vio)
bool
ProxySession::support_sni() const
{
- return _vc ? _vc->support_sni() : false;
+ if (this->_vc) {
+ return this->_vc->get_service<TLSSNISupport>() != nullptr;
+ } else {
+ return false;
+ }
}
PoolableSession *
diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc
index 209b5ae100..959cc4bf3b 100644
--- a/src/proxy/http/HttpSM.cc
+++ b/src/proxy/http/HttpSM.cc
@@ -501,7 +501,11 @@ HttpSM::setup_blind_tunnel_port()
t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
}
} else {
-
t_state.hdr_info.client_request.url_get()->host_set(netvc->get_server_name(),
strlen(netvc->get_server_name()));
+ const char *server_name = "";
+ if (auto *snis = netvc->get_service<TLSSNISupport>(); snis) {
+ server_name = snis->get_sni_server_name();
+ }
+ t_state.hdr_info.client_request.url_get()->host_set(server_name,
strlen(server_name));
t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
}
}
@@ -1375,7 +1379,11 @@ plugins required to work with sni_routing.
t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
}
} else {
-
t_state.hdr_info.client_request.url_get()->host_set(netvc->get_server_name(),
strlen(netvc->get_server_name()));
+ const char *server_name = "";
+ if (auto *snis = netvc->get_service<TLSSNISupport>(); snis) {
+ server_name = snis->get_sni_server_name();
+ }
+ t_state.hdr_info.client_request.url_get()->host_set(server_name,
strlen(server_name));
t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
}
}
@@ -4318,7 +4326,7 @@ HttpSM::check_sni_host()
// 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 *sni_value = snis->get_sni_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);
@@ -5132,9 +5140,11 @@ HttpSM::get_outbound_sni() const
swoc::TextView zret;
swoc::TextView policy{t_state.txn_conf->ssl_client_sni_policy,
swoc::TextView::npos};
+ TLSSNISupport *snis = nullptr;
if (_ua.get_txn()) {
if (auto *netvc = _ua.get_txn()->get_netvc(); netvc) {
- if (auto *snis = netvc->get_service<TLSSNISupport>(); snis &&
snis->hints_from_sni.outbound_sni_policy.has_value()) {
+ snis = netvc->get_service<TLSSNISupport>();
+ if (snis && snis->hints_from_sni.outbound_sni_policy.has_value()) {
policy.assign(snis->hints_from_sni.outbound_sni_policy->data(),
swoc::TextView::npos);
}
}
@@ -5146,7 +5156,12 @@ HttpSM::get_outbound_sni() const
char const *ptr = t_state.hdr_info.server_request.host_get(&len);
zret.assign(ptr, len);
} else if (_ua.get_txn() && policy == "server_name"_tv) {
- zret.assign(_ua.get_txn()->get_netvc()->get_server_name(),
swoc::TextView::npos);
+ const char *server_name = snis->get_sni_server_name();
+ if (server_name[0] == '\0') {
+ zret.assign(nullptr, swoc::TextView::npos);
+ } else {
+ zret.assign(snis->get_sni_server_name(), swoc::TextView::npos);
+ }
} else if (policy.front() == '@') { // guaranteed non-empty from previous
clause
zret = policy.remove_prefix(1);
} else {
diff --git a/src/proxy/http/HttpSessionManager.cc
b/src/proxy/http/HttpSessionManager.cc
index db0a8a074c..1c04d9cb13 100644
--- a/src/proxy/http/HttpSessionManager.cc
+++ b/src/proxy/http/HttpSessionManager.cc
@@ -34,6 +34,7 @@
#include "proxy/ProxySession.h"
#include "proxy/http/HttpSM.h"
#include "proxy/http/HttpDebugNames.h"
+#include "iocore/net/TLSSNISupport.h"
#include <iterator>
namespace
@@ -91,14 +92,18 @@ ServerSessionPool::validate_host_sni(HttpSM *sm,
NetVConnection *netvc)
// by fetching the hostname from the server request. So the connection
should only
// be reused if the hostname in the new request is the same as the host
name in the
// original request
- const char *session_sni = netvc->get_sni_servername();
- if (session_sni) {
- // TS-4468: If the connection matches, make sure the SNI server
- // name (if present) matches the request hostname
- int len = 0;
- const char *req_host =
sm->t_state.hdr_info.server_request.host_get(&len);
- retval = strncasecmp(session_sni, req_host, len) == 0;
- Dbg(dbg_ctl_http_ss, "validate_host_sni host=%*.s, sni=%s", len,
req_host, session_sni);
+ if (auto snis = netvc->get_service<TLSSNISupport>(); snis) {
+ const char *session_sni = snis->get_sni_server_name();
+ if (session_sni && session_sni[0] != '\0') {
+ // TS-4468: If the connection matches, make sure the SNI server
+ // name (if present) matches the request hostname
+ int len = 0;
+ const char *req_host =
sm->t_state.hdr_info.server_request.host_get(&len);
+ retval = strncasecmp(session_sni, req_host, len) == 0;
+ Dbg(dbg_ctl_http_ss, "validate_host_sni host=%*.s, sni=%s", len,
req_host, session_sni);
+ }
+ } else {
+ retval = false;
}
}
return retval;
@@ -112,14 +117,18 @@ ServerSessionPool::validate_sni(HttpSM *sm,
NetVConnection *netvc)
// a new connection.
//
if (sm->t_state.scheme == URL_WKSIDX_HTTPS) {
- const char *session_sni = netvc->get_sni_servername();
- std::string_view proposed_sni = sm->get_outbound_sni();
- Dbg(dbg_ctl_http_ss, "validate_sni proposed_sni=%.*s, sni=%s",
static_cast<int>(proposed_sni.length()), proposed_sni.data(),
- session_sni);
- if (!session_sni || proposed_sni.length() == 0) {
- retval = session_sni == nullptr && proposed_sni.length() == 0;
+ if (auto snis = netvc->get_service<TLSSNISupport>(); snis) {
+ const char *session_sni = snis->get_sni_server_name();
+ std::string_view proposed_sni = sm->get_outbound_sni();
+ Dbg(dbg_ctl_http_ss, "validate_sni proposed_sni=%.*s, sni=%s",
static_cast<int>(proposed_sni.length()), proposed_sni.data(),
+ session_sni);
+ if (!session_sni || session_sni[0] == '\0' || proposed_sni.length() ==
0) {
+ retval = session_sni == nullptr && proposed_sni.length() == 0;
+ } else {
+ retval = proposed_sni.compare(session_sni) == 0;
+ }
} else {
- retval = proposed_sni.compare(session_sni) == 0;
+ retval = false;
}
}
return retval;
diff --git a/src/proxy/private/SSLProxySession.cc
b/src/proxy/private/SSLProxySession.cc
index 3779b73a28..bfc01d098f 100644
--- a/src/proxy/private/SSLProxySession.cc
+++ b/src/proxy/private/SSLProxySession.cc
@@ -23,14 +23,15 @@
#include "SSLProxySession.h"
#include "iocore/net/NetVConnection.h"
+#include "iocore/net/TLSSNISupport.h"
class TLSSNISupport;
void
SSLProxySession::init(NetVConnection const &new_vc)
{
- if (new_vc.get_service<TLSSNISupport>() != nullptr) {
- if (char const *name = new_vc.get_server_name()) {
+ if (auto *snis = new_vc.get_service<TLSSNISupport>(); snis != nullptr) {
+ if (char const *name = snis->get_sni_server_name()) {
_client_sni_server_name.assign(name);
}
}
