diff --git a/api/envoy/extensions/filters/listener/tls_inspector/v3/tls_inspector.proto b/api/envoy/extensions/filters/listener/tls_inspector/v3/tls_inspector.proto index b4fe6d72f994d..365af523a94d1 100644 --- a/api/envoy/extensions/filters/listener/tls_inspector/v3/tls_inspector.proto +++ b/api/envoy/extensions/filters/listener/tls_inspector/v3/tls_inspector.proto @@ -18,6 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Allows detecting whether the transport appears to be TLS or plaintext. // [#extension: envoy.filters.listener.tls_inspector] +// [#next-free-field: 6] message TlsInspector { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.listener.tls_inspector.v2.TlsInspector"; @@ -48,4 +49,11 @@ message TlsInspector { // counter to be incremented if the ClientHello message is over implementation defined limit // (currently 16Kb). bool close_connection_on_client_hello_parsing_errors = 4; + + // The maximum size in bytes of the ClientHello that the tls_inspector will + // process. If the ClientHello is larger than this size, the tls_inspector + // will stop processing and indicate failure. If not defined, defaults to + // 16KiB. + google.protobuf.UInt32Value max_client_hello_size = 5 + [(validate.rules).uint32 = {lte: 16384 gt: 255}]; } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 05c15f9632224..5cdabee11648a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -426,6 +426,9 @@ new_features: change: | Propagate the transport error from the tls_inspector to the DownstreamTransportFailureReason in StreamInfo for access logging prior to the TLS handshake. +- area: tls_inspector + change: | + Add configuration parameter to TLS inspector for maximum acceptable client hello size. - area: ext_proc change: | Add support for forwarding cluster metadata to ext_proc server. diff --git a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc index a987c0b878c34..291efb7844ee4 100644 --- a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc +++ b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc @@ -50,8 +50,7 @@ const unsigned Config::TLS_MAX_SUPPORTED_VERSION = TLS1_3_VERSION; Config::Config( Stats::Scope& scope, - const envoy::extensions::filters::listener::tls_inspector::v3::TlsInspector& proto_config, - uint32_t max_client_hello_size) + const envoy::extensions::filters::listener::tls_inspector::v3::TlsInspector& proto_config) : stats_{ALL_TLS_INSPECTOR_STATS(POOL_COUNTER_PREFIX(scope, "tls_inspector."), POOL_HISTOGRAM_PREFIX(scope, "tls_inspector."))}, ssl_ctx_(SSL_CTX_new(TLS_with_buffers_method())), @@ -61,11 +60,12 @@ Config::Config( PROTOBUF_GET_WRAPPED_OR_DEFAULT(proto_config, enable_ja4_fingerprinting, false)), close_connection_on_client_hello_parsing_errors_( proto_config.close_connection_on_client_hello_parsing_errors()), - max_client_hello_size_(max_client_hello_size), + max_client_hello_size_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(proto_config, max_client_hello_size, + TLS_MAX_CLIENT_HELLO)), initial_read_buffer_size_( std::min(PROTOBUF_GET_WRAPPED_OR_DEFAULT(proto_config, initial_read_buffer_size, - max_client_hello_size), - max_client_hello_size)) { + max_client_hello_size_), + max_client_hello_size_)) { if (max_client_hello_size_ > TLS_MAX_CLIENT_HELLO) { throw EnvoyException(fmt::format("max_client_hello_size of {} is greater than maximum of {}.", max_client_hello_size_, size_t(TLS_MAX_CLIENT_HELLO))); diff --git a/source/extensions/filters/listener/tls_inspector/tls_inspector.h b/source/extensions/filters/listener/tls_inspector/tls_inspector.h index 2920fc16a7007..e969e2f26815c 100644 --- a/source/extensions/filters/listener/tls_inspector/tls_inspector.h +++ b/source/extensions/filters/listener/tls_inspector/tls_inspector.h @@ -54,8 +54,7 @@ enum class ParseState { class Config { public: Config(Stats::Scope& scope, - const envoy::extensions::filters::listener::tls_inspector::v3::TlsInspector& proto_config, - uint32_t max_client_hello_size = TLS_MAX_CLIENT_HELLO); + const envoy::extensions::filters::listener::tls_inspector::v3::TlsInspector& proto_config); const TlsInspectorStats& stats() const { return stats_; } bssl::UniquePtr newSsl(); diff --git a/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.cc b/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.cc index 8740916006b35..2718bfb6cb2bb 100644 --- a/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.cc +++ b/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.cc @@ -22,12 +22,7 @@ DEFINE_PROTO_FUZZER( ConfigSharedPtr cfg; try { - if (input.max_size() == 0) { - // If max_size not set, use default constructor - cfg = std::make_shared(*store.rootScope(), input.config()); - } else { - cfg = std::make_shared(*store.rootScope(), input.config(), input.max_size()); - } + cfg = std::make_shared(*store.rootScope(), input.config()); } catch (const Envoy::EnvoyException& e) { ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what()); return; diff --git a/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.proto b/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.proto index fec7a5a33a24d..a7748e26a80ef 100644 --- a/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.proto +++ b/test/extensions/filters/listener/tls_inspector/tls_inspector_fuzz_test.proto @@ -9,7 +9,6 @@ import "validate/validate.proto"; message TlsInspectorTestCase { envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector config = 1 [(validate.rules).message.required = true]; - uint32 max_size = 2 [(validate.rules).uint32.lte = 16384]; test.extensions.filters.listener.FilterFuzzWithDataTestCase fuzzed = 3 [(validate.rules).message.required = true]; } diff --git a/test/extensions/filters/listener/tls_inspector/tls_inspector_test.cc b/test/extensions/filters/listener/tls_inspector/tls_inspector_test.cc index 40fadde62303f..8c187d7e6b137 100644 --- a/test/extensions/filters/listener/tls_inspector/tls_inspector_test.cc +++ b/test/extensions/filters/listener/tls_inspector/tls_inspector_test.cc @@ -122,9 +122,9 @@ INSTANTIATE_TEST_SUITE_P(TlsProtocolVersions, TlsInspectorTest, // Test that an exception is thrown for an invalid value for max_client_hello_size TEST_P(TlsInspectorTest, MaxClientHelloSize) { envoy::extensions::filters::listener::tls_inspector::v3::TlsInspector proto_config; - EXPECT_THROW_WITH_MESSAGE( - Config(*store_.rootScope(), proto_config, Config::TLS_MAX_CLIENT_HELLO + 1), EnvoyException, - "max_client_hello_size of 16385 is greater than maximum of 16384."); + proto_config.mutable_max_client_hello_size()->set_value(Config::TLS_MAX_CLIENT_HELLO + 1); + EXPECT_THROW_WITH_MESSAGE(Config(*store_.rootScope(), proto_config), EnvoyException, + "max_client_hello_size of 16385 is greater than maximum of 16384."); } // Test that a ClientHello with an SNI value causes the correct name notification. @@ -518,7 +518,8 @@ TEST_P(TlsInspectorTest, RequestedMaxReadSizeDoesNotGoBeyondMaxSize) { const uint32_t initial_buffer_size = 15; const size_t max_size = 50; proto_config.mutable_initial_read_buffer_size()->set_value(initial_buffer_size); - cfg_ = std::make_shared(*store_.rootScope(), proto_config, max_size); + proto_config.mutable_max_client_hello_size()->set_value(max_size); + cfg_ = std::make_shared(*store_.rootScope(), proto_config); buffer_ = std::make_unique( *io_handle_, dispatcher_, [](bool) {}, [](Network::ListenerFilterBuffer&) {}, cfg_->initialReadBufferSize() == 0, cfg_->initialReadBufferSize());