Fix thread-safety issue in IndirectSignatureFactory hash computation

CoseHandler.Tests/CoseX509ThumbprintTests.cs

@@ -50,4 +50,67 @@ public void ConstructThumbprintWithUnsupportedAlgo()
     {
         _ = new CoseX509Thumprint(SelfSignedCert1, HashAlgorithmName.SHA3_512);
     }
+
+    /// <summary>
+    /// Validates that a single <see cref="CoseX509Thumprint"/> instance can safely call
+    /// <see cref="CoseX509Thumprint.Match"/> from multiple threads concurrently without
+    /// throwing <see cref="CryptographicException"/>.
+    /// Regression test for https://github.com/microsoft/CoseSignTool/issues/191.
+    /// </summary>
+    [TestMethod]
+    public void ConcurrentMatchShouldNotThrow()
+    {
+        // Arrange — one shared thumbprint, many threads calling Match
+        CoseX509Thumprint thumbprint = new(SelfSignedCert1);
+        int degreeOfParallelism = Environment.ProcessorCount * 2;
+        int iterationsPerThread = 50;
+
+        // Act & Assert — hammer Match() from many threads at once
+        Action concurrentAction = () =>
+        {
+            Parallel.For(0, degreeOfParallelism * iterationsPerThread, new ParallelOptions { MaxDegreeOfParallelism = degreeOfParallelism }, i =>
+            {
+                // Alternate between matching and non-matching certs
+                if (i % 2 == 0)
+                {
+                    thumbprint.Match(SelfSignedCert1).Should().BeTrue();
+                }
+                else
+                {
+                    thumbprint.Match(SelfSignedCert2).Should().BeFalse();
+                }
+            });
+        };
+
+        concurrentAction.Should().NotThrow<CryptographicException>();
+    }
+
+    /// <summary>
+    /// Validates that concurrent construction of <see cref="CoseX509Thumprint"/> instances
+    /// with different hash algorithms is thread-safe.
+    /// Regression test for https://github.com/microsoft/CoseSignTool/issues/191.
+    /// </summary>
+    [TestMethod]
+    public void ConcurrentConstructionAndMatchShouldNotThrow()
+    {
+        // Arrange
+        HashAlgorithmName[] algorithms = new[] { HashAlgorithmName.SHA256, HashAlgorithmName.SHA384, HashAlgorithmName.SHA512 };
+        int degreeOfParallelism = Environment.ProcessorCount * 2;
+        int iterationsPerThread = 30;
+
+        // Act & Assert — create thumbprints and match from many threads
+        Action concurrentAction = () =>
+        {
+            Parallel.For(0, degreeOfParallelism * iterationsPerThread, new ParallelOptions { MaxDegreeOfParallelism = degreeOfParallelism }, i =>
+            {
+                HashAlgorithmName algo = algorithms[i % algorithms.Length];
+                CoseX509Thumprint thumbprint = new(SelfSignedCert1, algo);
+
+                thumbprint.Match(SelfSignedCert1).Should().BeTrue();
+                thumbprint.Match(SelfSignedCert2).Should().BeFalse();
+            });
+        };
+
+        concurrentAction.Should().NotThrow<CryptographicException>();
+    }
 }

CoseIndirectSignature.Tests/IndirectSignatureFactoryTests.cs

@@ -502,4 +502,81 @@ public async Task TestCreateIndirectSignatureAsyncWithoutPayloadLocation()
         indirectSignature.TryGetPayloadHashAlgorithm(out CoseHashAlgorithm? algo).Should().BeTrue();
         algo!.Should().Be(CoseHashAlgorithm.SHA256);
     }
