Fix Duplicate ownership state machine under concurrency for NCrypt handles

[vcsjones]

Duplicating NCrypt safe handles is not currently thread safe. This is observable through CngKey.Handle which calls Duplicate on a SafeNCryptKeyHandle. The crux of the issue is that two threads can both observe _ownershipState == Owner and enter DuplicateOwnerHandle(). Each creates a different holder for the same native handle. The first transitions the original handle to Duplicate, the second then hits the assertion when it attempts the same transition.

This assert was observed in #130760.

In a release build, this can result in an invalid NCryptKeyHandle getting passed to native functions.

1. Both threads create separate holders for the same raw NCrypt handle.
2. The later thread overwrites the original handle's  _holder .
3. Managed reference counts no longer represent the native handle's actual users.
4. The winning holder can call NCryptFreeObject while duplicates associated with an overwritten holder remain open.

This simplest solution is to take a lock, which will unfortunately ding performance a little bit when accessing Handle.

A unit test was added to attempt racing the handles. Before the fix, this test would fail quickly with

Error Message:
Assert.All() Failure: 7 out of 8 items in the collection did not pass.
[0]: Item:  Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException: Method Debug.Fail failed with '_ownershipState == OwnershipState.Owner
', and was translated to Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException to avoid terminating the process hosting the test.
    at Microsoft.Win32.SafeHandles.SafeNCryptHandle.DuplicateOwnerHandle[T]() in E:\code\runtime\src\libraries\System.Security.Cryptography\src\Microsoft\Win32\SafeHandles\NCryptSafeHandles.cs:line 263
    at Microsoft.Win32.SafeHandles.SafeNCryptHandle.Duplicate[T]() in E:\code\runtime\src\libraries\System.Security.Cryptography\src\Microsoft\Win32\SafeHandles\NCryptSafeHandles.cs:line 219
    at Microsoft.Win32.SafeHandles.SafeNCryptKeyHandle.Duplicate() in E:\code\runtime\src\libraries\System.Security.Cryptography\src\Microsoft\Win32\SafeHandles\NCryptSafeHandles.cs:line 364
    at System.Security.Cryptography.CngKey.get_Handle() in E:\code\runtime\src\libraries\System.Security.Cryptography\src\System\Security\Cryptography\CngKey.StandardProperties.cs:line 93
    at System.Security.Cryptography.Tests.CngKeyTests.<>c__DisplayClass3_1.<Handle_ConcurrentFirstAccess>b__0() in E:\code\runtime\src\libraries\System.Security.Cryptography\tests\CngKeyTests.cs:line 86
    at System.Threading.Thread.StartCallback(Thread* pThread) in E:\code\runtime\src\coreclr\System.Private.CoreLib\src\System\Threading\Thread.CoreCLR.cs:line 117

After the lock the test consistently passes.

Doing this lock-free is non-trivial. There are multiple fields we need to ensure get published atomically, namely _ownershipState, _holder, and _parentHandle. However we avoid the lock entirely unless the current state is OwnershipState.Owner.

Fixes #130760

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 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]

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

[Copilot]

Pull request overview

This PR addresses a concurrency bug in the NCrypt safe-handle duplication logic by synchronizing the first “owner → duplicate” transition so concurrent CngKey.Handle callers can’t create competing holders for the same native handle.

Changes:

• Make SafeNCryptHandle.Duplicate<T>() thread-safe for the initial duplication by guarding the owner-state transition with a lazily-initialized lock and making _ownershipState volatile.
• Add a Windows-only regression test that races concurrent CngKey.Handle first access and validates that duplicated handles remain usable after disposing the original key and most duplicates.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/src/Microsoft/Win32/SafeHandles/NCryptSafeHandles.cs	Adds synchronization + volatile state publication around the first duplication transition.
src/libraries/System.Security.Cryptography/tests/CngKeyTests.cs	Adds a concurrency regression test for CngKey.Handle duplication.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0 new

[bartonjs]

/backport to release/8.0-staging

[github-actions]

Started backporting to release/8.0-staging (link to workflow run)

[bartonjs]

/backport to release/9.0-staging

[github-actions]

Started backporting to release/9.0-staging (link to workflow run)

[bartonjs]

/backport to release/10.0

[github-actions]

Started backporting to release/10.0 (link to workflow run)

[github-actions]

@bartonjs backporting to release/8.0-staging failed, the patch most likely resulted in conflicts. Please backport manually!

git am output

$ git cherry-pick 2e77bf12d7af1f8b639b20bf8307655b13e56ab8

CONFLICT (modify/delete): src/libraries/System.Security.Cryptography/tests/CngKeyTests.cs deleted in HEAD and modified in 2e77bf12d7a (Fix Duplicate ownership state machine under concurrency for NCrypt handles).  Version 2e77bf12d7a (Fix Duplicate ownership state machine under concurrency for NCrypt handles) of src/libraries/System.Security.Cryptography/tests/CngKeyTests.cs left in tree.
error: could not apply 2e77bf12d7a... Fix Duplicate ownership state machine under concurrency for NCrypt handles
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix Duplicate ownership state machine under concurrency for NCrypt handles
Using index info to reconstruct a base tree...
A	src/libraries/System.Security.Cryptography/tests/CngKeyTests.cs
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): src/libraries/System.Security.Cryptography/tests/CngKeyTests.cs deleted in HEAD and modified in Fix Duplicate ownership state machine under concurrency for NCrypt handles.  Version Fix Duplicate ownership state machine under concurrency for NCrypt handles of src/libraries/System.Security.Cryptography/tests/CngKeyTests.cs left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix Duplicate ownership state machine under concurrency for NCrypt handles
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output