Skip to content

Remove unused client certificate from TLS host resumption test - #134422

Merged
rzikm merged 1 commit into
dotnet:mainfrom
rzikm:rzikm/tls-resumption-failures
Sep 23, 2026
Merged

rzikm merged 1 commit into
dotnet:mainfrom
rzikm:rzikm/tls-resumption-failures

Conversation

@rzikm

@rzikm rzikm commented Sep 22, 2026

Copy link
Copy Markdown
Member

DifferentHost_NoResume supplies a client certificate that the server never requests. With a cold certificate-credential cache on Windows, this leads to fresh anonymous credentials on successive connections, preventing the positive resumption check from reaching the host-change assertion.

Remove the unused ClientCertificateContext. The initial handshake, resumption prerequisite, and negative assertion after changing the hostname remain unchanged. No product behavior changes.

Validated with Debug libraries/tests on Windows x64: all three protocol variants failed in isolation before the edit and passed afterward, including 10 additional fresh-process runs (30 passes, no skips). Functional innerloop: 5,275 passed, 36 existing skips; unit tests: 117 passed, 4 existing skips. No failures after the change. Linux and Windows x86 were not run.

Addresses the host-test setup behind #132851; the separate encryption-policy and Linux client-certificate failures are not claimed fixed.

Note

This PR was created with assistance from GitHub Copilot.

Keep DifferentHost_NoResume independent of certificate credential cache warm-up while preserving resumption and host-isolation assertions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
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.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

This test-only PR removes an unused client certificate context from the TLS host-resumption test, improving reliability without changing product behavior.

Changes:

  • Simplifies DifferentHost_NoResume while preserving its resumption and hostname assertions.
File Description
src/​libraries/​System.Net.Security/​tests/​FunctionalTests/​SslStreamAllowTlsResumeTests.cs Removes the unused client certificate context.

@wfurt

wfurt commented Sep 22, 2026

Copy link
Copy Markdown
Member

This is curious as unused object has impact on other tests. That looks like Test bug or perhaps product...? While the fix may stabilize the tests it somewhat concerns me. Maybe we should sweep the tests and use unique SNI everywhere....???

@rzikm

rzikm commented Sep 23, 2026

Copy link
Copy Markdown
Member Author

on Windows we fallback on anonymous credentials for the first flight, but the certificate is part of the credential cache key, so maybe there is some interaction there.

@rzikm

rzikm commented Sep 23, 2026

Copy link
Copy Markdown
Member Author

/ba-g test failure is unrelated

@rzikm
rzikm merged commit 7e1549a into dotnet:main Sep 23, 2026
83 of 86 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants