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 98814eea12918..a9433d2b5b37b 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -930,6 +930,15 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF "{}. \nUse QuicDownstreamTransport instead.", transport_socket.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/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/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..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