Skip to content

Commit 9117d5c

Browse files
vcsjonesvcsjones
committed
Add concurrent guards to Shake128/256
1 parent 595ad02 commit 9117d5c

File tree

9 files changed

+182
-28
lines changed

9 files changed

+182
-28
lines changed

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

+1
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@
490490
<Compile Include="System\Security\Cryptography\SHA3_512.cs" />
491491
<Compile Include="System\Security\Cryptography\Shake128.cs" />
492492
<Compile Include="System\Security\Cryptography\Shake256.cs" />
493+
<Compile Include="System\Security\Cryptography\ShakeCommon.cs" />
493494
<Compile Include="System\Security\Cryptography\SignatureDescription.cs" />
494495
<Compile Include="System\Security\Cryptography\SymmetricAlgorithm.cs" />
495496
<Compile Include="System\Security\Cryptography\SymmetricPadding.cs" />

‎src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Shake128.NonWindows.cs

+23
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,33 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Threading;
5+
46
namespace System.Security.Cryptography
57
{
68
public sealed partial class Shake128 : IDisposable
79
{
810
private const string HashAlgorithmId = HashAlgorithmNames.SHAKE128;
11+
private int _callOperations;
12+
13+
private TOut ConcurrentGuard<TIn, TOut>(TIn args, ConcurrentGuardDelegate<TIn, TOut> callback)
14+
{
15+
try
16+
{
17+
if (Interlocked.Increment(ref _callOperations) != 1)
18+
{
19+
throw GetConcurrentUseException();
20+
}
21+
22+
return callback(args, ref _hashProvider);
23+
}
24+
finally
25+
{
26+
Interlocked.Decrement(ref _callOperations);
27+
}
28+
}
29+
30+
private static CryptographicException GetConcurrentUseException() =>
31+
new CryptographicException(SR.Cryptography_ConcurrentUseNotSupported);
932
}
1033
}

‎src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Shake128.Windows.cs

+4
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,9 @@ public sealed partial class Shake128 : IDisposable
1111
// BCRYPT_CUSTOMIZATION_STRING.
1212
// So for SHAKE, we create a cSHAKE instance and don't specify S or N.
1313
private const string HashAlgorithmId = HashAlgorithmNames.CSHAKE128;
14+
15+
// Windows does not need a concurrent guard, so go right to the delegate.
16+
private TOut ConcurrentGuard<TIn, TOut>(TIn args, ConcurrentGuardDelegate<TIn, TOut> callback) =>
17+
callback(args, ref _hashProvider);
1418
}
1519
}

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

+38-14
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
using System.Threading;
88
using System.Threading.Tasks;
99

10+
#pragma warning disable CS8500 // takes address of managed type
11+
1012
namespace System.Security.Cryptography
1113
{
1214
/// <summary>
@@ -64,11 +66,16 @@ public void AppendData(byte[] data)
6466
/// </summary>
6567
/// <param name="data">The data to process.</param>
6668
/// <exception cref="ObjectDisposedException">The object has already been disposed.</exception>
67-
public void AppendData(ReadOnlySpan<byte> data)
69+
public unsafe void AppendData(ReadOnlySpan<byte> data)
6870
{
6971
CheckDisposed();
7072

71-
_hashProvider.Append(data);
73+
ConcurrentGuard<IntPtr, object?>((IntPtr)(&data), static (IntPtr dataPointer, ref LiteXof hashProvider) =>
74+
{
75+
ReadOnlySpan<byte> data = *(ReadOnlySpan<byte>*)dataPointer;
76+
hashProvider.Append(data);
77+
return null;
78+
});
7279
}
7380

7481
/// <summary>
@@ -87,10 +94,13 @@ public byte[] GetHashAndReset(int outputLength)
8794
ArgumentOutOfRangeException.ThrowIfNegative(outputLength);
8895
CheckDisposed();
8996

90-
byte[] output = new byte[outputLength];
91-
_hashProvider.Finalize(output);
92-
_hashProvider.Reset();
93-
return output;
97+
return ConcurrentGuard<int, byte[]>(outputLength, static (int outputLength, ref LiteXof hashProvider) =>
98+
{
99+
byte[] output = new byte[outputLength];
100+
hashProvider.Finalize(output);
101+
hashProvider.Reset();
102+
return output;
103+
});
94104
}
95105

96106
/// <summary>
@@ -100,12 +110,17 @@ public byte[] GetHashAndReset(int outputLength)
100110
/// <param name="destination">The buffer to fill with the hash.</param>
101111
/// <exception cref="ObjectDisposedException">The object has already been disposed.</exception>
102112
/// <seealso cref="GetCurrentHash(Span{byte})" />
103-
public void GetHashAndReset(Span<byte> destination)
113+
public unsafe void GetHashAndReset(Span<byte> destination)
104114
{
105115
CheckDisposed();
106116

107-
_hashProvider.Finalize(destination);
108-
_hashProvider.Reset();
117+
ConcurrentGuard<IntPtr, object?>((IntPtr)(&destination), static (IntPtr destinationPointer, ref LiteXof hashProvider) =>
118+
{
119+
Span<byte> destination = *(Span<byte>*)destinationPointer;
120+
hashProvider.Finalize(destination);
121+
hashProvider.Reset();
122+
return null;
123+
});
109124
}
110125

111126
/// <summary>
@@ -124,9 +139,12 @@ public byte[] GetCurrentHash(int outputLength)
124139
ArgumentOutOfRangeException.ThrowIfNegative(outputLength);
125140
CheckDisposed();
126141

127-
byte[] output = new byte[outputLength];
128-
_hashProvider.Current(output);
129-
return output;
142+
return ConcurrentGuard<int, byte[]>(outputLength, static (int outputLength, ref LiteXof hashProvider) =>
143+
{
144+
byte[] output = new byte[outputLength];
145+
hashProvider.Current(output);
146+
return output;
147+
});
130148
}
131149

132150
/// <summary>
@@ -136,10 +154,16 @@ public byte[] GetCurrentHash(int outputLength)
136154
/// <param name="destination">The buffer to fill with the hash.</param>
137155
/// <exception cref="ObjectDisposedException">The object has already been disposed.</exception>
138156
/// <seealso cref="GetHashAndReset(Span{byte})" />
139-
public void GetCurrentHash(Span<byte> destination)
157+
public unsafe void GetCurrentHash(Span<byte> destination)
140158
{
141159
CheckDisposed();
142-
_hashProvider.Current(destination);
160+
161+
ConcurrentGuard<IntPtr, object?>((IntPtr)(&destination), static (IntPtr destinationPointer, ref LiteXof hashProvider) =>
162+
{
163+
Span<byte> destination = *(Span<byte>*)destinationPointer;
164+
hashProvider.Current(destination);
165+
return null;
166+
});
143167
}
144168

145169
/// <summary>

‎src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Shake256.NonWindows.cs

+23
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,33 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Threading;
5+
46
namespace System.Security.Cryptography
57
{
68
public sealed partial class Shake256 : IDisposable
79
{
810
private const string HashAlgorithmId = HashAlgorithmNames.SHAKE256;
11+
private int _callOperations;
12+
13+
private TOut ConcurrentGuard<TIn, TOut>(TIn args, ConcurrentGuardDelegate<TIn, TOut> callback)
14+
{
15+
try
16+
{
17+
if (Interlocked.Increment(ref _callOperations) != 1)
18+
{
19+
throw GetConcurrentUseException();
20+
}
21+
22+
return callback(args, ref _hashProvider);
23+
}
24+
finally
25+
{
26+
Interlocked.Decrement(ref _callOperations);
27+
}
28+
}
29+
30+
private static CryptographicException GetConcurrentUseException() =>
31+
new CryptographicException(SR.Cryptography_ConcurrentUseNotSupported);
932
}
1033
}

‎src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Shake256.Windows.cs

+4
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,9 @@ public sealed partial class Shake256 : IDisposable
1111
// BCRYPT_CUSTOMIZATION_STRING.
1212
// So for SHAKE, we create a cSHAKE instance and don't specify S or N.
1313
private const string HashAlgorithmId = HashAlgorithmNames.CSHAKE256;
14+
15+
// Windows does not need a concurrent guard, so go right to the delegate.
16+
private TOut ConcurrentGuard<TIn, TOut>(TIn args, ConcurrentGuardDelegate<TIn, TOut> callback) =>
17+
callback(args, ref _hashProvider);
1418
}
1519
}

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

+38-14
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
using System.Threading;
88
using System.Threading.Tasks;
99

10+
#pragma warning disable CS8500 // takes address of managed type
11+
1012
namespace System.Security.Cryptography
1113
{
1214
/// <summary>
@@ -64,11 +66,16 @@ public void AppendData(byte[] data)
6466
/// </summary>
6567
/// <param name="data">The data to process.</param>
6668
/// <exception cref="ObjectDisposedException">The object has already been disposed.</exception>
67-
public void AppendData(ReadOnlySpan<byte> data)
69+
public unsafe void AppendData(ReadOnlySpan<byte> data)
6870
{
6971
CheckDisposed();
7072

71-
_hashProvider.Append(data);
73+
ConcurrentGuard<IntPtr, object?>((IntPtr)(&data), static (IntPtr dataPointer, ref LiteXof hashProvider) =>
74+
{
75+
ReadOnlySpan<byte> data = *(ReadOnlySpan<byte>*)dataPointer;
76+
hashProvider.Append(data);
77+
return null;
78+
});
7279
}
7380

7481
/// <summary>
@@ -87,10 +94,13 @@ public byte[] GetHashAndReset(int outputLength)
8794
ArgumentOutOfRangeException.ThrowIfNegative(outputLength);
8895
CheckDisposed();
8996

90-
byte[] output = new byte[outputLength];
91-
_hashProvider.Finalize(output);
92-
_hashProvider.Reset();
93-
return output;
97+
return ConcurrentGuard<int, byte[]>(outputLength, static (int outputLength, ref LiteXof hashProvider) =>
98+
{
99+
byte[] output = new byte[outputLength];
100+
hashProvider.Finalize(output);
101+
hashProvider.Reset();
102+
return output;
103+
});
94104
}
95105

96106
/// <summary>
@@ -100,12 +110,17 @@ public byte[] GetHashAndReset(int outputLength)
100110
/// <param name="destination">The buffer to fill with the hash.</param>
101111
/// <exception cref="ObjectDisposedException">The object has already been disposed.</exception>
102112
/// <seealso cref="GetCurrentHash(Span{byte})" />
103-
public void GetHashAndReset(Span<byte> destination)
113+
public unsafe void GetHashAndReset(Span<byte> destination)
104114
{
105115
CheckDisposed();
106116

107-
_hashProvider.Finalize(destination);
108-
_hashProvider.Reset();
117+
ConcurrentGuard<IntPtr, object?>((IntPtr)(&destination), static (IntPtr destinationPointer, ref LiteXof hashProvider) =>
118+
{
119+
Span<byte> destination = *(Span<byte>*)destinationPointer;
120+
hashProvider.Finalize(destination);
121+
hashProvider.Reset();
122+
return null;
123+
});
109124
}
110125

111126
/// <summary>
@@ -124,9 +139,12 @@ public byte[] GetCurrentHash(int outputLength)
124139
ArgumentOutOfRangeException.ThrowIfNegative(outputLength);
125140
CheckDisposed();
126141

127-
byte[] output = new byte[outputLength];
128-
_hashProvider.Current(output);
129-
return output;
142+
return ConcurrentGuard<int, byte[]>(outputLength, static (int outputLength, ref LiteXof hashProvider) =>
143+
{
144+
byte[] output = new byte[outputLength];
145+
hashProvider.Current(output);
146+
return output;
147+
});
130148
}
131149

132150
/// <summary>
@@ -136,10 +154,16 @@ public byte[] GetCurrentHash(int outputLength)
136154
/// <param name="destination">The buffer to fill with the hash.</param>
137155
/// <exception cref="ObjectDisposedException">The object has already been disposed.</exception>
138156
/// <seealso cref="GetHashAndReset(Span{byte})" />
139-
public void GetCurrentHash(Span<byte> destination)
157+
public unsafe void GetCurrentHash(Span<byte> destination)
140158
{
141159
CheckDisposed();
142-
_hashProvider.Current(destination);
160+
161+
ConcurrentGuard<IntPtr, object?>((IntPtr)(&destination), static (IntPtr destinationPointer, ref LiteXof hashProvider) =>
162+
{
163+
Span<byte> destination = *(Span<byte>*)destinationPointer;
164+
hashProvider.Current(destination);
165+
return null;
166+
});
143167
}
144168

145169
/// <summary>

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

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
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 delegate TOut ConcurrentGuardDelegate<in TIn, out TOut>(TIn args, ref LiteXof hashProvider);
7+
}

‎src/libraries/System.Security.Cryptography/tests/ShakeTestDriver.cs

+44
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using System.Linq;
77
using System.Threading;
88
using System.Threading.Tasks;
9+
using Microsoft.DotNet.RemoteExecutor;
10+
using Microsoft.DotNet.XUnitExtensions;
911
using Xunit;
1012

1113
namespace System.Security.Cryptography.Tests
@@ -536,5 +538,47 @@ public void IsSupported_AgreesWithPlatform()
536538
{
537539
Assert.Equal(TShakeTrait.IsSupported, PlatformDetection.SupportsSha3);
538540
}
541+
542+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
543+
public void GetHashAndReset_ConcurrentUseDoesNotCrashProcess()
544+
{
545+
if (!IsSupported)
546+
{
547+
throw new SkipTestException("Algorithm is not supported on this platform.");
548+
}
549+
550+
RemoteExecutor.Invoke(static () =>
551+
{
552+
using (TShake shake = TShakeTrait.Create())
553+
{
554+
Thread thread1 = new(ThreadWork);
555+
Thread thread2 = new(ThreadWork);
556+
thread1.Start(shake);
557+
thread2.Start(shake);
558+
thread1.Join();
559+
thread2.Join();
560+
}
561+
}).Dispose();
562+
563+
static void ThreadWork(object obj)
564+
{
565+
TShake shake = (TShake)obj;
566+
567+
try
568+
{
569+
byte[] input = new byte[128];
570+
571+
for (int i = 0; i < 10_000; i++)
572+
{
573+
TShakeTrait.AppendData(shake, input);
574+
TShakeTrait.GetHashAndReset(shake, 128);
575+
}
576+
}
577+
catch
578+
{
579+
// Ignore all managed exceptions. HashAlgorithm is not thread safe, but we don't want process crashes.
580+
}
581+
}
582+
}
539583
}
540584
}

0 commit comments

Comments
 (0)