Merge remote-tracking branch 'origin/master' into stream_info_cleanups

Signed-off-by: Matt Klein <mklein@lyft.com>

api/bazel/repository_locations.bzl

@@ -14,11 +14,11 @@ REPOSITORY_LOCATIONS_SPEC = dict(
         project_name = "protoc-gen-validate (PGV)",
         project_desc = "protoc plugin to generate polyglot message validators",
         project_url = "https://github.com/envoyproxy/protoc-gen-validate",
-        version = "1bcea29601b5624234a19b3d7f0ebd9e9984f583",
-        sha256 = "2062bbe50eddf3c98490339721fb02b5b5cd78f610f163b98bbf95ba7105553f",
+        version = "872b28c457822ed9c2a5405da3c33f386ac0e86f",
+        sha256 = "388ea2261bc1d2c6ef6ec01bfaa3aec451aedb245e23514033ccc9b5cc10c4ab",
         strip_prefix = "protoc-gen-validate-{version}",
         urls = ["https://github.com/envoyproxy/protoc-gen-validate/archive/{version}.tar.gz"],
-        release_date = "2020-11-30",
+        release_date = "2021-01-05",
         use_category = ["api"],
         implied_untracked_deps = [
             "com_github_iancoleman_strcase",

docs/root/version_history/current.rst

@@ -68,6 +68,7 @@ New Features
 * compression: the :ref:`compressor <envoy_v3_api_msg_extensions.filters.http.compressor.v3.Compressor>` filter adds support for compressing request payloads. Its configuration is unified with the :ref:`decompressor <envoy_v3_api_msg_extensions.filters.http.decompressor.v3.Decompressor>` filter with two new fields for different directions - :ref:`requests <envoy_v3_api_field_extensions.filters.http.compressor.v3.Compressor.request_direction_config>` and :ref:`responses <envoy_v3_api_field_extensions.filters.http.compressor.v3.Compressor.response_direction_config>`. The latter deprecates the old response-specific fields and, if used, roots the response-specific stats in `<stat_prefix>.compressor.<compressor_library.name>.<compressor_library_stat_prefix>.response.*` instead of `<stat_prefix>.compressor.<compressor_library.name>.<compressor_library_stat_prefix>.*`.
 * config: added ability to flush stats when the admin's :ref:`/stats endpoint <operations_admin_interface_stats>` is hit instead of on a timer via :ref:`stats_flush_on_admin <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.stats_flush_on_admin>`.
 * config: added new runtime feature `envoy.features.enable_all_deprecated_features` that allows the use of all deprecated features.
+* crash support: added the ability to dump L4 connection data on crash.
 * formatter: added new :ref:`text_format_source <envoy_v3_api_field_config.core.v3.SubstitutionFormatString.text_format_source>` field to support format strings both inline and from a file.
 * formatter: added support for custom date formatting to :ref:`%DOWNSTREAM_PEER_CERT_V_START% <config_access_log_format_downstream_peer_cert_v_start>` and :ref:`%DOWNSTREAM_PEER_CERT_V_END% <config_access_log_format_downstream_peer_cert_v_end>`, similar to :ref:`%START_TIME% <config_access_log_format_start_time>`.
 * grpc: implemented header value syntax support when defining :ref:`initial metadata <envoy_v3_api_field_config.core.v3.GrpcService.initial_metadata>` for gRPC-based `ext_authz` :ref:`HTTP <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.grpc_service>` and :ref:`network <envoy_v3_api_field_extensions.filters.network.ext_authz.v3.ExtAuthz.grpc_service>` filters, and :ref:`ratelimit <envoy_v3_api_field_config.ratelimit.v3.RateLimitServiceConfig.grpc_service>` filters.

include/envoy/common/scope_tracker.h

@@ -7,16 +7,19 @@
 namespace Envoy {
 
 /*
- * A class for tracking the scope of work.
- * Currently this is only used for best-effort tracking the L7 stream doing
- * work if a fatal error occurs.
+ * An interface for tracking the scope of work. Implementors of this interface
+ * can be registered to the dispatcher when they're active on the stack. If a
+ * fatal error occurs while they were active, the dumpState method will be
+ * called.
+ *
+ * Currently this is only used for the L4 network connection and L7 stream.
  */
 class ScopeTrackedObject {
 public:
   virtual ~ScopeTrackedObject() = default;
 
   /**
-   * Dump debug state of the object in question to the provided ostream
+   * Dump debug state of the object in question to the provided ostream.
    *
    * This is called on Envoy fatal errors, so should do minimal memory allocation.
    *

include/envoy/network/BUILD

@@ -130,6 +130,7 @@ envoy_cc_library(
     deps = [
         ":io_handle_interface",
         ":socket_interface",
+        "//include/envoy/common:scope_tracker_interface",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],
 )

include/envoy/network/listen_socket.h

@@ -7,6 +7,7 @@
 
 #include "envoy/common/exception.h"
 #include "envoy/common/pure.h"
+#include "envoy/common/scope_tracker.h"
 #include "envoy/config/core/v3/base.pb.h"
 #include "envoy/network/address.h"
 #include "envoy/network/io_handle.h"
@@ -25,7 +26,7 @@ namespace Network {
  * TODO(jrajahalme): Hide internals (e.g., fd) from listener filters by providing callbacks filters
  * may need (set/getsockopt(), peek(), recv(), etc.)
  */
-class ConnectionSocket : public virtual Socket {
+class ConnectionSocket : public virtual Socket, public virtual ScopeTrackedObject {
 public:
   /**
    * Set detected transport protocol (e.g. RAW_BUFFER, TLS).

source/common/common/dump_state_utils.h

@@ -14,6 +14,9 @@ namespace Envoy {
 #define DUMP_OPTIONAL_MEMBER(member)                                                               \
   ", " #member ": " << ((member).has_value() ? absl::StrCat((member).value()) : "null")
 
+#define DUMP_NULLABLE_MEMBER(member, value)                                                        \
+  ", " #member ": " << ((member) != nullptr ? value : "null")
+
 // Macro assumes local member variables
 // os (ostream)
 // indent_level (int)

source/common/network/BUILD

@@ -60,8 +60,10 @@ envoy_cc_library(
     hdrs = ["connection_impl_base.h"],
     deps = [
         ":filter_manager_lib",
+        "//include/envoy/common:scope_tracker_interface",
         "//include/envoy/event:dispatcher_interface",
         "//source/common/common:assert_lib",
+        "//source/common/common:dump_state_utils",
     ],
 )
 
@@ -246,6 +248,7 @@ envoy_cc_library(
         "//include/envoy/network:exception_interface",
         "//include/envoy/network:listen_socket_interface",
         "//source/common/common:assert_lib",
+        "//source/common/common:dump_state_utils",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],
 )

source/common/network/connection_impl.cc

@@ -12,8 +12,10 @@
 #include "envoy/network/socket.h"
 
 #include "common/common/assert.h"
+#include "common/common/dump_state_utils.h"
 #include "common/common/empty_string.h"
 #include "common/common/enum_to_int.h"
+#include "common/common/scope_tracker.h"
 #include "common/network/address_impl.h"
 #include "common/network/listen_socket_impl.h"
 #include "common/network/raw_buffer_socket.h"
@@ -26,8 +28,20 @@ namespace {
 constexpr absl::string_view kTransportSocketConnectTimeoutTerminationDetails =
     "transport socket timeout was reached";
 
+std::ostream& operator<<(std::ostream& os, Connection::State connection_state) {
+  switch (connection_state) {
+  case Connection::State::Open:
+    return os << "Open";
+  case Connection::State::Closing:
+    return os << "Closing";
+  case Connection::State::Closed:
+    return os << "Closed";
+  }
+  return os;
 }
 
+} // namespace
+
 void ConnectionImplUtility::updateBufferStats(uint64_t delta, uint64_t new_total,
                                               uint64_t& previous_total, Stats::Counter& stat_total,
                                               Stats::Gauge& stat_current) {
@@ -530,6 +544,7 @@ void ConnectionImpl::onWriteBufferHighWatermark() {
 }
 
 void ConnectionImpl::onFileEvent(uint32_t events) {
+  ScopeTrackerScopeState scope(this, this->dispatcher_);
   ENVOY_CONN_LOG(trace, "socket event: {}", *this, events);
 
   if (immediate_error_event_ != ConnectionEvent::Connected) {
@@ -753,6 +768,14 @@ void ConnectionImpl::flushWriteBuffer() {
   }
 }
 
+void ConnectionImpl::dumpState(std::ostream& os, int indent_level) const {
+  const char* spaces = spacesForLevel(indent_level);
+  os << spaces << "ConnectionImpl " << this << DUMP_MEMBER(connecting_) << DUMP_MEMBER(bind_error_)
+     << DUMP_MEMBER(state()) << DUMP_MEMBER(read_buffer_limit_) << "\n";
+
+  DUMP_DETAILS(socket_);
+}
+
 ServerConnectionImpl::ServerConnectionImpl(Event::Dispatcher& dispatcher,
                                            ConnectionSocketPtr&& socket,
                                            TransportSocketPtr&& transport_socket,

source/common/network/connection_impl.h

@@ -6,6 +6,7 @@
 #include <memory>
 #include <string>
 
+#include "envoy/common/scope_tracker.h"
 #include "envoy/network/transport_socket.h"
 
 #include "common/buffer/watermark_buffer.h"
@@ -41,9 +42,12 @@ class ConnectionImplUtility {
 };
 
 /**
- * Implementation of Network::Connection and Network::FilterManagerConnection.
+ * Implementation of Network::Connection, Network::FilterManagerConnection and
+ * Envoy::ScopeTrackedObject.
  */
-class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallbacks {
+class ConnectionImpl : public ConnectionImplBase,
+                       public TransportSocketCallbacks,
+                       public ScopeTrackedObject {
 public:
   ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket,
                  TransportSocketPtr&& transport_socket, StreamInfo::StreamInfo& stream_info,
@@ -122,6 +126,9 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback
   // Obtain global next connection ID. This should only be used in tests.
   static uint64_t nextGlobalIdForTest() { return next_global_id_; }
 
+  // ScopeTrackedObject
+  void dumpState(std::ostream& os, int indent_level) const override;
+
 protected:
   // A convenience function which returns true if
   // 1) The read disable count is zero or

source/common/network/listen_socket_impl.h

@@ -11,6 +11,7 @@
 #include "envoy/network/socket_interface.h"
 
 #include "common/common/assert.h"
+#include "common/common/dump_state_utils.h"
 #include "common/network/socket_impl.h"
 
 namespace Envoy {
@@ -123,6 +124,15 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket {
     return ioHandle().lastRoundTripTime();
   }
 
+  void dumpState(std::ostream& os, int indent_level) const override {
+    const char* spaces = spacesForLevel(indent_level);
+    os << spaces << "ListenSocketImpl " << this
+       << DUMP_NULLABLE_MEMBER(remote_address_, remote_address_->asStringView())
+       << DUMP_NULLABLE_MEMBER(direct_remote_address_, direct_remote_address_->asStringView())
+       << DUMP_NULLABLE_MEMBER(local_address_, local_address_->asStringView())
+       << DUMP_MEMBER(transport_protocol_) << DUMP_MEMBER(server_name_) << "\n";
+  }
+
 protected:
   std::string transport_protocol_;
   std::vector<std::string> application_protocols_;

test/common/network/connection_impl_test.cc

@@ -4,11 +4,13 @@
 
 #include "envoy/common/platform.h"
 #include "envoy/config/core/v3/base.pb.h"
+#include "envoy/network/address.h"
 
 #include "common/api/os_sys_calls_impl.h"
 #include "common/buffer/buffer_impl.h"
 #include "common/common/empty_string.h"
 #include "common/common/fmt.h"
+#include "common/common/utility.h"
 #include "common/event/dispatcher_impl.h"
 #include "common/network/address_impl.h"
 #include "common/network/connection_impl.h"
@@ -1855,6 +1857,76 @@ TEST_P(ConnectionImplTest, DelayedCloseTimeoutNullStats) {
   server_connection->close(ConnectionCloseType::NoFlush);
 }
 
+// Test DumpState methods.
+TEST_P(ConnectionImplTest, NetworkSocketDumpsWithoutAllocatingMemory) {
+  std::array<char, 1024> buffer;
+  OutputBufferStream ostream{buffer.data(), buffer.size()};
+  IoHandlePtr io_handle = std::make_unique<IoSocketHandleImpl>(0);
+  Address::InstanceConstSharedPtr server_addr;
+  Address::InstanceConstSharedPtr local_addr;
+  if (GetParam() == Network::Address::IpVersion::v4) {
+    server_addr = Network::Address::InstanceConstSharedPtr{
+        new Network::Address::Ipv4Instance("1.1.1.1", 80, nullptr)};
+    local_addr = Network::Address::InstanceConstSharedPtr{
+        new Network::Address::Ipv4Instance("1.2.3.4", 56789, nullptr)};
+  } else {
+    server_addr = Network::Address::InstanceConstSharedPtr{
+        new Network::Address::Ipv6Instance("::1", 80, nullptr)};
+    local_addr = Network::Address::InstanceConstSharedPtr{
+        new Network::Address::Ipv6Instance("::1:2:3:4", 56789, nullptr)};
+  }
+
+  auto connection_socket =
+      std::make_unique<ConnectionSocketImpl>(std::move(io_handle), local_addr, server_addr);
+  connection_socket->setRequestedServerName("envoyproxy.io");
+
+  // Start measuring memory and dump state.
+  Stats::TestUtil::MemoryTest memory_test;
+  connection_socket->dumpState(ostream, 0);
+  EXPECT_EQ(memory_test.consumedBytes(), 0);
+
+  // Check socket dump
+  EXPECT_THAT(ostream.contents(), HasSubstr("ListenSocketImpl"));
+  if (GetParam() == Network::Address::IpVersion::v4) {
+    EXPECT_THAT(
+        ostream.contents(),
+        HasSubstr(
+            "remote_address_: 1.1.1.1:80, direct_remote_address_: 1.1.1.1:80, local_address_: "
+            "1.2.3.4:56789, transport_protocol_: , server_name_: envoyproxy.io"));
+  } else {
+    EXPECT_THAT(
+        ostream.contents(),
+        HasSubstr("remote_address_: [::1]:80, direct_remote_address_: [::1]:80, local_address_: "
+                  "[::1:2:3:4]:56789, transport_protocol_: , server_name_: envoyproxy.io"));
+  }
+}
+
+TEST_P(ConnectionImplTest, NetworkConnectionDumpsWithoutAllocatingMemory) {
+  std::array<char, 1024> buffer;
+  OutputBufferStream ostream{buffer.data(), buffer.size()};
+  ConnectionMocks mocks = createConnectionMocks(false);
+  IoHandlePtr io_handle = std::make_unique<IoSocketHandleImpl>(0);
+
+  auto server_connection = std::make_unique<Network::ServerConnectionImpl>(
+      *mocks.dispatcher_,
+      std::make_unique<ConnectionSocketImpl>(std::move(io_handle), nullptr, nullptr),
+      std::move(mocks.transport_socket_), stream_info_, true);
+
+  // Start measuring memory and dump state.
+  Stats::TestUtil::MemoryTest memory_test;
+  server_connection->dumpState(ostream, 0);
+  EXPECT_EQ(memory_test.consumedBytes(), 0);
+
+  // Check connection data
+  EXPECT_THAT(ostream.contents(), HasSubstr("ConnectionImpl"));
+  EXPECT_THAT(ostream.contents(),
+              HasSubstr("connecting_: 0, bind_error_: 0, state(): Open, read_buffer_limit_: 0"));
+  // Check socket starts dumping
+  EXPECT_THAT(ostream.contents(), HasSubstr("ListenSocketImpl"));
+
+  server_connection->close(ConnectionCloseType::NoFlush);
+}
+
 class FakeReadFilter : public Network::ReadFilter {
 public:
   FakeReadFilter() = default;
@@ -1902,6 +1974,8 @@ class MockTransportConnectionImplTest : public testing::Test {
         dispatcher_, std::make_unique<ConnectionSocketImpl>(std::move(io_handle), nullptr, nullptr),
         TransportSocketPtr(transport_socket_), stream_info_, true);
     connection_->addConnectionCallbacks(callbacks_);
+    // File events will trigger setTrackedObject on the dispatcher.
+    EXPECT_CALL(dispatcher_, setTrackedObject(_)).WillRepeatedly(Return(nullptr));
   }
 
   ~MockTransportConnectionImplTest() override { connection_->close(ConnectionCloseType::NoFlush); }

test/mocks/network/mocks.h

@@ -2,6 +2,7 @@
 
 #include <cstdint>
 #include <list>
+#include <ostream>
 #include <string>
 #include <vector>
 
@@ -307,6 +308,7 @@ class MockConnectionSocket : public ConnectionSocket {
   MOCK_METHOD(Api::SysCallIntResult, getSocketOption, (int, int, void*, socklen_t*), (const));
   MOCK_METHOD(Api::SysCallIntResult, setBlockingForTest, (bool));
   MOCK_METHOD(absl::optional<std::chrono::milliseconds>, lastRoundTripTime, ());
+  MOCK_METHOD(void, dumpState, (std::ostream&, int), (const));
 
   IoHandlePtr io_handle_;
   Network::SocketAddressSetterSharedPtr address_provider_;

test/server/filter_chain_benchmark_test.cc

@@ -1,4 +1,5 @@
 #include <iostream>
+#include <ostream>
 
 #include "envoy/config/listener/v3/listener.pb.h"
 #include "envoy/config/listener/v3/listener_components.pb.h"
@@ -117,6 +118,7 @@ class MockConnectionSocket : public Network::ConnectionSocket {
   }
   Api::SysCallIntResult setBlockingForTest(bool) override { return {0, 0}; }
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() override { return {}; }
+  void dumpState(std::ostream&, int) const override {}
 
 private:
   Network::IoHandlePtr io_handle_;

tools/dependency/cve_scan.py

@@ -42,6 +42,11 @@
     # Node.js issue unrelated to http-parser, see
     # https://github.com/mhart/StringStream/issues/7.
     'CVE-2018-21270',
+    # These should not affect Curl 7.74.0, but we see false positives due to the
+    # relative release date and CPE wildcard.
+    'CVE-2020-8169',
+    'CVE-2020-8177',
+    'CVE-2020-8284',
 ])
 
 # Subset of CVE fields that are useful below.