Listener ECDS: reject the connection directly when missing config (#2…

…1544)

Signed-off-by: He Jie Xu <[email protected]>

source/server/active_stream_listener_base.h

@@ -84,8 +84,14 @@ class ActiveStreamListenerBase : public ActiveListenerImplBase,
 
   void onSocketAccepted(std::unique_ptr<ActiveTcpSocket> active_socket) {
     // Create and run the filters
-    config_->filterChainFactory().createListenerFilterChain(*active_socket);
-    active_socket->continueFilterChain(true);
+    if (config_->filterChainFactory().createListenerFilterChain(*active_socket)) {
+      active_socket->continueFilterChain(true);
+    } else {
+      // If create listener filter chain failed, it means the listener is missing
+      // config due to the ECDS. Then close the connection directly.
+      active_socket->socket_->close();
+      ASSERT(active_socket->iter_ == active_socket->accept_filters_.end());
+    }
 
     // Move active_socket to the sockets_ list if filter iteration needs to continue later.
     // Otherwise we let active_socket be destructed when it goes out of scope.

source/server/configuration_impl.cc

@@ -38,60 +38,15 @@ bool FilterChainUtility::buildFilterChain(Network::FilterManager& filter_manager
   return filter_manager.initializeReadFilters();
 }
 
-/**
- * All missing listener config stats. @see stats_macros.h
- */
-#define ALL_MISSING_LISTENER_CONFIG_STATS(COUNTER) COUNTER(extension_config_missing)
-/**
- * Struct definition for all missing listener config stats. @see stats_macros.h
- */
-struct MissingListenerConfigStats {
-  ALL_MISSING_LISTENER_CONFIG_STATS(GENERATE_COUNTER_STRUCT)
-};
-
-class MissingConfigTcpListenerFilter : public Network::ListenerFilter,
-                                       public Logger::Loggable<Logger::Id::filter> {
-public:
-  MissingConfigTcpListenerFilter(Stats::Scope& stats_scope)
-      : scope_(stats_scope), stats_({ALL_MISSING_LISTENER_CONFIG_STATS(POOL_COUNTER(scope_))}) {}
-
-  // Network::ListenerFilter
-  Network::FilterStatus onAccept(Network::ListenerFilterCallbacks& cb) override {
-    ENVOY_LOG(debug, "Listener filter: new connection accepted while missing configuration. "
-                     "Close socket and stop the iteration onAccept.");
-    cb.socket().ioHandle().close();
-    stats_.extension_config_missing_.inc();
-    return Network::FilterStatus::StopIteration;
-  }
-  Network::FilterStatus onData(Network::ListenerFilterBuffer&) override {
-    // The socket is already closed onAccept. Just return StopIteration here.
-    return Network::FilterStatus::StopIteration;
-  }
-  size_t maxReadBytes() const override { return 0; }
-
-private:
-  Stats::Scope& scope_;
-  MissingListenerConfigStats stats_;
-};
-
 bool FilterChainUtility::buildFilterChain(Network::ListenerFilterManager& filter_manager,
-                                          const Filter::ListenerFilterFactoriesList& factories,
-                                          Stats::Scope& stats_scope) {
-  bool added_missing_config_filter = false;
+                                          const Filter::ListenerFilterFactoriesList& factories) {
   for (const auto& filter_config_provider : factories) {
     auto config = filter_config_provider->config();
-    if (config.has_value()) {
-      auto config_value = config.value();
-      config_value(filter_manager);
-      continue;
-    }
-
-    // If a filter config is missing after warming, stop iteration.
-    if (!added_missing_config_filter) {
-      filter_manager.addAcceptFilter(nullptr,
-                                     std::make_unique<MissingConfigTcpListenerFilter>(stats_scope));
-      added_missing_config_filter = true;
+    if (!config.has_value()) {
+      return false;
     }
+    auto config_value = config.value();
+    config_value(filter_manager);
   }
 
   return true;

source/server/configuration_impl.h

@@ -82,8 +82,7 @@ class FilterChainUtility {
    * TODO(sumukhs): Coalesce with the above as they are very similar
    */
   static bool buildFilterChain(Network::ListenerFilterManager& filter_manager,
-                               const Filter::ListenerFilterFactoriesList& factories,
-                               Stats::Scope& stats_scope);
+                               const Filter::ListenerFilterFactoriesList& factories);
 
   /**
    * Given a UdpListenerFilterManager and a list of factories, create a new filter chain. Chain

source/server/listener_impl.cc

@@ -349,7 +349,9 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
               parent_.server_.singletonManager(), parent_.server_.threadLocal(),
               validation_visitor_, parent_.server_.api(), parent_.server_.options(),
               parent_.server_.accessLogManager())),
-      quic_stat_names_(parent_.quicStatNames()) {
+      quic_stat_names_(parent_.quicStatNames()),
+      missing_listener_config_stats_({ALL_MISSING_LISTENER_CONFIG_STATS(
+          POOL_COUNTER(listener_factory_context_->listenerScope()))}) {
 
   if ((address_->type() == Network::Address::Type::Ip &&
        config.address().socket_address().ipv4_compat()) &&
@@ -440,7 +442,9 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin,
                             parent_.inPlaceFilterChainUpdate(*this);
                           }),
       transport_factory_context_(origin.transport_factory_context_),
-      quic_stat_names_(parent_.quicStatNames()) {
+      quic_stat_names_(parent_.quicStatNames()),
+      missing_listener_config_stats_({ALL_MISSING_LISTENER_CONFIG_STATS(
+          POOL_COUNTER(listener_factory_context_->listenerScope()))}) {
   buildAccessLog();
   auto socket_type = Network::Utility::protobufAddressSocketType(config.address());
   validateConfig(socket_type);
@@ -832,8 +836,14 @@ bool ListenerImpl::createNetworkFilterChain(
 }
 
 bool ListenerImpl::createListenerFilterChain(Network::ListenerFilterManager& manager) {
-  return Configuration::FilterChainUtility::buildFilterChain(manager, listener_filter_factories_,
-                                                             listenerScope());
+  if (Configuration::FilterChainUtility::buildFilterChain(manager, listener_filter_factories_)) {
+    return true;
+  } else {
+    ENVOY_LOG(debug, "New connection accepted while missing configuration. "
+                     "Close socket and stop the iteration onAccept.");
+    missing_listener_config_stats_.extension_config_missing_.inc();
+    return false;
+  }
 }
 
 void ListenerImpl::createUdpListenerFilterChain(Network::UdpListenerFilterManager& manager,

source/server/listener_impl.h

@@ -28,6 +28,18 @@
 namespace Envoy {
 namespace Server {
 
+/**
+ * All missing listener config stats. @see stats_macros.h
+ */
+#define ALL_MISSING_LISTENER_CONFIG_STATS(COUNTER) COUNTER(extension_config_missing)
+
+/**
+ * Struct definition for all missing listener config stats. @see stats_macros.h
+ */
+struct MissingListenerConfigStats {
+  ALL_MISSING_LISTENER_CONFIG_STATS(GENERATE_COUNTER_STRUCT)
+};
+
 class ListenerMessageUtil {
 public:
   /**
@@ -469,6 +481,7 @@ class ListenerImpl final : public Network::ListenerConfig,
       transport_factory_context_;
 
   Quic::QuicStatNames& quic_stat_names_;
+  MissingListenerConfigStats missing_listener_config_stats_;
 
   // to access ListenerManagerImpl::factory_.
   friend class ListenerFilterChainFactoryBuilder;