Prevent concurrent use of cryptographic hash algorithms

src/libraries/System.Security.Cryptography/src/Resources/Strings.resx

@@ -273,6 +273,9 @@
   <data name="Cryptography_CipherModeNotSupported" xml:space="preserve">
     <value>The specified CipherMode '{0}' is not supported.</value>
   </data>
+  <data name="Cryptography_ConcurrentUseNotSupported" xml:space="preserve">
+    <value>Concurrent operations from multiple threads on this type are not supported.</value>
+  </data>
   <data name="Cryptography_CngKeyWrongAlgorithm" xml:space="preserve">
     <value>This key is for algorithm '{0}'. Expected '{1}'.</value>
   </data>

src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj

@@ -371,6 +371,8 @@
     <Compile Include="System\Security\Cryptography\CngProvider.cs" />
     <Compile Include="System\Security\Cryptography\CngUIPolicy.cs" />
     <Compile Include="System\Security\Cryptography\CngUIProtectionLevels.cs" />
+    <Compile Include="System\Security\Cryptography\ConcurrencyBlock.cs" />
+    <Compile Include="System\Security\Cryptography\ConcurrentSafeKmac.cs" />
     <Compile Include="System\Security\Cryptography\CryptoConfigForwarder.cs" />
     <Compile Include="System\Security\Cryptography\CryptographicOperations.cs" />
     <Compile Include="System\Security\Cryptography\CryptographicUnexpectedOperationException.cs" />

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ConcurrencyBlock.cs

@@ -0,0 +1,40 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Threading;
+
+namespace System.Security.Cryptography
+{
+    internal struct ConcurrencyBlock
+    {
+        private int _count;
+
+        internal static Scope Enter(ref ConcurrencyBlock block)
+        {
+            int count = Interlocked.Increment(ref block._count);
+
+            if (count != 1)
+            {
+                Interlocked.Decrement(ref block._count);
+                throw new CryptographicException(SR.Cryptography_ConcurrentUseNotSupported);
+            }
+
+            return new Scope(ref block._count);
+        }
+
+        internal ref struct Scope
+        {
+            private ref int _parentCount;
+
+            internal Scope(ref int parentCount)
+            {
+                _parentCount = ref parentCount;
+            }
+
+            internal void Dispose()
+            {
+                Interlocked.Decrement(ref _parentCount);
+            }
+        }
+    }
+}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ConcurrentSafeKmac.cs

@@ -0,0 +1,52 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace System.Security.Cryptography
+{
+    internal struct ConcurrentSafeKmac
+    {
+        private readonly LiteKmac _liteKmac;
+        private ConcurrencyBlock _block;
+
+        public int HashSizeInBytes => _liteKmac.HashSizeInBytes;
+
+        internal ConcurrentSafeKmac(string algorithmId, ReadOnlySpan<byte> key, ReadOnlySpan<byte> customizationString, bool xof)
+        {
+            _liteKmac = LiteHashProvider.CreateKmac(algorithmId, key, customizationString, xof);
+        }
+
+        public void Append(ReadOnlySpan<byte> data)
+        {
+            using (ConcurrencyBlock.Enter(ref _block))
+            {
+                _liteKmac.Append(data);
+            }
+        }
+
+        public int Current(Span<byte> destination)
+        {
+            using (ConcurrencyBlock.Enter(ref _block))
+            {
+                return _liteKmac.Current(destination);
+            }
+        }
+
+        public int Finalize(Span<byte> destination)
+        {
+            using (ConcurrencyBlock.Enter(ref _block))
+            {
+                return _liteKmac.Finalize(destination);
+            }
+        }
+
+        public void Reset()
+        {
+            using (ConcurrencyBlock.Enter(ref _block))
+            {
+                _liteKmac.Reset();
+            }
+        }
+
+        public void Dispose() => _liteKmac.Dispose();
+    }
+}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HashProviderCng.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Diagnostics;
+using System.Threading;
 using Microsoft.Win32.SafeHandles;
 using BCryptCreateHashFlags = Interop.BCrypt.BCryptCreateHashFlags;
 using BCryptOpenAlgorithmProviderFlags = Interop.BCrypt.BCryptOpenAlgorithmProviderFlags;
@@ -63,47 +64,57 @@ internal HashProviderCng(string hashAlgId, ReadOnlySpan<byte> key, bool isHmac)
         public sealed override unsafe void AppendHashData(ReadOnlySpan<byte> source)
         {
             Debug.Assert(_hHash != null);
-            NTSTATUS ntStatus = Interop.BCrypt.BCryptHashData(_hHash, source, source.Length, 0);
-            if (ntStatus != NTSTATUS.STATUS_SUCCESS)
+
+            using (ConcurrencyBlock.Enter(ref _block))
             {
-                throw Interop.BCrypt.CreateCryptographicException(ntStatus);
-            }
+                NTSTATUS ntStatus = Interop.BCrypt.BCryptHashData(_hHash, source, source.Length, 0);
+                if (ntStatus != NTSTATUS.STATUS_SUCCESS)
+                {
+                    throw Interop.BCrypt.CreateCryptographicException(ntStatus);
+                }
 
-            _running = true;
+                _running = true;
+            }
         }
 
         public override int FinalizeHashAndReset(Span<byte> destination)
         {
             Debug.Assert(destination.Length >= _hashSize);
-
             Debug.Assert(_hHash != null);
-            NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(_hHash, destination, _hashSize, 0);
-            if (ntStatus != NTSTATUS.STATUS_SUCCESS)
+
+            using (ConcurrencyBlock.Enter(ref _block))
             {
-                throw Interop.BCrypt.CreateCryptographicException(ntStatus);
-            }
+                NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(_hHash, destination, _hashSize, 0);
+
+                if (ntStatus != NTSTATUS.STATUS_SUCCESS)
+                {
+                    throw Interop.BCrypt.CreateCryptographicException(ntStatus);
+                }
 
-            _running = false;
-            Reset();
-            return _hashSize;
+                _running = false;
+                Reset();
+                return _hashSize;
+            }
         }
 
         public override int GetCurrentHash(Span<byte> destination)
         {
             Debug.Assert(destination.Length >= _hashSize);
-
             Debug.Assert(_hHash != null);
 
-            using (SafeBCryptHashHandle tmpHash = Interop.BCrypt.BCryptDuplicateHash(_hHash))
+            using (ConcurrencyBlock.Enter(ref _block))
             {
-                NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(tmpHash, destination, _hashSize, 0);
-
-                if (ntStatus != NTSTATUS.STATUS_SUCCESS)
+                using (SafeBCryptHashHandle tmpHash = Interop.BCrypt.BCryptDuplicateHash(_hHash))
                 {
-                    throw Interop.BCrypt.CreateCryptographicException(ntStatus);
-                }
+                    NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(tmpHash, destination, _hashSize, 0);
 
-                return _hashSize;
+                    if (ntStatus != NTSTATUS.STATUS_SUCCESS)
+                    {
+                        throw Interop.BCrypt.CreateCryptographicException(ntStatus);
+                    }
+
+                    return _hashSize;
+                }
             }
         }
 
@@ -125,21 +136,27 @@ public sealed override void Dispose(bool disposing)
 
         public override void Reset()
         {
+            // Reset does not need to use ConcurrencyBlock. It either no-ops, or creates an entirely new handle, exchanges
+            // them, and disposes of the old handle. We don't need to block concurrency on the Dispose because SafeHandle
+            // does that.
             if (_reusable && !_running)
                 return;
 
-            DestroyHash();
-
             BCryptCreateHashFlags flags = _reusable ?
                 BCryptCreateHashFlags.BCRYPT_HASH_REUSABLE_FLAG :
                 BCryptCreateHashFlags.None;
 
             SafeBCryptHashHandle hHash;
             NTSTATUS ntStatus = Interop.BCrypt.BCryptCreateHash(_hAlgorithm, out hHash, IntPtr.Zero, 0, _key, _key == null ? 0 : _key.Length, flags);
+
             if (ntStatus != NTSTATUS.STATUS_SUCCESS)
+            {
+                hHash.Dispose();
                 throw Interop.BCrypt.CreateCryptographicException(ntStatus);
+            }
 
-            _hHash = hHash;
+            SafeBCryptHashHandle? previousHash = Interlocked.Exchange(ref _hHash, hHash);
+            previousHash?.Dispose();
         }
 
         private void DestroyHash()
@@ -161,5 +178,6 @@ private void DestroyHash()
 
         private readonly int _hashSize;
         private bool _running;
+        private ConcurrencyBlock _block;
     }
 }

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HashProviderDispenser.Apple.cs

@@ -141,6 +141,7 @@ private sealed class AppleDigestProvider : HashProvider
         {
             private readonly LiteHash _liteHash;
             private bool _running;
+            private ConcurrencyBlock _block;
 
             public AppleDigestProvider(string hashAlgorithmId)
             {
@@ -149,21 +150,30 @@ public AppleDigestProvider(string hashAlgorithmId)
 
             public override void AppendHashData(ReadOnlySpan<byte> data)
             {
-                _liteHash.Append(data);
-                _running = true;
+                using (ConcurrencyBlock.Enter(ref _block))
+                {
+                    _liteHash.Append(data);
+                    _running = true;
+                }
             }
 
             public override int FinalizeHashAndReset(Span<byte> destination)
             {
-                int written = _liteHash.Finalize(destination);
-                // Apple's DigestFinal self-resets, so don't bother calling reset.
-                _running = false;
-                return written;
+                using (ConcurrencyBlock.Enter(ref _block))
+                {
+                    int written = _liteHash.Finalize(destination);
+                    // Apple's DigestFinal self-resets, so don't bother calling reset.
+                    _running = false;
+                    return written;
+                }
             }
 
             public override int GetCurrentHash(Span<byte> destination)
             {
-                return _liteHash.Current(destination);
+                using (ConcurrencyBlock.Enter(ref _block))
+                {
+                    return _liteHash.Current(destination);
+                }
             }
 
             public override int HashSizeInBytes => _liteHash.HashSizeInBytes;
@@ -178,10 +188,13 @@ public override void Dispose(bool disposing)
 
             public override void Reset()
             {
-                if (_running)
+                using (ConcurrencyBlock.Enter(ref _block))
                 {
-                    _liteHash.Reset();
-                    _running = false;
+                    if (_running)
+                    {
+                        _liteHash.Reset();
+                        _running = false;
+                    }
                 }
             }
         }
@@ -191,6 +204,7 @@ private sealed class AppleHmacProvider : HashProvider
             private readonly LiteHmac _liteHmac;
             private readonly byte[] _key;
             private bool _running;
+            private ConcurrencyBlock _block;
 
             public AppleHmacProvider(string hashAlgorithmId, ReadOnlySpan<byte> key)
             {
@@ -201,36 +215,45 @@ public AppleHmacProvider(string hashAlgorithmId, ReadOnlySpan<byte> key)
 
             public override void AppendHashData(ReadOnlySpan<byte> data)
             {
-                if (!_running)
+                using (ConcurrencyBlock.Enter(ref _block))
                 {
-                    _liteHmac.Reset(_key);
-                }
+                    if (!_running)
+                    {
+                        _liteHmac.Reset(_key);
+                    }
 
-                _liteHmac.Append(data);
-                _running = true;
+                    _liteHmac.Append(data);
+                    _running = true;
+                }
             }
 
             public override int FinalizeHashAndReset(Span<byte> destination)
             {
-                if (!_running)
+                using (ConcurrencyBlock.Enter(ref _block))
                 {
+                    if (!_running)
+                    {
+                        _liteHmac.Reset(_key);
+                    }
+
+                    int written = _liteHmac.Finalize(destination);
                     _liteHmac.Reset(_key);
+                    _running = false;
+                    return written;
                 }
-
-                int written = _liteHmac.Finalize(destination);
-                _liteHmac.Reset(_key);
-                _running = false;
-                return written;
             }
 
             public override int GetCurrentHash(Span<byte> destination)
             {
-                if (!_running)
+                using (ConcurrencyBlock.Enter(ref _block))
                 {
-                    _liteHmac.Reset(_key);
-                }
+                    if (!_running)
+                    {
+                        _liteHmac.Reset(_key);
+                    }
 
-                return _liteHmac.Current(destination);
+                    return _liteHmac.Current(destination);
+                }
             }
 
             public override int HashSizeInBytes => _liteHmac.HashSizeInBytes;