+
+    /// <summary>
+    /// Validates that a single <see cref="IndirectSignatureFactory"/> instance can safely produce
+    /// indirect signatures from multiple threads concurrently without throwing
+    /// <see cref="System.Security.Cryptography.CryptographicException"/>
+    /// ("Concurrent operations from multiple threads on this type are not supported.").
+    /// Regression test for https://github.com/microsoft/CoseSignTool/issues/191.
+    /// </summary>
+    [Test]
+    public void ConcurrentHashComputationShouldNotThrow()
+    {
+        // Arrange — one shared factory, many threads
+        ICoseSigningKeyProvider coseSigningKeyProvider = TestUtils.SetupMockSigningKeyProvider();
+        using IndirectSignatureFactory factory = new();
+        int degreeOfParallelism = Environment.ProcessorCount * 2;
+        int iterationsPerThread = 20;
+
+        // Act — hammer the factory from many threads at once
+        Action concurrentAction = () =>
+        {
+            Parallel.For(0, degreeOfParallelism * iterationsPerThread, new ParallelOptions { MaxDegreeOfParallelism = degreeOfParallelism }, i =>
+            {
+                byte[] payload = new byte[128];
+                Random.Shared.NextBytes(payload);
+
+                CoseSign1Message result = factory.CreateIndirectSignature(
+                    payload,
+                    coseSigningKeyProvider,
+                    "application/test.concurrent");
+
+                // Verify each result is valid
+                result.IsIndirectSignature().Should().BeTrue();
+                result.SignatureMatches(payload).Should().BeTrue();
+            });
+        };
+
+        // Assert — no CryptographicException from concurrent ComputeHash
+        concurrentAction.Should().NotThrow<CryptographicException>();
+    }
+
+    /// <summary>
+    /// Validates that the async indirect signature creation path is also safe under
+    /// concurrent usage from multiple threads.
+    /// Regression test for https://github.com/microsoft/CoseSignTool/issues/191.
+    /// </summary>
+    [Test]
+    public async Task ConcurrentAsyncHashComputationShouldNotThrow()
+    {
+        // Arrange — one shared factory, many concurrent tasks
+        ICoseSigningKeyProvider coseSigningKeyProvider = TestUtils.SetupMockSigningKeyProvider();
+        using IndirectSignatureFactory factory = new();
+        int concurrentTasks = Environment.ProcessorCount * 4;
+
+        // Act — launch many concurrent async operations
+        Task[] tasks = new Task[concurrentTasks];
+        for (int i = 0; i < concurrentTasks; i++)
+        {
+            tasks[i] = Task.Run(async () =>
+            {
+                byte[] payload = new byte[128];
+                Random.Shared.NextBytes(payload);
+
+                CoseSign1Message result = await factory.CreateIndirectSignatureAsync(
+                    payload,
+                    coseSigningKeyProvider,
+                    "application/test.concurrent.async");
+
+                // Verify each result is valid
+                result.IsIndirectSignature().Should().BeTrue();
+                result.SignatureMatches(payload).Should().BeTrue();
+            });
+        }
+
+        // Assert — all tasks complete without CryptographicException
+        Func<Task> awaitAll = async () => await Task.WhenAll(tasks);
+        await awaitAll.Should().NotThrowAsync<CryptographicException>();
+    }
 }

CoseIndirectSignature/CoseHashV.cs

@@ -39,7 +39,7 @@ public byte[] HashValue
             }
 
             // sanity check the length of the hash against the specified algorithm to be sure we're not allowing a mismatch.
