allow additional network filters for QUIC listeners (envoyproxy#22722)

Signed-off-by: Paul Sohn <paulsohn@google.com>

api/envoy/config/listener/v3/listener_components.proto

@@ -227,6 +227,12 @@ message FilterChain {
   // connections established with the listener. Order matters as the filters are
   // processed sequentially as connection events happen. Note: If the filter
   // list is empty, the connection will close by default.
+  //
+  // For QUIC listeners, network filters other than HTTP Connection Manager (HCM)
+  // can be created, but due to differences in the connection implementation compared
+  // to TCP, the onData() method will never be called. Therefore, network filters
+  // for QUIC listeners should only expect to do work at the start of a new connection
+  // (i.e. in onNewConnection()). HCM must be the last (or only) filter in the chain.
   repeated Filter filters = 3;
 
   // Whether the listener should expect a PROXY protocol V1 header on new

changelogs/current.yaml

@@ -119,5 +119,8 @@ new_features:
 - area: lua
   change: |
     added stats for lua filter, please see :ref:`lua filter stats <config_http_filters_lua_stats>`.
+- area: listener
+  change: |
+    allow network filters other than HTTP Connection Manager to be created for QUIC listeners.
 
 deprecated:

source/server/listener_manager_impl.cc

@@ -977,11 +977,13 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF
   const std::string hcm_str =
       "type.googleapis.com/"
       "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager";
-  if (is_quic && (filter_chain.filters().size() != 1 ||
-                  filter_chain.filters(0).typed_config().type_url() != hcm_str)) {
-    throw EnvoyException(fmt::format(
-        "error building network filter chain for quic listener: requires exactly one http_"
-        "connection_manager filter."));
+  if (is_quic &&
+      (filter_chain.filters().empty() ||
+       filter_chain.filters(filter_chain.filters().size() - 1).typed_config().type_url() !=
+           hcm_str)) {
+    throw EnvoyException(
+        fmt::format("error building network filter chain for quic listener: requires "
+                    "http_connection_manager filter to be last in the chain."));
   }
 #else
   // When QUIC is compiled out it should not be possible to configure either the QUIC transport

test/integration/BUILD

@@ -1926,6 +1926,7 @@ envoy_cc_test(
         "//test/common/quic:test_utils_lib",
         "//test/integration/filters:encoder_decoder_buffer_filter_lib",
         "//test/integration/filters:stream_info_to_headers_filter_lib",
+        "//test/integration/filters:test_network_filter_lib",
         "//test/test_common:test_runtime_lib",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/overload/v3:pkg_cc_proto",

test/integration/filters/BUILD

@@ -815,3 +815,22 @@ envoy_cc_test_library(
         "//source/extensions/filters/http/common:pass_through_filter_lib",
     ],
 )
+
+envoy_proto_library(
+    name = "test_network_filter_proto",
+    srcs = [":test_network_filter.proto"],
+)
+
+envoy_cc_test_library(
+    name = "test_network_filter_lib",
+    srcs = [
+        "test_network_filter.cc",
+    ],
+    deps = [
+        ":test_network_filter_proto_cc_proto",
+        "//envoy/network:filter_interface",
+        "//envoy/registry",
+        "//envoy/server:filter_config_interface",
+        "//source/extensions/filters/network/common:factory_base_lib",
+    ],
+)

test/integration/filters/test_network_filter.cc

@@ -0,0 +1,75 @@
+#include "envoy/network/filter.h"
+
+#include "source/extensions/filters/network/common/factory_base.h"
+
+#include "test/integration/filters/test_network_filter.pb.h"
+#include "test/integration/filters/test_network_filter.pb.validate.h"
+
+namespace Envoy {
+namespace {
+
+/**
+ * All stats for this filter. @see stats_macros.h
+ */
+#define ALL_TEST_NETWORK_FILTER_STATS(COUNTER)                                                     \
+  COUNTER(on_new_connection)                                                                       \
+  COUNTER(on_data)
+
+/**
+ * Struct definition for stats. @see stats_macros.h
+ */
+struct TestNetworkFilterStats {
+  ALL_TEST_NETWORK_FILTER_STATS(GENERATE_COUNTER_STRUCT)
+};
+
+class TestNetworkFilter : public Network::ReadFilter {
+public:
+  TestNetworkFilter(Stats::Scope& scope) : stats_(generateStats("test_network_filter", scope)) {}
+
+  Network::FilterStatus onData(Buffer::Instance&, bool) override {
+    stats_.on_data_.inc();
+    return Network::FilterStatus::Continue;
+  }
+
+  Network::FilterStatus onNewConnection() override {
+    stats_.on_new_connection_.inc();
+    return Network::FilterStatus::Continue;
+  }
+
+  void initializeReadFilterCallbacks(Network::ReadFilterCallbacks& callbacks) override {
+    read_callbacks_ = &callbacks;
+  }
+
+private:
+  Envoy::Network::ReadFilterCallbacks* read_callbacks_{};
+  TestNetworkFilterStats generateStats(const std::string& prefix, Stats::Scope& scope) {
+    return {ALL_TEST_NETWORK_FILTER_STATS(POOL_COUNTER_PREFIX(scope, prefix))};
+  }
+  TestNetworkFilterStats stats_;
+};
+
+class TestNetworkFilterConfigFactory : public Extensions::NetworkFilters::Common::FactoryBase<
+                                           test::integration::filters::TestNetworkFilterConfig> {
+public:
+  TestNetworkFilterConfigFactory()
+      : Extensions::NetworkFilters::Common::FactoryBase<
+            test::integration::filters::TestNetworkFilterConfig>("envoy.test.test_network_filter") {
+  }
+
+private:
+  Network::FilterFactoryCb createFilterFactoryFromProtoTyped(
+      const test::integration::filters::TestNetworkFilterConfig& config,
+      Server::Configuration::FactoryContext& context) override {
+    return [config, &context](Network::FilterManager& filter_manager) -> void {
+      filter_manager.addReadFilter(std::make_shared<TestNetworkFilter>(context.scope()));
+    };
+  }
+};
+
+// perform static registration
+static Registry::RegisterFactory<TestNetworkFilterConfigFactory,
+                                 Server::Configuration::NamedNetworkFilterConfigFactory>
+    register_;
+
+} // namespace
+} // namespace Envoy

test/integration/filters/test_network_filter.proto

@@ -0,0 +1,6 @@
+syntax = "proto3";
+
+package test.integration.filters;
+
+message TestNetworkFilterConfig {
+}

test/integration/quic_http_integration_test.cc

@@ -1145,6 +1145,21 @@ name: "envoy.tls.cert_validator.timed_cert_validator"
   }
 }
 
+TEST_P(QuicHttpIntegrationTest, MultipleNetworkFilters) {
+  config_helper_.addNetworkFilter(R"EOF(
+      name: envoy.test.test_network_filter
+      typed_config:
+        "@type": type.googleapis.com/test.integration.filters.TestNetworkFilterConfig
+)EOF");
+  initialize();
+  codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http")));
+  auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_);
+  waitForNextUpstreamRequest();
+  test_server_->waitForCounterEq("test_network_filter.on_new_connection", 1);
+  EXPECT_EQ(test_server_->counter("test_network_filter.on_data")->value(), 0);
+  codec_client_->close();
+}
+
 class QuicInplaceLdsIntegrationTest : public QuicHttpIntegrationTest {
 public:
   void inplaceInitialize(bool add_default_filter_chain = false) {

test/server/BUILD

@@ -336,6 +336,7 @@ envoy_cc_test(
         ":listener_manager_impl_test_lib",
         ":utility_lib",
         "//source/extensions/filters/http/router:config",
+        "//source/extensions/filters/network/rbac:config",
         "//source/extensions/request_id/uuid:config",
         "//source/extensions/transport_sockets/raw_buffer:config",
         "//source/extensions/transport_sockets/tls:config",

test/server/listener_manager_impl_quic_only_test.cc

@@ -332,8 +332,72 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongCodec) {
 
 #if defined(ENVOY_ENABLE_QUIC)
   EXPECT_THROW_WITH_REGEX(addOrUpdateListener(listener_proto), EnvoyException,
-                          "error building network filter chain for quic listener: requires exactly "
-                          "one http_connection_manager filter.");
+                          "error building network filter chain for quic listener: requires "
+                          "http_connection_manager filter to be last in the chain.");
+#else
+  EXPECT_THROW_WITH_REGEX(addOrUpdateListener(listener_proto), EnvoyException,
+                          "QUIC is configured but not enabled in the build.");
+#endif
+}
+
+TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithNetworkFilterAfterHcm) {
+  std::string yaml = TestEnvironment::substitute(R"EOF(
+address:
+  socket_address:
+    address: 127.0.0.1
+    protocol: UDP
+    port_value: 1234
+filter_chains:
+- filter_chain_match:
+    transport_protocol: "quic"
+  name: foo
+  filters:
+  - name: envoy.filters.network.http_connection_manager
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+      codec_type: HTTP3
+      stat_prefix: hcm
+      route_config:
+        name: local_route
+      http_filters:
+        - name: envoy.filters.http.router
+          typed_config:
+            "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
+  - name: envoy.filters.network.rbac
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC
+      stat_prefix: rbac
+  transport_socket:
+    name: envoy.transport_sockets.quic
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.transport_sockets.quic.v3.QuicDownstreamTransport
+      downstream_tls_context:
+        common_tls_context:
+          tls_certificates:
+          - certificate_chain:
+              filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem"
+            private_key:
+              filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_key.pem"
+          validation_context:
+            trusted_ca:
+              filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem"
+            match_typed_subject_alt_names:
+            - matcher:
+                exact: localhost
+              san_type: URI
+            - matcher:
+                exact: 127.0.0.1
+              san_type: IP_ADDRESS
+udp_listener_config:
+  quic_options: {}
+  )EOF",
+                                                 Network::Address::IpVersion::v4);
+  envoy::config::listener::v3::Listener listener_proto = parseListenerFromV3Yaml(yaml);
+
+#if defined(ENVOY_ENABLE_QUIC)
+  EXPECT_THROW_WITH_REGEX(addOrUpdateListener(listener_proto), EnvoyException,
+                          "error building network filter chain for quic listener: requires "
+                          "http_connection_manager filter to be last in the chain.");
 #else
   EXPECT_THROW_WITH_REGEX(addOrUpdateListener(listener_proto), EnvoyException,
                           "QUIC is configured but not enabled in the build.");