Skip to content

Avoid finalizer race in the SslStreamNetworkStreamTests cert creation - #57381

Merged
danmoseley merged 1 commit into
dotnet:mainfrom
bartonjs:sslstream_cert_chains
Aug 14, 2021
Merged

Avoid finalizer race in the SslStreamNetworkStreamTests cert creation#57381
danmoseley merged 1 commit into
dotnet:mainfrom
bartonjs:sslstream_cert_chains

Conversation

@bartonjs

@bartonjs bartonjs commented Aug 13, 2021

Copy link
Copy Markdown
Member

The TestHelpers.GenerateCertificates implementation for extending the length of the
certificate chain had a finalizable value race condition: if the endEntity certificate got
finalized between the time the private key handle was obtained and the key got rebound
to the new copy, then the key object was pointing at a deleted key and the operation failed.

Rather than rebuild the chain after the fact, this change just asks the PKI builder to build a longer chain.

Fixes #53597.

@bartonjs
bartonjs requested a review from wfurt August 13, 2021 23:20
@ghost

ghost commented Aug 13, 2021

Copy link
Copy Markdown

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

Issue Details

The TestHelpers.GenerateCertificates implementation for extending the length of the
certificate chain had a finalizable value race condition: if the endEntity certificate got
finalized between the time the private key handle was obtained and the key got rebound
to the new copy, then the key object was pointing at a deleted key and the operation failed.

Rather than rebuild the chain after the fact, this change just asks the PKI builder to build a longer chain.

Author: bartonjs
Assignees: -
Labels:

area-System.Net.Security

Milestone: -

@wfurt wfurt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@danmoseley

Copy link
Copy Markdown
Contributor

Failing test is unrelated and already "fixed"

@danmoseley
danmoseley merged commit 73b249e into dotnet:main Aug 14, 2021
@bartonjs
bartonjs deleted the sslstream_cert_chains branch August 14, 2021 05:46
@karelz karelz added this to the 6.0.0 milestone Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SslStreamNetworkStreamTest failures on Mac

4 participants