From 6d72f9a984dff177995314652ffa89c58ec02b1a Mon Sep 17 00:00:00 2001 From: ohadvano <49730675+ohadvano@users.noreply.github.com> Date: Thu, 9 Mar 2023 20:57:29 +0200 Subject: [PATCH] Add option to disable TCP tunneling by filter state (#25885) Signed-off-by: Ohad Vano --- .../network/tcp_proxy/v3/tcp_proxy.proto | 2 + changelogs/current.yaml | 3 ++ .../network_filters/tcp_proxy_filter.rst | 12 ++++++ envoy/stream_info/BUILD | 8 ++++ envoy/stream_info/bool_accessor.h | 21 ++++++++++ source/common/stream_info/BUILD | 8 ++++ .../common/stream_info/bool_accessor_impl.h | 30 ++++++++++++++ source/common/tcp_proxy/upstream.h | 2 + source/extensions/upstreams/tcp/generic/BUILD | 1 + .../upstreams/tcp/generic/config.cc | 12 +++++- .../extensions/upstreams/tcp/generic/config.h | 3 ++ test/common/stream_info/BUILD | 8 ++++ .../stream_info/bool_accessor_impl_test.cc | 27 +++++++++++++ test/extensions/upstreams/tcp/generic/BUILD | 1 + .../upstreams/tcp/generic/config_test.cc | 40 +++++++++++++++++++ 15 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 envoy/stream_info/bool_accessor.h create mode 100644 source/common/stream_info/bool_accessor_impl.h create mode 100644 test/common/stream_info/bool_accessor_impl_test.cc diff --git a/api/envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto b/api/envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto index d44fbc83bf89..71ef949448df 100644 --- a/api/envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto +++ b/api/envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto @@ -202,6 +202,8 @@ message TcpProxy { // If set, this configures tunneling, e.g. configuration options to tunnel TCP payload over // HTTP CONNECT. If this message is absent, the payload will be proxied upstream as per usual. + // It is possible to dynamically override this configuration and disable tunneling per connection, + // by setting a per-connection filter state object for the key ``envoy.tcp_proxy.disable_tunneling``. TunnelingConfig tunneling_config = 12; // The maximum duration of a connection. The duration is defined as the period since a connection diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 67c25242627e..8923829c0c25 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -178,6 +178,9 @@ new_features: - area: route change: | support route callback after route matches for :ref:`VirtualHost.matcher `. +- area: tcp_proxy + change: | + added an option to dynamically disable TCP tunneling even if set in the filter config, by setting a filter state object for the key ``envoy.tcp_proxy.disable_tunneling``. deprecated: - area: ext_authz diff --git a/docs/root/configuration/listeners/network_filters/tcp_proxy_filter.rst b/docs/root/configuration/listeners/network_filters/tcp_proxy_filter.rst index c689bd27f383..7917eef4b358 100644 --- a/docs/root/configuration/listeners/network_filters/tcp_proxy_filter.rst +++ b/docs/root/configuration/listeners/network_filters/tcp_proxy_filter.rst @@ -37,6 +37,18 @@ To define metadata that a suitable upstream host must match, use one of the foll In addition, dynamic metadata can be set by earlier network filters on the ``StreamInfo``. Setting the dynamic metadata must happen before ``onNewConnection()`` is called on the ``TcpProxy`` filter to affect load balancing. +.. _config_network_filters_tcp_proxy_tunneling_over_http: + +Tunneling TCP over HTTP +----------------------- + +The TCP proxy filter can be used to tunnel raw TCP over HTTP ``CONNECT`` or HTTP ``POST`` requests. Refer to :ref:`HTTP upgrades ` for more information. + +TCP tunneling configuration can be used by setting :ref:`Tunneling Config ` + +Additionally, if tunneling was enabled for a TCP session by configuration, it can be dynamically disabled per connection, +by setting a per-connection filter state object under the key ``envoy.tcp_proxy.disable_tunneling``. Refer to the implementation for more details. + .. _config_network_filters_tcp_proxy_stats: Statistics diff --git a/envoy/stream_info/BUILD b/envoy/stream_info/BUILD index 0cbf7df2163c..10c253696cac 100644 --- a/envoy/stream_info/BUILD +++ b/envoy/stream_info/BUILD @@ -51,3 +51,11 @@ envoy_cc_library( name = "stream_id_provider_interface", hdrs = ["stream_id_provider.h"], ) + +envoy_cc_library( + name = "bool_accessor_interface", + hdrs = ["bool_accessor.h"], + deps = [ + ":filter_state_interface", + ], +) diff --git a/envoy/stream_info/bool_accessor.h b/envoy/stream_info/bool_accessor.h new file mode 100644 index 000000000000..13a112e110cc --- /dev/null +++ b/envoy/stream_info/bool_accessor.h @@ -0,0 +1,21 @@ +#pragma once + +#include "envoy/common/pure.h" +#include "envoy/stream_info/filter_state.h" + +namespace Envoy { +namespace StreamInfo { + +/** + * A FilterState object that tracks a single boolean value. + */ +class BoolAccessor : public FilterState::Object { +public: + /** + * @return the tracked value. + */ + virtual bool value() const PURE; +}; + +} // namespace StreamInfo +} // namespace Envoy diff --git a/source/common/stream_info/BUILD b/source/common/stream_info/BUILD index 417a23230b10..c683fcbc91d9 100644 --- a/source/common/stream_info/BUILD +++ b/source/common/stream_info/BUILD @@ -71,3 +71,11 @@ envoy_cc_library( "//source/common/common:utility_lib", ], ) + +envoy_cc_library( + name = "bool_accessor_lib", + hdrs = ["bool_accessor_impl.h"], + deps = [ + "//envoy/stream_info:bool_accessor_interface", + ], +) diff --git a/source/common/stream_info/bool_accessor_impl.h b/source/common/stream_info/bool_accessor_impl.h new file mode 100644 index 000000000000..868bfefab953 --- /dev/null +++ b/source/common/stream_info/bool_accessor_impl.h @@ -0,0 +1,30 @@ +#pragma once + +#include "envoy/stream_info/bool_accessor.h" + +namespace Envoy { +namespace StreamInfo { + +/* + * A FilterState object that tracks a single boolean value. + */ +class BoolAccessorImpl : public BoolAccessor { +public: + BoolAccessorImpl(bool value) : value_(value) {} + + // From FilterState::Object + ProtobufTypes::MessagePtr serializeAsProto() const override { + auto message = std::make_unique(); + message->set_value(value_); + return message; + } + + // From BoolAccessor. + bool value() const override { return value_; } + +private: + bool value_; +}; + +} // namespace StreamInfo +} // namespace Envoy diff --git a/source/common/tcp_proxy/upstream.h b/source/common/tcp_proxy/upstream.h index c17d7354127d..452df36f376a 100644 --- a/source/common/tcp_proxy/upstream.h +++ b/source/common/tcp_proxy/upstream.h @@ -15,6 +15,8 @@ namespace Envoy { namespace TcpProxy { +constexpr absl::string_view DisableTunnelingFilterStateKey = "envoy.tcp_proxy.disable_tunneling"; + class TcpConnPool : public GenericConnPool, public Tcp::ConnectionPool::Callbacks { public: TcpConnPool(Upstream::ThreadLocalCluster& thread_local_cluster, diff --git a/source/extensions/upstreams/tcp/generic/BUILD b/source/extensions/upstreams/tcp/generic/BUILD index 673d44aeae31..551a4ca75359 100644 --- a/source/extensions/upstreams/tcp/generic/BUILD +++ b/source/extensions/upstreams/tcp/generic/BUILD @@ -18,6 +18,7 @@ envoy_cc_extension( ], visibility = ["//visibility:public"], deps = [ + "//envoy/stream_info:bool_accessor_interface", "//source/common/http:codec_client_lib", "//source/common/tcp_proxy:upstream_lib", "@envoy_api//envoy/extensions/upstreams/tcp/generic/v3:pkg_cc_proto", diff --git a/source/extensions/upstreams/tcp/generic/config.cc b/source/extensions/upstreams/tcp/generic/config.cc index aadcf96b0cad..ed3770304c78 100644 --- a/source/extensions/upstreams/tcp/generic/config.cc +++ b/source/extensions/upstreams/tcp/generic/config.cc @@ -1,5 +1,6 @@ #include "source/extensions/upstreams/tcp/generic/config.h" +#include "envoy/stream_info/bool_accessor.h" #include "envoy/upstream/cluster_manager.h" #include "source/common/http/codec_client.h" @@ -16,7 +17,7 @@ TcpProxy::GenericConnPoolPtr GenericConnPoolFactory::createGenericConnPool( TcpProxy::TunnelingConfigHelperOptConstRef config, Upstream::LoadBalancerContext* context, Envoy::Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, StreamInfo::StreamInfo& downstream_info) const { - if (config.has_value()) { + if (config.has_value() && !disableTunnelingByFilterState(downstream_info)) { Http::CodecType pool_type; if ((thread_local_cluster.info()->features() & Upstream::ClusterInfo::Features::HTTP2) != 0) { pool_type = Http::CodecType::HTTP2; @@ -35,6 +36,15 @@ TcpProxy::GenericConnPoolPtr GenericConnPoolFactory::createGenericConnPool( return (ret->valid() ? std::move(ret) : nullptr); } +bool GenericConnPoolFactory::disableTunnelingByFilterState( + StreamInfo::StreamInfo& downstream_info) const { + const StreamInfo::BoolAccessor* disable_tunneling = + downstream_info.filterState()->getDataReadOnly( + TcpProxy::DisableTunnelingFilterStateKey); + + return disable_tunneling != nullptr && disable_tunneling->value() == true; +} + REGISTER_FACTORY(GenericConnPoolFactory, TcpProxy::GenericConnPoolFactory); } // namespace Generic diff --git a/source/extensions/upstreams/tcp/generic/config.h b/source/extensions/upstreams/tcp/generic/config.h index dfb0f5528558..c4fee935ef96 100644 --- a/source/extensions/upstreams/tcp/generic/config.h +++ b/source/extensions/upstreams/tcp/generic/config.h @@ -28,6 +28,9 @@ class GenericConnPoolFactory : public TcpProxy::GenericConnPoolFactory { return std::make_unique< envoy::extensions::upstreams::tcp::generic::v3::GenericConnectionPoolProto>(); } + +private: + bool disableTunnelingByFilterState(StreamInfo::StreamInfo& downstream_info) const; }; DECLARE_FACTORY(GenericConnPoolFactory); diff --git a/test/common/stream_info/BUILD b/test/common/stream_info/BUILD index afc7eac5f9ea..cc7c42a1c8bd 100644 --- a/test/common/stream_info/BUILD +++ b/test/common/stream_info/BUILD @@ -77,3 +77,11 @@ envoy_cc_test( "//source/common/stream_info:uint32_accessor_lib", ], ) + +envoy_cc_test( + name = "bool_accessor_impl_test", + srcs = ["bool_accessor_impl_test.cc"], + deps = [ + "//source/common/stream_info:bool_accessor_lib", + ], +) diff --git a/test/common/stream_info/bool_accessor_impl_test.cc b/test/common/stream_info/bool_accessor_impl_test.cc new file mode 100644 index 000000000000..dcc8c5ccc9f6 --- /dev/null +++ b/test/common/stream_info/bool_accessor_impl_test.cc @@ -0,0 +1,27 @@ +#include "source/common/stream_info/bool_accessor_impl.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace StreamInfo { +namespace { + +TEST(BoolAccessorImplTest, FalseValue) { + BoolAccessorImpl accessor(false); + EXPECT_EQ(false, accessor.value()); +} + +TEST(BoolAccessorImplTest, TrueValue) { + BoolAccessorImpl accessor(true); + EXPECT_EQ(true, accessor.value()); +} + +TEST(BoolAccessorImplTest, TestProto) { + BoolAccessorImpl accessor(true); + auto message = accessor.serializeAsProto(); + EXPECT_NE(nullptr, message); +} + +} // namespace +} // namespace StreamInfo +} // namespace Envoy diff --git a/test/extensions/upstreams/tcp/generic/BUILD b/test/extensions/upstreams/tcp/generic/BUILD index 46881ead2768..2a9375294a53 100644 --- a/test/extensions/upstreams/tcp/generic/BUILD +++ b/test/extensions/upstreams/tcp/generic/BUILD @@ -12,6 +12,7 @@ envoy_cc_test( name = "config_test", srcs = ["config_test.cc"], deps = [ + "//source/common/stream_info:bool_accessor_lib", "//source/common/tcp_proxy", "//source/extensions/upstreams/tcp/generic:config", "//test/mocks/server:factory_context_mocks", diff --git a/test/extensions/upstreams/tcp/generic/config_test.cc b/test/extensions/upstreams/tcp/generic/config_test.cc index 4d5239f53d62..10dcc66b126e 100644 --- a/test/extensions/upstreams/tcp/generic/config_test.cc +++ b/test/extensions/upstreams/tcp/generic/config_test.cc @@ -1,3 +1,4 @@ +#include "source/common/stream_info/bool_accessor_impl.h" #include "source/common/tcp_proxy/tcp_proxy.h" #include "source/extensions/upstreams/tcp/generic/config.h" @@ -34,6 +35,45 @@ class TcpConnPoolTest : public ::testing::Test { NiceMock context_; }; +TEST_F(TcpConnPoolTest, TestNoTunnelingConfig) { + EXPECT_CALL(thread_local_cluster_, tcpConnPool(_, _)).WillOnce(Return(absl::nullopt)); + EXPECT_EQ(nullptr, factory_.createGenericConnPool( + thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(), + &lb_context_, callbacks_, downstream_stream_info_)); +} + +TEST_F(TcpConnPoolTest, TestTunnelingDisabledByFilterState) { + envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; + config_proto.set_hostname("host"); + const TcpProxy::TunnelingConfigHelperImpl config(config_proto, context_); + + downstream_stream_info_.filterState()->setData( + TcpProxy::DisableTunnelingFilterStateKey, + std::make_shared(true), + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Connection); + + EXPECT_CALL(thread_local_cluster_, tcpConnPool(_, _)).WillOnce(Return(absl::nullopt)); + EXPECT_EQ(nullptr, factory_.createGenericConnPool( + thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(config), + &lb_context_, callbacks_, downstream_stream_info_)); +} + +TEST_F(TcpConnPoolTest, TestTunnelingNotDisabledIfFilterStateHasFalseValue) { + envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; + config_proto.set_hostname("host"); + const TcpProxy::TunnelingConfigHelperImpl config(config_proto, context_); + + downstream_stream_info_.filterState()->setData( + TcpProxy::DisableTunnelingFilterStateKey, + std::make_shared(false), + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Connection); + + EXPECT_CALL(thread_local_cluster_, httpConnPool(_, _, _)).WillOnce(Return(absl::nullopt)); + EXPECT_EQ(nullptr, factory_.createGenericConnPool( + thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(config), + &lb_context_, callbacks_, downstream_stream_info_)); +} + TEST_F(TcpConnPoolTest, TestNoConnPool) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; config_proto.set_hostname("host");