Add dynamic metadata for TLS inspector filter errors (#40529)

Export dynamic metadata from the TLS inspector to provide more detailed
reason for subsequent extensions in case SNI could not be detected.

Signed-off-by: Elisha Ziskind <[email protected]>

Author: eziskind

changelogs/current.yaml

@@ -318,6 +318,9 @@ new_features:
 - area: rbac
   change: |
     Switch the IP matcher to use LC-Trie for performance improvements.
+- area: tls_inspector
+  change: |
+    Added dynamic metadata when failing to parse the ``ClientHello``.
 - area: lua
   change: |
     Added ``route()`` to the Stream handle API, allowing Lua scripts to retrieve route information. So far, the only method

docs/root/configuration/advanced/well_known_dynamic_metadata.rst

@@ -32,6 +32,7 @@ The following Envoy filters can be configured to consume dynamic metadata emitte
   <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.metadata_context_namespaces>`
 * :ref:`RateLimit Filter limit override <config_http_filters_rate_limit_override_dynamic_metadata>`
 * :ref:`Original destination listener filter <config_listener_filters_original_dst>`
+* :ref:`TLS Inspector listener filter <config_listener_filters_tls_inspector_dynamic_metadata>`
 
 .. _shared_dynamic_metadata:
 

docs/root/configuration/listeners/listener_filters/tls_inspector.rst

@@ -81,4 +81,12 @@ This filter has a statistics tree rooted at *tls_inspector* with the following s
       If the connection terminates early nothing is recorded if we didn't have
       sufficient bytes for either of the cases above.
 
+.. _config_listener_filters_tls_inspector_dynamic_metadata:
+
+Dynamic Metadata
+----------------
+
+If the filter fails to detect TLS it will populate dynamic metadata under the key
+`envoy.filters.listener.tls_inspector` indicating the reason (eg. ``ClientHello`` too
+large or not detected at all).
 

source/extensions/filters/listener/tls_inspector/tls_inspector.cc

@@ -175,6 +175,13 @@ Network::FilterStatus Filter::onData(Network::ListenerFilterBuffer& buffer) {
   return Network::FilterStatus::StopIteration;
 }
 
+void Filter::setDynamicMetadata(absl::string_view failure_reason) {
+  Protobuf::Struct metadata;
+  auto& fields = *metadata.mutable_fields();
+  fields[failureReasonKey()].set_string_value(failure_reason);
+  cb_->setDynamicMetadata(dynamicMetadataKey(), metadata);
+}
+
 ParseState Filter::parseClientHello(const void* data, size_t len,
                                     uint64_t bytes_already_processed) {
   // Ownership remains here though we pass a reference to it in `SSL_set0_rbio()`.
@@ -198,6 +205,7 @@ ParseState Filter::parseClientHello(const void* data, size_t len,
         // We've hit the specified size limit. This is an unreasonably large ClientHello;
         // indicate failure.
         config_->stats().client_hello_too_large_.inc();
+        setDynamicMetadata(failureReasonClientHelloTooLarge());
         return ParseState::Error;
       }
       if (read_ >= requested_read_bytes_) {
@@ -219,9 +227,11 @@ ParseState Filter::parseClientHello(const void* data, size_t len,
           // We've hit the specified size limit. This is an unreasonably large ClientHello;
           // indicate failure.
           config_->stats().client_hello_too_large_.inc();
+          setDynamicMetadata(failureReasonClientHelloTooLarge());
           return ParseState::Error;
         }
         config_->stats().tls_not_found_.inc();
+        setDynamicMetadata(failureReasonClientHelloNotDetected());
         ENVOY_LOG(
             debug, "tls inspector: parseClientHello failed: {}",
             Extensions::TransportSockets::Tls::Utility::getLastCryptoError().value_or("unknown"));
@@ -369,6 +379,22 @@ void Filter::createJA4Hash(const SSL_CLIENT_HELLO* ssl_client_hello) {
   cb_->socket().setJA4Hash(fingerprint);
 }
 
+const std::string& Filter::dynamicMetadataKey() {
+  CONSTRUCT_ON_FIRST_USE(std::string, "envoy.filters.listener.tls_inspector");
+}
+
+const std::string& Filter::failureReasonKey() {
+  CONSTRUCT_ON_FIRST_USE(std::string, "failure_reason");
+}
+
+const std::string& Filter::failureReasonClientHelloTooLarge() {
+  CONSTRUCT_ON_FIRST_USE(std::string, "ClientHelloTooLarge");
+}
+
+const std::string& Filter::failureReasonClientHelloNotDetected() {
+  CONSTRUCT_ON_FIRST_USE(std::string, "ClientHelloNotDetected");
+}
+
 } // namespace TlsInspector
 } // namespace ListenerFilters
 } // namespace Extensions

source/extensions/filters/listener/tls_inspector/tls_inspector.h

@@ -47,6 +47,7 @@ enum class ParseState {
   // Parser reports unrecoverable error.
   Error
 };
+
 /**
  * Global configuration for TLS inspector.
  */
@@ -93,6 +94,11 @@ class Filter : public Network::ListenerFilter, Logger::Loggable<Logger::Id::filt
   Network::FilterStatus onData(Network::ListenerFilterBuffer& buffer) override;
   size_t maxReadBytes() const override { return requested_read_bytes_; }
 
+  static const std::string& dynamicMetadataKey();
+  static const std::string& failureReasonKey();
+  static const std::string& failureReasonClientHelloTooLarge();
+  static const std::string& failureReasonClientHelloNotDetected();
+
 private:
   ParseState parseClientHello(const void* data, size_t len, uint64_t bytes_already_processed);
   ParseState onRead();
@@ -101,6 +107,7 @@ class Filter : public Network::ListenerFilter, Logger::Loggable<Logger::Id::filt
   void createJA3Hash(const SSL_CLIENT_HELLO* ssl_client_hello);
   void createJA4Hash(const SSL_CLIENT_HELLO* ssl_client_hello);
   uint32_t maxConfigReadBytes() const { return config_->maxClientHelloSize(); }
+  void setDynamicMetadata(absl::string_view failure_reason);
 
   ConfigSharedPtr config_;
   Network::ListenerFilterCallbacks* cb_{};

test/extensions/filters/listener/tls_inspector/tls_inspector_integration_test.cc

@@ -260,6 +260,36 @@ TEST_P(TlsInspectorIntegrationTest, ContinueOnListenerTimeout) {
   EXPECT_THAT(waitForAccessLog(listener_access_log_name_), testing::Eq("-"));
 }
 
+TEST_P(TlsInspectorIntegrationTest, TlsInspectorMetadataPopulatedInAccessLog) {
+  initializeWithTlsInspector(
+      /*ssl_client=*/false,
+      /*log_format=*/"%DYNAMIC_METADATA(envoy.filters.listener.tls_inspector:failure_reason)%",
+      false, false, false);
+  Network::Address::InstanceConstSharedPtr address =
+      Ssl::getSslAddress(version_, lookupPort("echo"));
+  context_ =
+      Ssl::createClientSslTransportSocketFactory(/*ssl_options=*/{}, *context_manager_, *api_);
+  auto transport_socket_factory = std::make_unique<Network::RawBufferSocketFactory>();
+  Network::TransportSocketPtr transport_socket =
+      transport_socket_factory->createTransportSocket(nullptr, nullptr);
+  client_ = dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(),
+                                                std::move(transport_socket), nullptr, nullptr);
+  std::shared_ptr<WaitForPayloadReader> payload_reader =
+      std::make_shared<WaitForPayloadReader>(*dispatcher_);
+  client_->addReadFilter(payload_reader);
+  client_->addConnectionCallbacks(connect_callbacks_);
+  client_->connect();
+  Buffer::OwnedImpl buffer("fake data");
+  client_->write(buffer, false);
+  while (!connect_callbacks_.connected() && !connect_callbacks_.closed()) {
+    dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
+  }
+  // The timeout is set as one seconds, advance 2 seconds to trigger the timeout.
+  timeSystem().advanceTimeWaitImpl(std::chrono::milliseconds(2000));
+  client_->close(Network::ConnectionCloseType::NoFlush);
+  EXPECT_THAT(waitForAccessLog(listener_access_log_name_), testing::Eq("ClientHelloNotDetected"));
+}
+
 // The `JA3` fingerprint is correct in the access log.
 TEST_P(TlsInspectorIntegrationTest, JA3FingerprintIsSet) {
   // These TLS options will create a client hello message with

test/extensions/filters/listener/tls_inspector/tls_inspector_test.cc

@@ -103,7 +103,7 @@ class TlsInspectorTest : public testing::TestWithParam<std::tuple<uint16_t, uint
   Stats::TestUtil::TestStore store_;
   ConfigSharedPtr cfg_;
   std::unique_ptr<Filter> filter_;
-  Network::MockListenerFilterCallbacks cb_;
+  NiceMock<Network::MockListenerFilterCallbacks> cb_;
   Network::MockConnectionSocket socket_;
   NiceMock<Event::MockDispatcher> dispatcher_;
   Event::FileReadyCb file_event_callback_;
@@ -281,6 +281,12 @@ TEST_P(TlsInspectorTest, ClientHelloTooBig) {
   mockSysCallForPeek(client_hello, true);
   EXPECT_CALL(socket_, detectedTransportProtocol()).Times(::testing::AnyNumber());
   EXPECT_TRUE(file_event_callback_(Event::FileReadyType::Read).ok());
+
+  Protobuf::Struct expected_metadata;
+  auto& fields = *expected_metadata.mutable_fields();
+  fields[Filter::failureReasonKey()].set_string_value(Filter::failureReasonClientHelloTooLarge());
+  EXPECT_CALL(cb_, setDynamicMetadata(Filter::dynamicMetadataKey(), ProtoEq(expected_metadata)));
+
   auto state = filter_->onData(*buffer_);
   EXPECT_EQ(Network::FilterStatus::StopIteration, state);
   EXPECT_EQ(1, cfg_->stats().client_hello_too_large_.value());
@@ -409,6 +415,13 @@ TEST_P(TlsInspectorTest, NotSsl) {
   mockSysCallForPeek(data);
   // trigger the event to copy the client hello message into buffer:q
   EXPECT_TRUE(file_event_callback_(Event::FileReadyType::Read).ok());
+
+  Protobuf::Struct expected_metadata;
+  auto& fields = *expected_metadata.mutable_fields();
+  fields[Filter::failureReasonKey()].set_string_value(
+      Filter::failureReasonClientHelloNotDetected());
+  EXPECT_CALL(cb_, setDynamicMetadata(Filter::dynamicMetadataKey(), ProtoEq(expected_metadata)));
+
   auto state = filter_->onData(*buffer_);
   EXPECT_EQ(Network::FilterStatus::Continue, state);
   EXPECT_EQ(1, cfg_->stats().tls_not_found_.value());