Repository navigation
Fix cached Windows TLS credential lifetime race - #135337
Merged
rzikm merged 1 commit intoOct 8, 2026
Merged
Conversation
Acquire a reference before returning cached credentials and balance it across both handshake paths, replacement, failure, and disposal. Recover closed-handle races only during cache reference acquisition. Add deterministic eviction and cleanup coverage for client/server authentication, TLS 1.2/1.3, client certificates, disabled resumption, and rejection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa96f0b5-bd4d-400d-baf1-9268a7f37ed0
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
wfurt
approved these changes
Oct 7, 2026
Member
Author
|
/ba-g SmtpClient failures are unrelated (and reverted already), no SslStream-related tests are failing |
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
2 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.
Windows SslStream handshakes can throw ObjectDisposedException when credential-cache scavenging releases the last reference after cache lookup but before ASC/ISC acquires its reference. The legacy retry loop does not handle that exception, and the race affects both handshake implementations.
Acquire a credential reference before returning a cache hit and retain it through the PAL call. Release it on handshake success/failure, credential replacement, and stream disposal. Recover an already-closed handle only during cache-reference acquisition, treating it as a cache miss rather than catching arbitrary handshake exceptions. Cache keys and public authentication behavior are unchanged.
Regression coverage
Add 28 process-isolated eviction cases covering client/server roles, TLS 1.2/1.3, both handshake implementations, client-certificate reselection, disabled resumption, and certificate rejection. Eviction is triggered synchronously by the cache-hit diagnostic event, before SSPI uses the credential. The tests also verify that evicted handles close after stream disposal.
All 28 final cases fail with the reported ObjectDisposedException against the preserved unchanged x64 Release product and pass against the fixed x64/x86 Debug and Release products.
Validation
Builds used a Q: subst mapping to avoid long paths. Windows x64/x86 runtime and library builds passed, as did the managed product builds for all platform targets.
Release omits 32 existing DEBUG-only resumption cases; both Debug suites exercise them. One earlier x64 Debug run failed a TLS-resumption flag assertion. Focused baseline/fixed checks and subsequent full suites passed; that failure's cause was not established. Runtime testing was Windows-only; Linux/macOS and older Windows versions were not exercised.
A BenchmarkDotNet ShortRun of cache lookup plus caller reference release measured 50.76 ns before and 59.30 ns after, with zero allocations in both. These are isolated cache-operation measurements, not end-to-end handshake timings.
Fixes: #133904
Resolves #133904
Note
The code changes and this description were generated with GitHub Copilot assistance.