-            HashAlgorithm algo = IndirectSignatureFactory.GetHashAlgorithmFromCoseHashAlgorithm(Algorithm);
+            using HashAlgorithm algo = IndirectSignatureFactory.GetHashAlgorithmFromCoseHashAlgorithm(Algorithm);
             if (value.Length != (algo.HashSize / 8))
             {
                 throw new ArgumentOutOfRangeException(nameof(value), @$"The hash value length of {value.Length} did not match the CoseHashAlgorithm {Algorithm} required length of {algo.HashSize / 8}");

CoseIndirectSignature/IndirectSignatureFactory.CoseHashEnvelope.cs

@@ -39,9 +39,10 @@ private object CreateIndirectSignatureWithChecksInternalCoseHashEnvelopeFormat(
 
         if (!payloadHashed)
         {
+            using HashAlgorithm hasher = this.CreateHashAlgorithm();
             hash = streamPayload != null
-                                 ? InternalHashAlgorithm.ComputeHash(streamPayload)
-                                 : InternalHashAlgorithm.ComputeHash(bytePayload!.Value.ToArray());
+                                 ? hasher.ComputeHash(streamPayload)
+                                 : hasher.ComputeHash(bytePayload!.Value.ToArray());
         }
         else
         {
@@ -100,9 +101,10 @@ private async Task<object> CreateIndirectSignatureWithChecksInternalCoseHashEnve
             // Note: HashAlgorithm.ComputeHashAsync is not available in netstandard2.0
             // For better async support in the future, consider targeting net6.0+ where
             // ComputeHashAsync is available on specific hash algorithm implementations
+            using HashAlgorithm hasher = this.CreateHashAlgorithm();
             hash = streamPayload != null
-                                 ? InternalHashAlgorithm.ComputeHash(streamPayload)
-                                 : InternalHashAlgorithm.ComputeHash(bytePayload!.Value.ToArray());
+                                 ? hasher.ComputeHash(streamPayload)
+                                 : hasher.ComputeHash(bytePayload!.Value.ToArray());
         }
         else
         {

CoseIndirectSignature/IndirectSignatureFactory.Direct.cs

@@ -35,9 +35,10 @@ private object CreateIndirectSignatureWithChecksInternalDirectFormat(
         string extendedContentType;
         if (!payloadHashed)
         {
+            using HashAlgorithm hasher = this.CreateHashAlgorithm();
             hash = streamPayload != null
-                                 ? InternalHashAlgorithm.ComputeHash(streamPayload)
-                                 : InternalHashAlgorithm.ComputeHash(bytePayload!.Value.ToArray());
+                                 ? hasher.ComputeHash(streamPayload)
+                                 : hasher.ComputeHash(bytePayload!.Value.ToArray());
             extendedContentType = ExtendContentTypeDirect(contentType, HashAlgorithmName);
         }
         else
@@ -118,9 +119,10 @@ private async Task<object> CreateIndirectSignatureWithChecksInternalDirectFormat
         string extendedContentType;
         if (!payloadHashed)
         {
+            using HashAlgorithm hasher = this.CreateHashAlgorithm();
             hash = streamPayload != null
-                                 ? InternalHashAlgorithm.ComputeHash(streamPayload)
-                                 : InternalHashAlgorithm.ComputeHash(bytePayload!.Value.ToArray());
+                                 ? hasher.ComputeHash(streamPayload)
+                                 : hasher.ComputeHash(bytePayload!.Value.ToArray());
             extendedContentType = ExtendContentTypeDirect(contentType, HashAlgorithmName);
         }
         else

CoseIndirectSignature/IndirectSignatureFactory.cs

@@ -36,22 +36,32 @@ public enum IndirectSignatureVersion
         CoseHashEnvelope
     }
 
-    private readonly HashAlgorithm InternalHashAlgorithm;
     private readonly uint HashLength;
     private readonly CoseHashAlgorithm InternalCoseHashAlgorithm;
     private readonly HashAlgorithmName InternalHashAlgorithmName;
     private readonly ICoseSign1MessageFactory InternalMessageFactory;
 
     /// <summary>
-    /// The HashAlgorithm this factory is using.
+    /// Creates a new <see cref="System.Security.Cryptography.HashAlgorithm"/> instance matching the algorithm this factory is configured with.
+    /// Each access returns a new instance. Callers are responsible for disposing the returned instance.
     /// </summary>
-    public HashAlgorithm HashAlgorithm => InternalHashAlgorithm;
+    public HashAlgorithm HashAlgorithm => CreateHashAlgorithm();
 
     /// <summary>
-    /// The HashAlgorightmName this factory is using.
+    /// The HashAlgorithmName this factory is using.
     /// </summary>
     public HashAlgorithmName HashAlgorithmName => InternalHashAlgorithmName;
 
+    /// <summary>
+    /// Creates a new <see cref="System.Security.Cryptography.HashAlgorithm"/> instance that is safe for use on the calling thread.
+    /// </summary>
+    /// <returns>A new <see cref="System.Security.Cryptography.HashAlgorithm"/> instance. Callers are responsible for disposing this instance.</returns>
+    private HashAlgorithm CreateHashAlgorithm()
+    {
+        return CoseSign1MessageIndirectSignatureExtensions.CreateHashAlgorithmFromName(InternalHashAlgorithmName)
+            ?? throw new InvalidOperationException($"Failed to create HashAlgorithm for {InternalHashAlgorithmName}");
+    }
+
     /// <summary>
     /// The CoseSign1 Message Factory this factory is using.
     /// </summary>
@@ -81,21 +91,49 @@ public IndirectSignatureFactory() : this(HashAlgorithmName.SHA256)
     public IndirectSignatureFactory(HashAlgorithmName hashAlgorithmName, ICoseSign1MessageFactory coseSign1MessageFactory)
     {
         InternalHashAlgorithmName = hashAlgorithmName;
-        InternalHashAlgorithm = CoseSign1MessageIndirectSignatureExtensions.CreateHashAlgorithmFromName(hashAlgorithmName) ?? throw new ArgumentOutOfRangeException(nameof(hashAlgorithmName), $"hashAlgorithmName[{hashAlgorithmName}] could not be instantiated into a valid HashAlgorithm");
         InternalMessageFactory = coseSign1MessageFactory;
-        HashLength = (uint)InternalHashAlgorithm.HashSize / 8;
-        InternalCoseHashAlgorithm = GetCoseHashAlgorithmFromHashAlgorithm(InternalHashAlgorithm);
+        InternalCoseHashAlgorithm = GetCoseHashAlgorithmFromName(hashAlgorithmName);
+        HashLength = GetHashLengthFromName(hashAlgorithmName);
     }
 
-    private CoseHashAlgorithm GetCoseHashAlgorithmFromHashAlgorithm(HashAlgorithm algorithm)
+    private static CoseHashAlgorithm GetCoseHashAlgorithmFromName(HashAlgorithmName algorithmName)
     {
-        return algorithm switch
+        if (algorithmName == HashAlgorithmName.SHA256)
         {
-            SHA256 => CoseHashAlgorithm.SHA256,
-            SHA384 => CoseHashAlgorithm.SHA384,
-            SHA512 => CoseHashAlgorithm.SHA512,
-            _ => throw new ArgumentException($@"No mapping for hash algorithm {algorithm.GetType().FullName} to any {nameof(CoseHashAlgorithm)}")
-        };
+            return CoseHashAlgorithm.SHA256;
+        }
+
+        if (algorithmName == HashAlgorithmName.SHA384)
+        {
+            return CoseHashAlgorithm.SHA384;
+        }
+
+        if (algorithmName == HashAlgorithmName.SHA512)
+        {
+            return CoseHashAlgorithm.SHA512;
+        }
+
+        throw new ArgumentOutOfRangeException(nameof(algorithmName), $"No mapping for hash algorithm {algorithmName} to any {nameof(CoseHashAlgorithm)}");
+    }
+
+    private static uint GetHashLengthFromName(HashAlgorithmName algorithmName)
+    {
+        if (algorithmName == HashAlgorithmName.SHA256)
+        {
+            return 32;
+        }
+
+        if (algorithmName == HashAlgorithmName.SHA384)
+        {
+            return 48;
+        }
+
+        if (algorithmName == HashAlgorithmName.SHA512)
+        {
+            return 64;
+        }
+
+        throw new ArgumentOutOfRangeException(nameof(algorithmName), $"Unsupported hash algorithm: {algorithmName}");
     }
 
     /// <summary>
@@ -314,21 +352,13 @@ internal static bool HashMatches(CoseHashAlgorithm hashAlgorithm, ReadOnlyMemory
             { 64, HashAlgorithmName.SHA512 }
         });
 
-    private bool DisposedValue;
     /// <summary>
-    /// Dispose pattern implementation
+    /// Dispose pattern implementation. No unmanaged resources to release;
+    /// HashAlgorithm instances are now created and disposed per-operation.
     /// </summary>
     /// <param name="disposing">True if called from Dispose()</param>
     private void Dispose(bool disposing)
     {
-        if (!DisposedValue)
-        {
-            if (disposing)
-            {
-                HashAlgorithm.Dispose();
-            }
-            DisposedValue = true;
-        }
     }
 
     /// <inheritdoc/>