tls: disable TLS inspector injection

docs/root/version_history/current.rst

@@ -21,6 +21,7 @@ Minor Behavior Changes
 * http: upstream protocol will now only be logged if an upstream stream was established.
 * jwt_authn filter: added support of Jwt time constraint verification with a clock skew (default to 60 seconds) and added a filter config field :ref:`clock_skew_seconds <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtProvider.clock_skew_seconds>` to configure it.
 * kill_request: enable a way to configure kill header name in KillRequest proto.
+* listener: injection of the :ref:`TLS inspector <config_listener_filters_tls_inspector>` has been disabled by default. This feature is controlled by the runtime guard `envoy.reloadable_features.disable_tls_inspector_injection`.
 * memory: enable new tcmalloc with restartable sequences for aarch64 builds.
 * mongo proxy metrics: swapped network connection remote and local closed counters previously set reversed (`cx_destroy_local_with_active_rq` and `cx_destroy_remote_with_active_rq`).
 * performance: improve performance when handling large HTTP/1 bodies.

source/common/runtime/runtime_features.cc

@@ -63,6 +63,7 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.allow_response_for_timeout",
     "envoy.reloadable_features.consume_all_retry_headers",
     "envoy.reloadable_features.check_ocsp_policy",
+    "envoy.reloadable_features.disable_tls_inspector_injection",
     "envoy.reloadable_features.disallow_unbounded_access_logs",
     "envoy.reloadable_features.early_errors_via_hcm",
     "envoy.reloadable_features.enable_dns_cache_circuit_breakers",

source/server/listener_impl.cc

@@ -263,6 +263,8 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
       listener_filters_timeout_(
           PROTOBUF_GET_MS_OR_DEFAULT(config, listener_filters_timeout, 15000)),
       continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()),
+      disable_tls_inspector_injection_(Runtime::runtimeFeatureEnabled(
+          "envoy.reloadable_features.disable_tls_inspector_injection")),
       listener_factory_context_(std::make_shared<PerListenerFactoryContextImpl>(
           parent.server_, validation_visitor_, config, this, *this,
           parent.factory_.createDrainManager(config.drain_type()))),
@@ -342,6 +344,8 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin,
       listener_filters_timeout_(
           PROTOBUF_GET_MS_OR_DEFAULT(config, listener_filters_timeout, 15000)),
       continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()),
+      disable_tls_inspector_injection_(Runtime::runtimeFeatureEnabled(
+          "envoy.reloadable_features.disable_tls_inspector_injection")),
       connection_balancer_(origin.connection_balancer_),
       listener_factory_context_(std::make_shared<PerListenerFactoryContextImpl>(
           origin.listener_factory_context_->listener_factory_context_base_, this, *this)),
@@ -556,6 +560,9 @@ void ListenerImpl::buildProxyProtocolListenerFilter() {
 }
 
 void ListenerImpl::buildTlsInspectorListenerFilter() {
+  if (disable_tls_inspector_injection_) {
+    return;
+  }
   // TODO(zuercher) remove the deprecated TLS inspector name when the deprecated names are removed.
   const bool need_tls_inspector = needTlsInspector(config_);
   // Automatically inject TLS Inspector if it wasn't configured explicitly and it's needed.
@@ -738,7 +745,7 @@ bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::L
   }
 
   // See buildTlsInspectorListenerFilter().
-  if (needTlsInspector(config_) ^ needTlsInspector(config)) {
+  if (!disable_tls_inspector_injection_ && (needTlsInspector(config_) ^ needTlsInspector(config))) {
     return false;
   }
   return ListenerMessageUtil::filterChainOnlyChange(config_, config);

source/server/listener_impl.h

@@ -396,6 +396,7 @@ class ListenerImpl final : public Network::ListenerConfig,
   Network::Socket::OptionsSharedPtr listen_socket_options_;
   const std::chrono::milliseconds listener_filters_timeout_;
   const bool continue_on_listener_filters_timeout_;
+  const bool disable_tls_inspector_injection_;
   Network::ActiveUdpListenerFactoryPtr udp_listener_factory_;
   Network::UdpPacketWriterFactoryPtr udp_writer_factory_;
   Network::UdpListenerWorkerRouterPtr udp_listener_worker_router_;

test/server/listener_manager_impl_test.cc

@@ -3296,7 +3296,33 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithOverlappi
                             "overlapping matching rules are defined");
 }
 
+TEST_F(ListenerManagerImplWithRealFiltersTest, TlsFilterChainWithTlsInspectorInjectionDisabled) {
+  const std::string yaml = TestEnvironment::substitute(R"EOF(
+    address:
+      socket_address: { address: 127.0.0.1, port_value: 1234 }
+    filter_chains:
+    - filter_chain_match:
+        transport_protocol: "tls"
+    - filter_chain_match:
+        # empty
+  )EOF",
+                                                       Network::Address::IpVersion::v4);
+
+  EXPECT_CALL(server_.api_.random_, uuid());
+  EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, {true}));
+  manager_->addOrUpdateListener(parseListenerFromV3Yaml(yaml), "", true);
+  EXPECT_EQ(1U, manager_->listeners().size());
+
+  // Make sure there are no listener filters (i.e. no automatically injected TLS Inspector).
+  Network::ListenerConfig& listener = manager_->listeners().back().get();
+  Network::FilterChainFactory& filterChainFactory = listener.filterChainFactory();
+  Network::MockListenerFilterManager manager;
+  EXPECT_CALL(manager, addAcceptFilter_(_, _)).Times(0);
+  EXPECT_TRUE(filterChainFactory.createListenerFilterChain(manager));
+}
+
 TEST_F(ListenerManagerImplWithRealFiltersTest, TlsFilterChainWithoutTlsInspector) {
+  auto tls_inspector_injection_enabled_guard = enableTlsInspectorInjectionForThisTest();
   const std::string yaml = TestEnvironment::substitute(R"EOF(
     address:
       socket_address: { address: 127.0.0.1, port_value: 1234 }
@@ -3327,6 +3353,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TlsFilterChainWithoutTlsInspector
 // Test the tls inspector is not injected twice when the deprecated name is used.
 TEST_F(ListenerManagerImplWithRealFiltersTest,
        DEPRECATED_FEATURE_TEST(TlsFilterChainWithDeprecatedTlsInspectorName)) {
+  auto tls_inspector_injection_enabled_guard = enableTlsInspectorInjectionForThisTest();
   const std::string yaml = TestEnvironment::substitute(R"EOF(
     address:
       socket_address: { address: 127.0.0.1, port_value: 1234 }
@@ -3358,6 +3385,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest,
 }
 
 TEST_F(ListenerManagerImplWithRealFiltersTest, SniFilterChainWithoutTlsInspector) {
+  auto tls_inspector_injection_enabled_guard = enableTlsInspectorInjectionForThisTest();
   const std::string yaml = TestEnvironment::substitute(R"EOF(
     address:
       socket_address: { address: 127.0.0.1, port_value: 1234 }
@@ -3386,6 +3414,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SniFilterChainWithoutTlsInspector
 }
 
 TEST_F(ListenerManagerImplWithRealFiltersTest, AlpnFilterChainWithoutTlsInspector) {
+  auto tls_inspector_injection_enabled_guard = enableTlsInspectorInjectionForThisTest();
   const std::string yaml = TestEnvironment::substitute(R"EOF(
     address:
       socket_address: { address: 127.0.0.1, port_value: 1234 }
@@ -3414,6 +3443,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, AlpnFilterChainWithoutTlsInspecto
 }
 
 TEST_F(ListenerManagerImplWithRealFiltersTest, CustomTransportProtocolWithSniWithoutTlsInspector) {
+  auto tls_inspector_injection_enabled_guard = enableTlsInspectorInjectionForThisTest();
   const std::string yaml = TestEnvironment::substitute(R"EOF(
     address:
       socket_address: { address: 127.0.0.1, port_value: 1234 }
@@ -4689,6 +4719,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfAn
 
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfImplicitTlsInspectorChanges) {
+  auto tls_inspector_injection_enabled_guard = enableTlsInspectorInjectionForThisTest();
 
   EXPECT_CALL(*worker_, start(_));
   manager_->startWorkers(guard_dog_);

test/server/listener_manager_impl_test.h

@@ -279,6 +279,14 @@ class ListenerManagerImplTest : public testing::Test {
     return scoped_runtime;
   }
 
+  ABSL_MUST_USE_RESULT
+  auto enableTlsInspectorInjectionForThisTest() {
+    auto scoped_runtime = std::make_unique<TestScopedRuntime>();
+    Runtime::LoaderSingleton::getExisting()->mergeValues(
+        {{"envoy.reloadable_features.disable_tls_inspector_injection", "false"}});
+    return scoped_runtime;
+  }
+
   NiceMock<Api::MockOsSysCalls> os_sys_calls_;
   TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls_{&os_sys_calls_};
   Api::OsSysCallsImpl os_sys_calls_actual_;