[1.21] CVE-2021-43826

Signed-off-by: Yan Avlasov <[email protected]>

docs/root/version_history/current.rst

@@ -16,6 +16,7 @@ Bug Fixes
 
 * data plane: fix crash when internal redirect selects a route configured with direct response or redirect actions.
 * jwt_authn: fixed the crash when a CONNECT request is sent to JWT filter configured with regex match on the Host header.
+* tcp_proxy: fix a crash that occurs when configured for :ref:`upstream tunneling <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.tunneling_config>` and the downstream connection disconnects while the the upstream connection or http/2 stream is still being established.
 * upstream: fix stack overflow when a cluster with large number of idle connections is removed.
 
 Removed Config or Runtime

source/common/tcp_proxy/tcp_proxy.cc

@@ -526,8 +526,14 @@ Network::FilterStatus Filter::onNewConnection() {
 }
 
 void Filter::onDownstreamEvent(Network::ConnectionEvent event) {
+  if (event == Network::ConnectionEvent::LocalClose ||
+      event == Network::ConnectionEvent::RemoteClose) {
+    downstream_closed_ = true;
+  }
+
   ENVOY_CONN_LOG(trace, "on downstream event {}, has upstream = {}", read_callbacks_->connection(),
                  static_cast<int>(event), upstream_ == nullptr);
+
   if (upstream_) {
     Tcp::ConnectionPool::ConnectionDataPtr conn_data(upstream_->onDownstreamEvent(event));
     if (conn_data != nullptr &&
@@ -576,7 +582,9 @@ void Filter::onUpstreamEvent(Network::ConnectionEvent event) {
             Upstream::Outlier::Result::LocalOriginConnectFailed);
       }
 
-      initializeUpstreamConnection();
+      if (!downstream_closed_) {
+        initializeUpstreamConnection();
+      }
     } else {
       if (read_callbacks_->connection().state() == Network::Connection::State::Open) {
         read_callbacks_->connection().close(Network::ConnectionCloseType::FlushWrite);

source/common/tcp_proxy/tcp_proxy.h

@@ -375,6 +375,7 @@ class Filter : public Network::ReadFilter,
   Network::Socket::OptionsSharedPtr upstream_options_;
   uint32_t connect_attempts_{};
   bool connecting_{};
+  bool downstream_closed_{};
 };
 
 // This class deals with an upstream connection that needs to finish flushing, when the downstream

test/integration/tcp_tunneling_integration_test.cc

@@ -747,6 +747,69 @@ TEST_P(TcpTunnelingIntegrationTest, ResetStreamTest) {
   tcp_client_->waitForDisconnect();
 }
 
+TEST_P(TcpTunnelingIntegrationTest, UpstreamConnectingDownstreamDisconnect) {
+  if (upstreamProtocol() == Http::CodecType::HTTP1) {
+    return;
+  }
+
+#if defined(WIN32)
+  // TODO(ggreenway): figure out why this test fails on Windows and remove this disable.
+  // Failing tests:
+  // IpAndHttpVersions/TcpTunnelingIntegrationTest.UpstreamConnectingDownstreamDisconnect/IPv4_HttpDownstream_Http3UpstreamBareHttp2,
+  // IpAndHttpVersions/TcpTunnelingIntegrationTest.UpstreamConnectingDownstreamDisconnect/IPv6_HttpDownstream_Http2UpstreamWrappedHttp2,
+  // Times out at the end of the test on `ASSERT_TRUE(upstream_request_->waitForReset());`.
+  return;
+#endif
+
+  config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
+    envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy proxy_config;
+    proxy_config.set_stat_prefix("tcp_stats");
+    proxy_config.set_cluster("cluster_0");
+    proxy_config.mutable_tunneling_config()->set_hostname("host.com:80");
+
+    // Enable retries. The crash is due to retrying after the downstream connection is closed, which
+    // can't occur if retries are not enabled.
+    proxy_config.mutable_max_connect_attempts()->set_value(2);
+
+    auto* listeners = bootstrap.mutable_static_resources()->mutable_listeners();
+    for (auto& listener : *listeners) {
+      if (listener.name() != "tcp_proxy") {
+        continue;
+      }
+      auto* filter_chain = listener.mutable_filter_chains(0);
+      auto* filter = filter_chain->mutable_filters(0);
+      filter->mutable_typed_config()->PackFrom(proxy_config);
+
+      // Use TLS because it will respond to a TCP half-close during handshake by closing the
+      // connection.
+      envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context;
+      ConfigHelper::initializeTls({}, *tls_context.mutable_common_tls_context());
+      filter_chain->mutable_transport_socket()->set_name("envoy.transport_sockets.tls");
+      filter_chain->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context);
+
+      break;
+    }
+  });
+
+  enableHalfClose(false);
+  initialize();
+
+  IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy"));
+
+  // Wait for the request for a connection, but don't send a response back yet. This ensures that
+  // tcp_proxy is stuck in `connecting_`.
+  ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
+  ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));
+  ASSERT_TRUE(upstream_request_->waitForHeadersComplete());
+
+  // Close the client connection. The TLS transport socket will detect this even while
+  // `readDisable(true)` on the connection, and will raise a `RemoteClose` event.
+  tcp_client->close();
+
+  ASSERT_TRUE(upstream_request_->waitForReset());
+  ASSERT_TRUE(fake_upstream_connection_->close());
+}
+
 TEST_P(TcpTunnelingIntegrationTest, TestIdletimeoutWithLargeOutstandingData) {
   enableHalfClose(false);
   config_helper_.setBufferLimits(1024, 1024);