http3: validating codec

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.

source/extensions/filters/network/http_connection_manager/config.cc

@@ -571,13 +571,19 @@ 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
     break;
   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;

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(

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

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,13 +167,15 @@ 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_;
   const envoy::config::listener::v3::Listener& config_;
   Network::DrainDecision& drain_decision_;
   Stats::Scope& global_scope_;
   Stats::Scope& listener_scope_;
+  bool is_quic_;
 };
 
 /**

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(

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;

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.

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

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));

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));

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",

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