tls: add cache limit and idle timeout to on-demand secret selector#46380
Open
botengyao wants to merge 5 commits into
Open
tls: add cache limit and idle timeout to on-demand secret selector#46380botengyao wants to merge 5 commits into
botengyao wants to merge 5 commits into
Conversation
Cap the number of cached secrets (max_secrets, default 1024) and optionally evict secrets unused for cache_idle_timeout. Overflowed handshakes are rejected and counted in cert_overflow; idle evictions cancel the SDS subscription and are counted in cert_evicted. Signed-off-by: Boteng Yao <botengyao@gmail.com>
- Reclaim pending entries with no certificate and no live handshakes when the cache is full instead of rejecting, so abandoned fetches for unknown names cannot permanently occupy the cache (new cert_reclaimed counter). - Require cache_idle_timeout >= 1ms to prevent a busy sweep timer. - Only track handshake use when idle eviction is enabled, and share the use flag across certificate rotations so late worker marks are not lost. - Evict within (1, 2] idle timeouts using a single grace period; SDS updates no longer count as activity. - Pin configured prefetch names across removal and re-fetch. - Compact expired pending-handshake callbacks; guard deferred removals with an entry generation; escape peer-controlled names in logs. - Add deterministic sweep and reclaim unit tests, plus integration tests for poisoning recovery, upstream overflow, and eviction with an established connection. Signed-off-by: Boteng Yao <botengyao@gmail.com>
- max_secrets is now opt-in: unset preserves the previous unbounded behavior, avoiding a breaking default on an active API. A warning is logged when the limit is set without cache_idle_timeout, since resolved secrets are only released by SDS removal or idle eviction. - Compact expired pending-handshake callbacks at geometric size thresholds, keeping insertion amortized constant instead of quadratic under a burst of handshakes for one unresolved secret. - Add a resolved-name cache poisoning recovery test via idle eviction, plus tests for the unlimited default, the compaction bound with live handshakes, and the load-time warning. Signed-off-by: Boteng Yao <botengyao@gmail.com>
- Skip the cross-worker context removal for entries that never installed a certificate (reclaimed or pending evictions), and erase the contexts of all secrets evicted in one sweep with a single batched thread local update instead of one per name, avoiding an O(evictions x workers) callback burst. Removals no longer count towards cert_updated. - Bump the entry generation on every successful SDS certificate update so that a deferred removal cannot erase a certificate the server published after signaling the removal: the later message wins. Signed-off-by: Boteng Yao <botengyao@gmail.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Boteng Yao <botengyao@gmail.com>
Member
Author
|
/docs |
|
Docs for this Pull Request will be rendered here: https://storage.googleapis.com/envoy-cncf-pr/46380/docs/index.html The docs are (re-)rendered each time the CI |
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
3 participants
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: add cache limit and idle timeout to on-demand secret selector
Additional Description:
The on-demand secret certificate selector cached one SDS subscription per unique mapped secret name without bound. This adds two opt-in cache bounds:
max_secrets: caps the number of fetched and cached secrets. When the cache is full, admitting a new secret first reclaims an entry with no certificate and no pending handshakes (newcert_reclaimedcounter), so abandoned fetches for unknown names cannot permanently occupy the cache; otherwise the handshake is rejected (newcert_overflowcounter). Handshakes using cached secrets are unaffected, and slots are freed when the SDS server removes a resource (including via resource TTL expiry). Unset preserves the previous unbounded behavior.cache_idle_timeout(>= 1ms): evicts secrets unused by any handshake for the configured duration, canceling their SDS subscriptions and incrementing the newcert_evictedcounter. Prefetched secrets are pinned and never idle-evicted. Since resolved secrets are not reclaimed by the limit, a warning is logged when onlymax_secretsis set; the two settings are recommended together when peers control the mapped secret name.This PR was written with AI assistance and reviewed by the author.
Risk Level: low (both bounds are opt-in; unset configurations keep the existing behavior)
Testing: deterministic unit tests for sweep timing, reclaim, and compaction; integration tests for overflow, cache poisoning recovery (pending and resolved), idle eviction with established connections, and prefetch pinning
Docs Changes: docs/root/configuration/security/secret.rst
Release Notes: new_features entry included
Platform Specific Features: n/a