tls_inspector: Add missing TLS version check#44274
tls_inspector: Add missing TLS version check#44274yanavlasov wants to merge 6 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Yan Avlasov <yavlasov@google.com>
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a check in the tls_inspector listener filter to enforce minimum (1.0) and maximum (1.3) client TLS versions, managed by a new runtime guard. The implementation captures the TLS version during the handshake and triggers a failure if it falls outside the supported range, updating statistics and metadata accordingly. The PR also includes new unit tests and a helper for constructing TLS ClientHello messages. Review feedback suggests adding the missing header for std::iota, fixing typos in the changelog, and using absl::string_view for string parameters in test helpers to avoid unnecessary copies.
Signed-off-by: Yan Avlasov <yavlasov@google.com>
| } | ||
|
|
||
| const std::string& Filter::failureReasonClientHelloWrongTlsVersion() { | ||
| CONSTRUCT_ON_FIRST_USE(std::string, "ClientHelloWrongTlsVersion"); |
There was a problem hiding this comment.
| CONSTRUCT_ON_FIRST_USE(std::string, "ClientHelloWrongTlsVersion"); | |
| CONSTRUCT_ON_FIRST_USE(std::string, "ClientHelloInvalidTlsVersion"); |
| init(); | ||
| const std::string servername("example.com"); | ||
| // Generate ClientHello with SSL v3 version (0x0300) | ||
| std::vector<uint8_t> client_hello = CreateClientHello(servername, 0, true, 0x0300); |
There was a problem hiding this comment.
Should there also be a test for version too high?
2 participants
Commit Message:
tls_inspector defined but never implemented check for client's TLS version. This allowed connections with a too low version to be allowed.
Adding runtime guard in case someone depends on the broken functionality.
Risk Level: medium, flag protected.
Testing: unit tests
Docs Changes: n/a
Release Notes: yes
Platform Specific Features: n/a
Runtime guard: envoy.reloadable_features.tls_inspector_enforce_client_tls_version