Fix thread-safety issue in IndirectSignatureFactory hash computation#192
Merged
Fix thread-safety issue in IndirectSignatureFactory hash computation#192
Conversation
InternalHashAlgorithm is a shared instance cached in the factory. HashAlgorithm.ComputeHash() is not thread-safe and throws CryptographicException under concurrent usage. The fix creates a fresh HashAlgorithm instance per ComputeHash call via a CreateHashAlgorithm() helper method, ensuring thread safety without contention. Added regression tests that exercise the factory from multiple threads concurrently to validate both sync and async code paths. Fixes #191 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CoseX509Thumprint cached a shared HashAlgorithm instance (Hasher) and reused it in Match(), which is not thread-safe under concurrent calls. Replaced with per-call CreateHashAlgorithm() that returns fresh disposable instances, matching the same pattern used for the IndirectSignatureFactory fix. Added concurrent Match and construction regression tests. Related to #191 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tance from property Per review feedback, removed the cached InternalHashAlgorithm field entirely. The public HashAlgorithm property now returns a fresh instance per access via CreateHashAlgorithm(), preserving the public contract while eliminating the thread-unsafe shared state. HashLength and InternalCoseHashAlgorithm are now derived from HashAlgorithmName via static helpers. Dispose is now a no-op since there are no cached resources to release. CoseHashV path (obsoleted) is already safe — its constructors create their own using HashAlgorithm instances internally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The HashValue property setter created a HashAlgorithm via GetHashAlgorithmFromCoseHashAlgorithm for length validation but never disposed it. Added using to ensure proper cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
elantiguamsft
approved these changes
Apr 7, 2026
NN2000X
approved these changes
Apr 7, 2026
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
4 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.
Summary
Fixes #191 —
IndirectSignatureFactory.InternalHashAlgorithmis not thread-safe and throwsCryptographicExceptionunder concurrent usage.Problem
The
IndirectSignatureFactoryclass caches a singleHashAlgorithminstance (InternalHashAlgorithm) and reuses it across allComputeHashcalls.HashAlgorithm.ComputeHash()is not thread-safe, causingCryptographicException: Concurrent operations from multiple threads on this type are not supported.under high concurrency.Fix
CreateHashAlgorithm()helper method that creates a freshHashAlgorithminstance per callInternalHashAlgorithm.ComputeHash()calls in Direct, CoseHashEnvelope, and their async variants withusingblocks that create and dispose thread-local instancesInternalHashAlgorithmis retained for the publicHashAlgorithmproperty andHashSizecomputation (backward compatible)Testing
ConcurrentHashComputationShouldNotThrow— hammers the factory withParallel.Forfrom many threadsConcurrentAsyncHashComputationShouldNotThrow— launches many concurrent async tasksAll 99 existing tests continue to pass.