diff --git a/api/envoy/config/listener/v3/listener_components.proto b/api/envoy/config/listener/v3/listener_components.proto index c443453ecdf9..303b5fb564e3 100644 --- a/api/envoy/config/listener/v3/listener_components.proto +++ b/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 diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 4d065543db44..f4cf64c98bdd 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -119,5 +119,8 @@ new_features: - area: lua change: | added stats for lua filter, please see :ref:`lua filter stats `. +- area: listener + change: | + allow network filters other than HTTP Connection Manager to be created for QUIC listeners. deprecated: diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index d388912f0627..9fa6bfb74a20 100644 --- a/source/server/listener_manager_impl.cc +++ b/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 diff --git a/test/integration/BUILD b/test/integration/BUILD index 98a40d7b99e8..7a69c2d66c86 100644 --- a/test/integration/BUILD +++ b/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", diff --git a/test/integration/filters/BUILD b/test/integration/filters/BUILD index 888146f88b6a..652e7914e7d0 100644 --- a/test/integration/filters/BUILD +++ b/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", + ], +) diff --git a/test/integration/filters/test_network_filter.cc b/test/integration/filters/test_network_filter.cc new file mode 100644 index 000000000000..3ea6de271ce2 --- /dev/null +++ b/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(context.scope())); + }; + } +}; + +// perform static registration +static Registry::RegisterFactory + register_; + +} // namespace +} // namespace Envoy diff --git a/test/integration/filters/test_network_filter.proto b/test/integration/filters/test_network_filter.proto new file mode 100644 index 000000000000..ba9cb9e953d0 --- /dev/null +++ b/test/integration/filters/test_network_filter.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; + +package test.integration.filters; + +message TestNetworkFilterConfig { +} diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index b8bb8703c48c..50f69addc3f8 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/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) { diff --git a/test/server/BUILD b/test/server/BUILD index fe22ee73f4bd..d45781a12bf5 100644 --- a/test/server/BUILD +++ b/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", diff --git a/test/server/listener_manager_impl_quic_only_test.cc b/test/server/listener_manager_impl_quic_only_test.cc index 72718ad2b7fd..31be3f24a8bd 100644 --- a/test/server/listener_manager_impl_quic_only_test.cc +++ b/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.");