Add option to disable TCP tunneling by filter state (envoyproxy#25885)

Signed-off-by: Ohad Vano <[email protected]>
lizan · Mar 9, 2023 · 6d72f9a · 6d72f9a
1 parent d5ab0a6
commit 6d72f9a
Show file tree

Hide file tree

Showing 15 changed files with 177 additions and 1 deletion.
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
@@ -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
@@ -178,6 +178,9 @@ new_features:
 - area: route
   change: |
     support route callback after route matches for :ref:`VirtualHost.matcher <envoy_v3_api_field_config.route.v3.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
@@ -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 <tunneling-tcp-over-http>` for more information.
+
+TCP tunneling configuration can be used by setting :ref:`Tunneling Config <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.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
@@ -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
@@ -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
@@ -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
@@ -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<ProtobufWkt::BoolValue>();
+    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
@@ -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
@@ -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
@@ -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<StreamInfo::BoolAccessor>(
+          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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<Server::Configuration::MockFactoryContext> 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<StreamInfo::BoolAccessorImpl>(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<StreamInfo::BoolAccessorImpl>(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");