WIP: tls: scope upstream session cache by sni#45982
Draft
dio wants to merge 3 commits into
Draft
Conversation
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Message: tls: scope upstream session cache by sni
Additional Description:
This fixes upstream client TLS session cache keying when one
ClientContextImplis used for multiple logical upstream hosts. Previously, client sessions were cached at the context level, so a session learned while connecting with one effective SNI could be offered on a later connection using a different effective SNI.This is treated as a bug fix because the old cache key was broader than the TLS identity used for the connection. Session resumption should be scoped to the logical server identity represented by the effective SNI, not just to the Envoy client context object that created the connection.
If reviewers consider the
max_session_keyssemantics change more prominent than the cache-keying fix, I can move the changelog entry to minor behavior changes.The cache is now scoped by effective SNI using the same precedence as ClientHello SNI selection:
server_nameoverrideauto_host_snihost hostnamesniFor example, with a single upstream TLS context:
a.example.coma.example.comb.example.coma.example.comcached sessionb.example.coma.example.comagaina.example.combucketThe existing
max_session_keyssetting now limits sessions within each effective SNI bucket. A fixed internal cap bounds the number of distinct SNI buckets. When the distinct-SNI cap is exceeded, Envoy evicts the least-recently-used SNI bucket, dropping only its cached sessions. Future connections for the evicted SNI still work normally; they perform a full TLS handshake and may populate the cache again.This behavior is guarded by the default-on runtime feature
envoy.reloadable_features.scope_upstream_tls_session_cache_by_sni. Disabling the runtime feature restores the previous context-wide session cache behavior as a rollback path while the guard exists.No dynamic module ABI or config API changes are included in this PR.
AI assistance was used to help implement and test this change. I reviewed and understand the submitted code.
Risk Level: Medium
Testing:
bazel test --config=clang -c dbg //test/common/tls:ssl_socket_test --test_filter=*ClientSessionCache*PATH=/tmp/envoy-format-venv/bin:/Library/Developer/CommandLineTools/usr/bin:/opt/homebrew/bin:$PATH ASPELL_DICT=tools/spelling/spelling_dictionary.txt tools/local_fix_format.sh -skip-bazel -maingit diff --checkDocs Changes: Added changelog fragment.
Release Notes: Added bug fix note for upstream TLS session cache scoping by SNI.
Platform Specific Features: N/A
Issues:
#45962