From 0b16c30ffb0ead856bf1a8de0025eb3e88489217 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 22 Jul 2021 12:16:24 -0400 Subject: [PATCH 1/5] http3: validating codec Signed-off-by: Alyssa Wilk --- source/server/listener_manager_impl.cc | 10 +++ test/server/BUILD | 2 + .../listener_manager_impl_quic_only_test.cc | 65 ++++++++++++++++++- 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 98814eea12918..74a1c9d5252c1 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -930,6 +930,16 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF "{}. \nUse QuicDownstreamTransport instead.", transport_socket.DebugString())); } + const std::string config_str = + filter_chain.filters_size() == 0 ? "" : filter_chain.filters(0).DebugString(); + const std::string hcm_str = + "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; + if (is_quic && (filter_chain.filters().size() != 1 || !absl::StrContains(config_str, hcm_str) || + !absl::StrContains(config_str, "codec_type: HTTP3"))) { + throw EnvoyException(fmt::format( + "error building network filter chain for quic listener: requires exactly one http_" + "connection_manager filter with an HTTP/3 codec.")); + } #else // When QUIC is compiled out it should not be possible to configure either the QUIC transport // socket or the QUIC listener and get to this point. diff --git a/test/server/BUILD b/test/server/BUILD index 15fdece73619b..04d1e2248ff63 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -322,6 +322,8 @@ envoy_cc_test( deps = [ ":listener_manager_impl_test_lib", ":utility_lib", + "//source/extensions/filters/http/router:config", + "//source/extensions/request_id/uuid:config", "//source/extensions/transport_sockets/raw_buffer:config", "//source/extensions/transport_sockets/tls:config", "//test/test_common:threadsafe_singleton_injector_lib", diff --git a/test/server/listener_manager_impl_quic_only_test.cc b/test/server/listener_manager_impl_quic_only_test.cc index 6f1f08fe4fa78..3b695f63c70d0 100644 --- a/test/server/listener_manager_impl_quic_only_test.cc +++ b/test/server/listener_manager_impl_quic_only_test.cc @@ -36,7 +36,16 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryAndSslContext) { filter_chains: - filter_chain_match: transport_protocol: "quic" - filters: [] + 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 transport_socket: name: envoy.transport_sockets.quic typed_config: @@ -171,6 +180,60 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongTransportSoc #endif } +TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongCodec) { + const 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" + 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: HTTP2 + stat_prefix: hcm + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.router + 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_subject_alt_names: + - exact: localhost + - exact: 127.0.0.1 +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(manager_->addOrUpdateListener(listener_proto, "", true), EnvoyException, + "error building network filter chain for quic listener: requires exactly " + "one http_connection_manager filter with an HTTP/3 codec."); +#else + EXPECT_THROW_WITH_REGEX(manager_->addOrUpdateListener(listener_proto, "", true), EnvoyException, + "QUIC is configured but not enabled in the build."); +#endif +} + } // namespace } // namespace Server } // namespace Envoy From ce3a6ed8c8a1bde35d1bf2800e45586b0ce74ddd Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 26 Jul 2021 10:43:46 -0400 Subject: [PATCH 2/5] comments Signed-off-by: Alyssa Wilk --- source/server/listener_manager_impl.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 74a1c9d5252c1..2924b5e41b920 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -933,8 +933,10 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF const std::string config_str = filter_chain.filters_size() == 0 ? "" : filter_chain.filters(0).DebugString(); 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 || !absl::StrContains(config_str, hcm_str) || + if (is_quic && (filter_chain.filters().size() != 1 || + filter_chain.filters(0).typed_config().type_url() != hcm_str || !absl::StrContains(config_str, "codec_type: HTTP3"))) { throw EnvoyException(fmt::format( "error building network filter chain for quic listener: requires exactly one http_" From f6a603a4d2c629d52a2ee7c84c6920f9a11e99a6 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 27 Jul 2021 10:58:57 -0400 Subject: [PATCH 3/5] moving away from string compares Signed-off-by: Alyssa Wilk --- envoy/server/factory_context.h | 5 ++ .../network/http_connection_manager/config.cc | 6 ++ source/server/api_listener_impl.cc | 7 +- source/server/filter_chain_manager_impl.cc | 10 ++- source/server/filter_chain_manager_impl.h | 5 +- source/server/listener_impl.cc | 7 +- source/server/listener_impl.h | 3 + source/server/listener_manager_impl.cc | 12 ---- .../http_connection_manager/config_test.cc | 50 ++++++++++++-- test/mocks/server/factory_context.h | 1 + test/mocks/server/listener_factory_context.h | 1 + .../listener_manager_impl_quic_only_test.cc | 65 +------------------ 12 files changed, 87 insertions(+), 85 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index bc93d6bd8547e..03cbfe9c23ff4 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -211,6 +211,11 @@ class FactoryContext : public virtual CommonFactoryContext { */ virtual Stats::Scope& listenerScope() PURE; + /** + * @return bool if these filters are created under the scope of a Quic listener. + */ + virtual bool isQuicListener() const PURE; + /** * @return const envoy::config::core::v3::Metadata& the config metadata associated with this * listener. diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 4e5b9c4757d78..92e4c8a1f8abc 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -571,6 +571,9 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( HTTP3: #ifdef ENVOY_ENABLE_QUIC codec_type_ = CodecType::HTTP3; + if (!context_.isQuicListener()) { + throw EnvoyException("HTTP/3 codec configured on non-QUIC listener."); + } #else throw EnvoyException("HTTP3 configured but not enabled in the build."); #endif @@ -578,6 +581,9 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( default: NOT_REACHED_GCOVR_EXCL_LINE; } + if (codec_type_ != CodecType::HTTP3 && context_.isQuicListener()) { + throw EnvoyException("Non-HTTP/3 codec configured on QUIC listener."); + } const auto& filters = config.http_filters(); DependencyManager dependency_manager; diff --git a/source/server/api_listener_impl.cc b/source/server/api_listener_impl.cc index e9963392cca5c..1a5e1f53883c5 100644 --- a/source/server/api_listener_impl.cc +++ b/source/server/api_listener_impl.cc @@ -13,13 +13,18 @@ namespace Envoy { namespace Server { +bool isQuic(const envoy::config::listener::v3::Listener& config) { + return config.has_udp_listener_config() && config.udp_listener_config().has_quic_options(); +} + ApiListenerImplBase::ApiListenerImplBase(const envoy::config::listener::v3::Listener& config, ListenerManagerImpl& parent, const std::string& name) : config_(config), parent_(parent), name_(name), address_(Network::Address::resolveProtoAddress(config.address())), global_scope_(parent_.server_.stats().createScope("")), listener_scope_(parent_.server_.stats().createScope(fmt::format("listener.api.{}.", name_))), - factory_context_(parent_.server_, config_, *this, *global_scope_, *listener_scope_), + factory_context_(parent_.server_, config_, *this, *global_scope_, *listener_scope_, + isQuic(config)), read_callbacks_(SyntheticReadCallbacks(*this)) {} void ApiListenerImplBase::SyntheticReadCallbacks::SyntheticConnection::raiseConnectionEvent( diff --git a/source/server/filter_chain_manager_impl.cc b/source/server/filter_chain_manager_impl.cc index 095d16eb6b505..9bac52cd773ef 100644 --- a/source/server/filter_chain_manager_impl.cc +++ b/source/server/filter_chain_manager_impl.cc @@ -139,6 +139,10 @@ Stats::Scope& PerFilterChainFactoryContextImpl::listenerScope() { return parent_context_.listenerScope(); } +bool PerFilterChainFactoryContextImpl::isQuicListener() const { + return parent_context_.isQuicListener(); +} + FilterChainManagerImpl::FilterChainManagerImpl( const Network::Address::InstanceConstSharedPtr& address, Configuration::FactoryContext& factory_context, Init::Manager& init_manager, @@ -742,9 +746,10 @@ Configuration::FilterChainFactoryContextPtr FilterChainManagerImpl::createFilter FactoryContextImpl::FactoryContextImpl(Server::Instance& server, const envoy::config::listener::v3::Listener& config, Network::DrainDecision& drain_decision, - Stats::Scope& global_scope, Stats::Scope& listener_scope) + Stats::Scope& global_scope, Stats::Scope& listener_scope, + bool is_quic) : server_(server), config_(config), drain_decision_(drain_decision), - global_scope_(global_scope), listener_scope_(listener_scope) {} + global_scope_(global_scope), listener_scope_(listener_scope), is_quic_(is_quic) {} AccessLog::AccessLogManager& FactoryContextImpl::accessLogManager() { return server_.accessLogManager(); @@ -791,5 +796,6 @@ envoy::config::core::v3::TrafficDirection FactoryContextImpl::direction() const } Network::DrainDecision& FactoryContextImpl::drainDecision() { return drain_decision_; } Stats::Scope& FactoryContextImpl::listenerScope() { return listener_scope_; } +bool FactoryContextImpl::isQuicListener() const { return is_quic_; } } // namespace Server } // namespace Envoy diff --git a/source/server/filter_chain_manager_impl.h b/source/server/filter_chain_manager_impl.h index 4e6caf625dbf9..8218e89777127 100644 --- a/source/server/filter_chain_manager_impl.h +++ b/source/server/filter_chain_manager_impl.h @@ -81,6 +81,7 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Configuration::ServerFactoryContext& getServerFactoryContext() const override; Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; + bool isQuicListener() const override; void startDraining() override { is_draining_.store(true); } @@ -135,7 +136,7 @@ class FactoryContextImpl : public Configuration::FactoryContext { public: FactoryContextImpl(Server::Instance& server, const envoy::config::listener::v3::Listener& config, Network::DrainDecision& drain_decision, Stats::Scope& global_scope, - Stats::Scope& listener_scope); + Stats::Scope& listener_scope, bool is_quic); // Configuration::FactoryContext AccessLog::AccessLogManager& accessLogManager() override; @@ -166,6 +167,7 @@ class FactoryContextImpl : public Configuration::FactoryContext { envoy::config::core::v3::TrafficDirection direction() const override; Network::DrainDecision& drainDecision() override; Stats::Scope& listenerScope() override; + bool isQuicListener() const override; private: Server::Instance& server_; @@ -173,6 +175,7 @@ class FactoryContextImpl : public Configuration::FactoryContext { Network::DrainDecision& drain_decision_; Stats::Scope& global_scope_; Stats::Scope& listener_scope_; + bool is_quic_; }; /** diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index 44feb19180a99..92d10db186dc8 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -193,7 +193,8 @@ ListenerFactoryContextBaseImpl::ListenerFactoryContextBaseImpl( !config.stat_prefix().empty() ? config.stat_prefix() : Network::Address::resolveProtoAddress(config.address())->asString()))), - validation_visitor_(validation_visitor), drain_manager_(std::move(drain_manager)) {} + validation_visitor_(validation_visitor), drain_manager_(std::move(drain_manager)), + is_quic_(config.udp_listener_config().has_quic_options()) {} AccessLog::AccessLogManager& ListenerFactoryContextBaseImpl::accessLogManager() { return server_.accessLogManager(); @@ -251,6 +252,7 @@ ListenerFactoryContextBaseImpl::getTransportSocketFactoryContext() const { return server_.transportSocketFactoryContext(); } Stats::Scope& ListenerFactoryContextBaseImpl::listenerScope() { return *listener_scope_; } +bool ListenerFactoryContextBaseImpl::isQuicListener() const { return is_quic_; } Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; } @@ -661,6 +663,9 @@ PerListenerFactoryContextImpl::getTransportSocketFactoryContext() const { Stats::Scope& PerListenerFactoryContextImpl::listenerScope() { return listener_factory_context_base_->listenerScope(); } +bool PerListenerFactoryContextImpl::isQuicListener() const { + return listener_factory_context_base_->isQuicListener(); +} Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } bool ListenerImpl::createNetworkFilterChain( diff --git a/source/server/listener_impl.h b/source/server/listener_impl.h index dfc04ef5b5a3e..effa7368f4681 100644 --- a/source/server/listener_impl.h +++ b/source/server/listener_impl.h @@ -129,6 +129,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex Configuration::ServerFactoryContext& getServerFactoryContext() const override; Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; + bool isQuicListener() const override; // DrainDecision bool drainClose() const override { @@ -148,6 +149,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex Stats::ScopePtr listener_scope_; // Stats with listener named scope. ProtobufMessage::ValidationVisitor& validation_visitor_; const Server::DrainManagerPtr drain_manager_; + bool is_quic_; }; class ListenerImpl; @@ -201,6 +203,7 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; + bool isQuicListener() const override; // ListenerFactoryContext const Network::ListenerConfig& listenerConfig() const override; diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 2924b5e41b920..98814eea12918 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -930,18 +930,6 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF "{}. \nUse QuicDownstreamTransport instead.", transport_socket.DebugString())); } - const std::string config_str = - filter_chain.filters_size() == 0 ? "" : filter_chain.filters(0).DebugString(); - 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 || - !absl::StrContains(config_str, "codec_type: HTTP3"))) { - throw EnvoyException(fmt::format( - "error building network filter chain for quic listener: requires exactly one http_" - "connection_manager filter with an HTTP/3 codec.")); - } #else // When QUIC is compiled out it should not be possible to configure either the QUIC transport // socket or the QUIC listener and get to this point. diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index bf073cd25a2a2..95c46f1cd8374 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -201,10 +201,23 @@ stat_prefix: router )EOF"; #ifdef ENVOY_ENABLE_QUIC - HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_, - date_provider_, route_config_provider_manager_, - scoped_routes_config_provider_manager_, http_tracer_manager_, - filter_config_provider_manager_); + { + EXPECT_CALL(context_, isQuicListener()).WillOnce(Return(false)); + + EXPECT_THROW_WITH_MESSAGE( + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), + context_, date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, + http_tracer_manager_, filter_config_provider_manager_), + EnvoyException, "HTTP/3 codec configured on non-QUIC listener."); + } + { + EXPECT_CALL(context_, isQuicListener()).WillOnce(Return(true)); + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_, + filter_config_provider_manager_); + } #else EXPECT_THROW_WITH_MESSAGE( HttpConnectionManagerConfig(parseHttpConnectionManagerFromYaml(yaml_string), context_, @@ -215,6 +228,35 @@ stat_prefix: router #endif } +TEST_F(HttpConnectionManagerConfigTest, Http3HalfConfigured) { + const std::string yaml_string = R"EOF( +codec_type: http1 +server_name: foo +stat_prefix: router +route_config: + virtual_hosts: + - name: service + domains: + - "*" + routes: + - match: + prefix: "/" + route: + cluster: cluster +http_filters: +- name: envoy.filters.http.router + )EOF"; + + EXPECT_CALL(context_, isQuicListener()).WillOnce(Return(true)); + + EXPECT_THROW_WITH_MESSAGE( + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, + http_tracer_manager_, filter_config_provider_manager_), + EnvoyException, "Non-HTTP/3 codec configured on QUIC listener."); +} + TEST_F(HttpConnectionManagerConfigTest, TracingNotEnabledAndNoTracingConfigInBootstrap) { const std::string yaml_string = R"EOF( codec_type: http1 diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index 41f3c20bf4400..ec7e6f8659ef0 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -38,6 +38,7 @@ class MockFactoryContext : public virtual FactoryContext { MOCK_METHOD(ThreadLocal::Instance&, threadLocal, ()); MOCK_METHOD(Server::Admin&, admin, ()); MOCK_METHOD(Stats::Scope&, listenerScope, ()); + MOCK_METHOD(bool, isQuicListener, (), (const)); MOCK_METHOD(const LocalInfo::LocalInfo&, localInfo, (), (const)); MOCK_METHOD(const envoy::config::core::v3::Metadata&, listenerMetadata, (), (const)); MOCK_METHOD(envoy::config::core::v3::TrafficDirection, direction, (), (const)); diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index a72237a2eaf4e..095aad5931dcb 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -40,6 +40,7 @@ class MockListenerFactoryContext : public ListenerFactoryContext { MOCK_METHOD(ThreadLocal::Instance&, threadLocal, ()); MOCK_METHOD(Server::Admin&, admin, ()); MOCK_METHOD(Stats::Scope&, listenerScope, ()); + MOCK_METHOD(bool, isQuicListener, (), (const)); MOCK_METHOD(const LocalInfo::LocalInfo&, localInfo, (), (const)); MOCK_METHOD(const envoy::config::core::v3::Metadata&, listenerMetadata, (), (const)); MOCK_METHOD(envoy::config::core::v3::TrafficDirection, direction, (), (const)); diff --git a/test/server/listener_manager_impl_quic_only_test.cc b/test/server/listener_manager_impl_quic_only_test.cc index 3b695f63c70d0..6f1f08fe4fa78 100644 --- a/test/server/listener_manager_impl_quic_only_test.cc +++ b/test/server/listener_manager_impl_quic_only_test.cc @@ -36,16 +36,7 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryAndSslContext) { filter_chains: - filter_chain_match: transport_protocol: "quic" - 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 + filters: [] transport_socket: name: envoy.transport_sockets.quic typed_config: @@ -180,60 +171,6 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongTransportSoc #endif } -TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongCodec) { - const 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" - 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: HTTP2 - stat_prefix: hcm - route_config: - name: local_route - http_filters: - - name: envoy.filters.http.router - 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_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 -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(manager_->addOrUpdateListener(listener_proto, "", true), EnvoyException, - "error building network filter chain for quic listener: requires exactly " - "one http_connection_manager filter with an HTTP/3 codec."); -#else - EXPECT_THROW_WITH_REGEX(manager_->addOrUpdateListener(listener_proto, "", true), EnvoyException, - "QUIC is configured but not enabled in the build."); -#endif -} - } // namespace } // namespace Server } // namespace Envoy From 6f082a771d46b7919f247dc81d616a2db50f360b Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 27 Jul 2021 15:04:22 -0400 Subject: [PATCH 4/5] reinstating HCM checks Signed-off-by: Alyssa Wilk --- source/server/listener_manager_impl.cc | 11 ++++ .../listener_manager_impl_quic_only_test.cc | 56 ++++++++++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 98814eea12918..e855546ca5d9c 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -930,6 +930,17 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF "{}. \nUse QuicDownstreamTransport instead.", transport_socket.DebugString())); } + const std::string config_str = + filter_chain.filters_size() == 0 ? "" : filter_chain.filters(0).DebugString(); + 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.")); + } #else // When QUIC is compiled out it should not be possible to configure either the QUIC transport // socket or the QUIC listener and get to this point. diff --git a/test/server/listener_manager_impl_quic_only_test.cc b/test/server/listener_manager_impl_quic_only_test.cc index 6f1f08fe4fa78..8d4f1bb2c4e30 100644 --- a/test/server/listener_manager_impl_quic_only_test.cc +++ b/test/server/listener_manager_impl_quic_only_test.cc @@ -36,7 +36,16 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryAndSslContext) { filter_chains: - filter_chain_match: transport_protocol: "quic" - filters: [] + 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 transport_socket: name: envoy.transport_sockets.quic typed_config: @@ -171,6 +180,51 @@ TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongTransportSoc #endif } +TEST_F(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithWrongCodec) { + const 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" + filters: [] + 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_subject_alt_names: + - exact: localhost + - exact: 127.0.0.1 +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(manager_->addOrUpdateListener(listener_proto, "", true), EnvoyException, + "error building network filter chain for quic listener: requires exactly " + "one http_connection_manager filter."); +#else + EXPECT_THROW_WITH_REGEX(manager_->addOrUpdateListener(listener_proto, "", true), EnvoyException, + "QUIC is configured but not enabled in the build."); +#endif +} + } // namespace } // namespace Server } // namespace Envoy From fa96d79cdfd7035dfb5913fef1b9e8a16aabc472 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 27 Jul 2021 15:38:41 -0400 Subject: [PATCH 5/5] fix Signed-off-by: Alyssa Wilk --- source/server/listener_manager_impl.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index e855546ca5d9c..a9433d2b5b37b 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -930,8 +930,6 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF "{}. \nUse QuicDownstreamTransport instead.", transport_socket.DebugString())); } - const std::string config_str = - filter_chain.filters_size() == 0 ? "" : filter_chain.filters(0).DebugString(); const std::string hcm_str = "type.googleapis.com/" "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager";