Skip to content

Commit db8a764

Browse files
vcsjonesvcsjones
authored
Prevent concurrent use of cryptographic hash algorithms
1 parent 60ef7ec commit db8a764

28 files changed

+714
-124
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,9 @@
273273
<data name="Cryptography_CipherModeNotSupported" xml:space="preserve">
274274
<value>The specified CipherMode '{0}' is not supported.</value>
275275
</data>
276+
<data name="Cryptography_ConcurrentUseNotSupported" xml:space="preserve">
277+
<value>Concurrent operations from multiple threads on this type are not supported.</value>
278+
</data>
276279
<data name="Cryptography_CngKeyWrongAlgorithm" xml:space="preserve">
277280
<value>This key is for algorithm '{0}'. Expected '{1}'.</value>
278281
</data>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,8 @@
371371
<Compile Include="System\Security\Cryptography\CngProvider.cs" />
372372
<Compile Include="System\Security\Cryptography\CngUIPolicy.cs" />
373373
<Compile Include="System\Security\Cryptography\CngUIProtectionLevels.cs" />
374+
<Compile Include="System\Security\Cryptography\ConcurrencyBlock.cs" />
375+
<Compile Include="System\Security\Cryptography\ConcurrentSafeKmac.cs" />
374376
<Compile Include="System\Security\Cryptography\CryptoConfigForwarder.cs" />
375377
<Compile Include="System\Security\Cryptography\CryptographicOperations.cs" />
376378
<Compile Include="System\Security\Cryptography\CryptographicUnexpectedOperationException.cs" />

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Threading;
5+
6+
namespace System.Security.Cryptography
7+
{
8+
internal struct ConcurrencyBlock
9+
{
10+
private int _count;
11+
12+
internal static Scope Enter(ref ConcurrencyBlock block)
13+
{
14+
int count = Interlocked.Increment(ref block._count);
15+
16+
if (count != 1)
17+
{
18+
Interlocked.Decrement(ref block._count);
19+
throw new CryptographicException(SR.Cryptography_ConcurrentUseNotSupported);
20+
}
21+
22+
return new Scope(ref block._count);
23+
}
24+
25+
internal ref struct Scope
26+
{
27+
private ref int _parentCount;
28+
29+
internal Scope(ref int parentCount)
30+
{
31+
_parentCount = ref parentCount;
32+
}
33+
34+
internal void Dispose()
35+
{
36+
Interlocked.Decrement(ref _parentCount);
37+
}
38+
}
39+
}
40+
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace System.Security.Cryptography
5+
{
6+
internal struct ConcurrentSafeKmac
7+
{
8+
private readonly LiteKmac _liteKmac;
9+
private ConcurrencyBlock _block;
10+
11+
public int HashSizeInBytes => _liteKmac.HashSizeInBytes;
12+
13+
internal ConcurrentSafeKmac(string algorithmId, ReadOnlySpan<byte> key, ReadOnlySpan<byte> customizationString, bool xof)
14+
{
15+
_liteKmac = LiteHashProvider.CreateKmac(algorithmId, key, customizationString, xof);
16+
}
17+
18+
public void Append(ReadOnlySpan<byte> data)
19+
{
20+
using (ConcurrencyBlock.Enter(ref _block))
21+
{
22+
_liteKmac.Append(data);
23+
}
24+
}
25+
26+
public int Current(Span<byte> destination)
27+
{
28+
using (ConcurrencyBlock.Enter(ref _block))
29+
{
30+
return _liteKmac.Current(destination);
31+
}
32+
}
33+
34+
public int Finalize(Span<byte> destination)
35+
{
36+
using (ConcurrencyBlock.Enter(ref _block))
37+
{
38+
return _liteKmac.Finalize(destination);
39+
}
40+
}
41+
42+
public void Reset()
43+
{
44+
using (ConcurrencyBlock.Enter(ref _block))
45+
{
46+
_liteKmac.Reset();
47+
}
48+
}
49+
50+
public void Dispose() => _liteKmac.Dispose();
51+
}
52+
}

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

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Diagnostics;
6+
using System.Threading;
67
using Microsoft.Win32.SafeHandles;
78
using BCryptCreateHashFlags = Interop.BCrypt.BCryptCreateHashFlags;
89
using BCryptOpenAlgorithmProviderFlags = Interop.BCrypt.BCryptOpenAlgorithmProviderFlags;
@@ -63,47 +64,57 @@ internal HashProviderCng(string hashAlgId, ReadOnlySpan<byte> key, bool isHmac)
6364
public sealed override unsafe void AppendHashData(ReadOnlySpan<byte> source)
6465
{
6566
Debug.Assert(_hHash != null);
66-
NTSTATUS ntStatus = Interop.BCrypt.BCryptHashData(_hHash, source, source.Length, 0);
67-
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
67+
68+
using (ConcurrencyBlock.Enter(ref _block))
6869
{
69-
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
70-
}
70+
NTSTATUS ntStatus = Interop.BCrypt.BCryptHashData(_hHash, source, source.Length, 0);
71+
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
72+
{
73+
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
74+
}
7175

72-
_running = true;
76+
_running = true;
77+
}
7378
}
7479

7580
public override int FinalizeHashAndReset(Span<byte> destination)
7681
{
7782
Debug.Assert(destination.Length >= _hashSize);
78-
7983
Debug.Assert(_hHash != null);
80-
NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(_hHash, destination, _hashSize, 0);
81-
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
84+
85+
using (ConcurrencyBlock.Enter(ref _block))
8286
{
83-
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
84-
}
87+
NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(_hHash, destination, _hashSize, 0);
88+
89+
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
90+
{
91+
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
92+
}
8593

86-
_running = false;
87-
Reset();
88-
return _hashSize;
94+
_running = false;
95+
Reset();
96+
return _hashSize;
97+
}
8998
}
9099

91100
public override int GetCurrentHash(Span<byte> destination)
92101
{
93102
Debug.Assert(destination.Length >= _hashSize);
94-
95103
Debug.Assert(_hHash != null);
96104

97-
using (SafeBCryptHashHandle tmpHash = Interop.BCrypt.BCryptDuplicateHash(_hHash))
105+
using (ConcurrencyBlock.Enter(ref _block))
98106
{
99-
NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(tmpHash, destination, _hashSize, 0);
100-
101-
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
107+
using (SafeBCryptHashHandle tmpHash = Interop.BCrypt.BCryptDuplicateHash(_hHash))
102108
{
103-
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
104-
}
109+
NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(tmpHash, destination, _hashSize, 0);
105110

106-
return _hashSize;
111+
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
112+
{
113+
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
114+
}
115+
116+
return _hashSize;
117+
}
107118
}
108119
}
109120

@@ -125,21 +136,27 @@ public sealed override void Dispose(bool disposing)
125136

126137
public override void Reset()
127138
{
139+
// Reset does not need to use ConcurrencyBlock. It either no-ops, or creates an entirely new handle, exchanges
140+
// them, and disposes of the old handle. We don't need to block concurrency on the Dispose because SafeHandle
141+
// does that.
128142
if (_reusable && !_running)
129143
return;
130144

131-
DestroyHash();
132-
133145
BCryptCreateHashFlags flags = _reusable ?
134146
BCryptCreateHashFlags.BCRYPT_HASH_REUSABLE_FLAG :
135147
BCryptCreateHashFlags.None;
136148

137149
SafeBCryptHashHandle hHash;
138150
NTSTATUS ntStatus = Interop.BCrypt.BCryptCreateHash(_hAlgorithm, out hHash, IntPtr.Zero, 0, _key, _key == null ? 0 : _key.Length, flags);
151+
139152
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
153+
{
154+
hHash.Dispose();
140155
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
156+
}
141157

142-
_hHash = hHash;
158+
SafeBCryptHashHandle? previousHash = Interlocked.Exchange(ref _hHash, hHash);
159+
previousHash?.Dispose();
143160
}
144161

145162
private void DestroyHash()
@@ -161,5 +178,6 @@ private void DestroyHash()
161178

162179
private readonly int _hashSize;
163180
private bool _running;
181+
private ConcurrencyBlock _block;
164182
}
165183
}

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

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ private sealed class AppleDigestProvider : HashProvider
141141
{
142142
private readonly LiteHash _liteHash;
143143
private bool _running;
144+
private ConcurrencyBlock _block;
144145

145146
public AppleDigestProvider(string hashAlgorithmId)
146147
{
@@ -149,21 +150,30 @@ public AppleDigestProvider(string hashAlgorithmId)
149150

150151
public override void AppendHashData(ReadOnlySpan<byte> data)
151152
{
152-
_liteHash.Append(data);
153-
_running = true;
153+
using (ConcurrencyBlock.Enter(ref _block))
154+
{
155+
_liteHash.Append(data);
156+
_running = true;
157+
}
154158
}
155159

156160
public override int FinalizeHashAndReset(Span<byte> destination)
157161
{
158-
int written = _liteHash.Finalize(destination);
159-
// Apple's DigestFinal self-resets, so don't bother calling reset.
160-
_running = false;
161-
return written;
162+
using (ConcurrencyBlock.Enter(ref _block))
163+
{
164+
int written = _liteHash.Finalize(destination);
165+
// Apple's DigestFinal self-resets, so don't bother calling reset.
166+
_running = false;
167+
return written;
168+
}
162169
}
163170

164171
public override int GetCurrentHash(Span<byte> destination)
165172
{
166-
return _liteHash.Current(destination);
173+
using (ConcurrencyBlock.Enter(ref _block))
174+
{
175+
return _liteHash.Current(destination);
176+
}
167177
}
168178

169179
public override int HashSizeInBytes => _liteHash.HashSizeInBytes;
@@ -178,10 +188,13 @@ public override void Dispose(bool disposing)
178188

179189
public override void Reset()
180190
{
181-
if (_running)
191+
using (ConcurrencyBlock.Enter(ref _block))
182192
{
183-
_liteHash.Reset();
184-
_running = false;
193+
if (_running)
194+
{
195+
_liteHash.Reset();
196+
_running = false;
197+
}
185198
}
186199
}
187200
}
@@ -191,6 +204,7 @@ private sealed class AppleHmacProvider : HashProvider
191204
private readonly LiteHmac _liteHmac;
192205
private readonly byte[] _key;
193206
private bool _running;
207+
private ConcurrencyBlock _block;
194208

195209
public AppleHmacProvider(string hashAlgorithmId, ReadOnlySpan<byte> key)
196210
{
@@ -201,36 +215,45 @@ public AppleHmacProvider(string hashAlgorithmId, ReadOnlySpan<byte> key)
201215

202216
public override void AppendHashData(ReadOnlySpan<byte> data)
203217
{
204-
if (!_running)
218+
using (ConcurrencyBlock.Enter(ref _block))
205219
{
206-
_liteHmac.Reset(_key);
207-
}
220+
if (!_running)
221+
{
222+
_liteHmac.Reset(_key);
223+
}
208224

209-
_liteHmac.Append(data);
210-
_running = true;
225+
_liteHmac.Append(data);
226+
_running = true;
227+
}
211228
}
212229

213230
public override int FinalizeHashAndReset(Span<byte> destination)
214231
{
215-
if (!_running)
232+
using (ConcurrencyBlock.Enter(ref _block))
216233
{
234+
if (!_running)
235+
{
236+
_liteHmac.Reset(_key);
237+
}
238+
239+
int written = _liteHmac.Finalize(destination);
217240
_liteHmac.Reset(_key);
241+
_running = false;
242+
return written;
218243
}
219-
220-
int written = _liteHmac.Finalize(destination);
221-
_liteHmac.Reset(_key);
222-
_running = false;
223-
return written;
224244
}
225245

226246
public override int GetCurrentHash(Span<byte> destination)
227247
{
228-
if (!_running)
248+
using (ConcurrencyBlock.Enter(ref _block))
229249
{
230-
_liteHmac.Reset(_key);
231-
}
250+
if (!_running)
251+
{
252+
_liteHmac.Reset(_key);
253+
}
232254

233-
return _liteHmac.Current(destination);
255+
return _liteHmac.Current(destination);
256+
}
234257
}
235258

236259
public override int HashSizeInBytes => _liteHmac.HashSizeInBytes;

0 commit comments

Comments
 (0)