Skip to content

Commit bbf7c87

Browse files
Clean up some usage of SHA1 and SHA256 in the code base (#24696)
* AntiForgery: Use SHA256 one-shot * AuthN: Use SHA256 one-shot APIs * MVC/Razor: Cleanup SHA256 references * WebSockets: Use SHA1 one-shots * dotnet-openapi: Use preferred SHA256 factory * SignalR: Prefer RNG.Fill over RNG.GetBytes
1 parent adea663 commit bbf7c87

File tree

13 files changed

+35
-125
lines changed

13 files changed

+35
-125
lines changed

src/Antiforgery/src/Internal/AntiforgeryOptionsSetup.cs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Linq;
5+
using System.Security.Cryptography;
66
using System.Text;
77
using Microsoft.AspNetCore.DataProtection;
88
using Microsoft.AspNetCore.WebUtilities;
@@ -35,12 +35,8 @@ public void Configure(AntiforgeryOptions options)
3535

3636
private static string ComputeCookieName(string applicationId)
3737
{
38-
using (var sha256 = CryptographyAlgorithms.CreateSHA256())
39-
{
40-
var hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(applicationId));
41-
var subHash = hash.Take(8).ToArray();
42-
return WebEncoders.Base64UrlEncode(subHash);
43-
}
38+
byte[] fullHash = SHA256.HashData(Encoding.UTF8.GetBytes(applicationId));
39+
return WebEncoders.Base64UrlEncode(fullHash, 0, 8);
4440
}
4541
}
4642
}

src/Antiforgery/src/Internal/AntiforgerySerializationContext.cs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ internal class AntiforgerySerializationContext
2727
private MemoryStream? _stream;
2828
private BinaryReader? _reader;
2929
private BinaryWriter? _writer;
30-
private SHA256? _sha256;
3130

3231
public MemoryStream Stream
3332
{
@@ -82,23 +81,6 @@ private set
8281
}
8382
}
8483

85-
public SHA256 Sha256
86-
{
87-
get
88-
{
89-
if (_sha256 == null)
90-
{
91-
_sha256 = CryptographyAlgorithms.CreateSHA256();
92-
}
93-
94-
return _sha256;
95-
}
96-
private set
97-
{
98-
_sha256 = value;
99-
}
100-
}
101-
10284
public char[] GetChars(int count)
10385
{
10486
if (_chars == null || _chars.Length < count)

src/Antiforgery/src/Internal/CryptographyAlgorithms.cs

Lines changed: 0 additions & 25 deletions
This file was deleted.

src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Diagnostics;
77
using System.Security.Claims;
8+
using System.Security.Cryptography;
89
using Microsoft.Extensions.ObjectPool;
910

1011
namespace Microsoft.AspNetCore.Antiforgery
@@ -134,9 +135,13 @@ private byte[] ComputeSha256(IEnumerable<string> parameters)
134135

135136
writer.Flush();
136137

137-
var sha256 = serializationContext.Sha256;
138-
var stream = serializationContext.Stream;
139-
var bytes = sha256.ComputeHash(stream.ToArray(), 0, checked((int)stream.Length));
138+
bool success = serializationContext.Stream.TryGetBuffer(out ArraySegment<byte> buffer);
139+
if (!success)
140+
{
141+
throw new InvalidOperationException();
142+
}
143+
144+
var bytes = SHA256.HashData(buffer);
140145

141146
return bytes;
142147
}

src/Middleware/WebSockets/src/HandshakeHelpers.cs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -110,23 +110,20 @@ public static string CreateResponseKey(string requestKey)
110110
// this concatenated value to obtain a 20-byte value and base64-encoding"
111111
// https://tools.ietf.org/html/rfc6455#section-4.2.2
112112

113-
using (var algorithm = SHA1.Create())
114-
{
115-
// requestKey is already verified to be small (24 bytes) by 'IsRequestKeyValid()' and everything is 1:1 mapping to UTF8 bytes
116-
// so this can be hardcoded to 60 bytes for the requestKey + static websocket string
117-
Span<byte> mergedBytes = stackalloc byte[60];
118-
Encoding.UTF8.GetBytes(requestKey, mergedBytes);
119-
EncodedWebSocketKey.CopyTo(mergedBytes.Slice(24));
120-
121-
Span<byte> hashedBytes = stackalloc byte[20];
122-
var success = algorithm.TryComputeHash(mergedBytes, hashedBytes, out var written);
123-
if (!success || written != 20)
124-
{
125-
throw new InvalidOperationException("Could not compute the hash for the 'Sec-WebSocket-Accept' header.");
126-
}
113+
// requestKey is already verified to be small (24 bytes) by 'IsRequestKeyValid()' and everything is 1:1 mapping to UTF8 bytes
114+
// so this can be hardcoded to 60 bytes for the requestKey + static websocket string
115+
Span<byte> mergedBytes = stackalloc byte[60];
116+
Encoding.UTF8.GetBytes(requestKey, mergedBytes);
117+
EncodedWebSocketKey.CopyTo(mergedBytes.Slice(24));
127118

128-
return Convert.ToBase64String(hashedBytes);
119+
Span<byte> hashedBytes = stackalloc byte[20];
120+
var written = SHA1.HashData(mergedBytes, hashedBytes);
121+
if (written != 20)
122+
{
123+
throw new InvalidOperationException("Could not compute the hash for the 'Sec-WebSocket-Accept' header.");
129124
}
125+
126+
return Convert.ToBase64String(hashedBytes);
130127
}
131128
}
132129
}

src/Mvc/Mvc.Razor/src/Infrastructure/CryptographyAlgorithms.cs

Lines changed: 0 additions & 25 deletions
This file was deleted.

src/Mvc/Mvc.Razor/src/Infrastructure/DefaultFileVersionProvider.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Security.Cryptography;
56
using Microsoft.AspNetCore.Hosting;
67
using Microsoft.AspNetCore.Http;
78
using Microsoft.AspNetCore.Mvc.ViewFeatures;
@@ -97,7 +98,7 @@ public string AddFileVersionToPath(PathString requestPathBase, string path)
9798

9899
private static string GetHashForFile(IFileInfo fileInfo)
99100
{
100-
using (var sha256 = CryptographyAlgorithms.CreateSHA256())
101+
using (var sha256 = SHA256.Create())
101102
{
102103
using (var readStream = fileInfo.CreateReadStream())
103104
{
@@ -107,4 +108,4 @@ private static string GetHashForFile(IFileInfo fileInfo)
107108
}
108109
}
109110
}
110-
}
111+
}

src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,11 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Globalization;
7+
using System.Security.Cryptography;
78
using System.Text;
89
using Microsoft.AspNetCore.Http;
9-
using Microsoft.AspNetCore.Mvc.Razor.Infrastructure;
1010
using Microsoft.AspNetCore.Razor.TagHelpers;
1111
using Microsoft.AspNetCore.Routing;
12-
using Microsoft.Extensions.Internal;
1312
using Microsoft.Extensions.Primitives;
1413

1514
namespace Microsoft.AspNetCore.Mvc.TagHelpers.Cache
@@ -180,12 +179,9 @@ public string GenerateHashedKey()
180179
// The key is typically too long to be useful, so we use a cryptographic hash
181180
// as the actual key (better randomization and key distribution, so small vary
182181
// values will generate dramatically different keys).
183-
using (var sha256 = CryptographyAlgorithms.CreateSHA256())
184-
{
185-
var contentBytes = Encoding.UTF8.GetBytes(key);
186-
var hashedBytes = sha256.ComputeHash(contentBytes);
187-
return Convert.ToBase64String(hashedBytes);
188-
}
182+
var contentBytes = Encoding.UTF8.GetBytes(key);
183+
var hashedBytes = SHA256.HashData(contentBytes);
184+
return Convert.ToBase64String(hashedBytes);
189185
}
190186

191187
/// <inheritdoc />

src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountHandler.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
6868
// Store this for use during the code redemption.
6969
properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
7070

71-
using var sha256 = SHA256.Create();
72-
var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
71+
var challengeBytes = SHA256.HashData(Encoding.UTF8.GetBytes(codeVerifier));
7372
var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes);
7473

7574
queryStrings[OAuthConstants.CodeChallengeKey] = codeChallenge;

src/Security/Authentication/OAuth/src/OAuthHandler.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,7 @@ protected virtual string BuildChallengeUrl(AuthenticationProperties properties,
280280
// Store this for use during the code redemption.
281281
properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
282282

283-
using var sha256 = SHA256.Create();
284-
var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
283+
var challengeBytes = SHA256.HashData(Encoding.UTF8.GetBytes(codeVerifier));
285284
var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes);
286285

287286
parameters[OAuthConstants.CodeChallengeKey] = codeChallenge;

src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,8 +375,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert
375375
// Store this for use during the code redemption. See RunAuthorizationCodeReceivedEventAsync.
376376
properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
377377

378-
using var sha256 = SHA256.Create();
379-
var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
378+
var challengeBytes = SHA256.HashData(Encoding.UTF8.GetBytes(codeVerifier));
380379
var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes);
381380

382381
message.Parameters.Add(OAuthConstants.CodeChallengeKey, codeChallenge);

src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ internal partial class HttpConnectionManager
2323
// TODO: Consider making this configurable? At least for testing?
2424
private static readonly TimeSpan _heartbeatTickRate = TimeSpan.FromSeconds(1);
2525

26-
private static readonly RNGCryptoServiceProvider _keyGenerator = new RNGCryptoServiceProvider();
27-
2826
private readonly ConcurrentDictionary<string, (HttpConnectionContext Connection, ValueStopwatch Timer)> _connections =
2927
new ConcurrentDictionary<string, (HttpConnectionContext Connection, ValueStopwatch Timer)>(StringComparer.Ordinal);
3028
private readonly TimerAwaitable _nextHeartbeat;
@@ -113,7 +111,7 @@ private static string MakeNewConnectionId()
113111
// 128 bit buffer / 8 bits per byte = 16 bytes
114112
Span<byte> buffer = stackalloc byte[16];
115113
// Generate the id with RNGCrypto because we want a cryptographically random id, which GUID is not
116-
_keyGenerator.GetBytes(buffer);
114+
RandomNumberGenerator.Fill(buffer);
117115
return WebEncoders.Base64UrlEncode(buffer);
118116
}
119117

src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -513,19 +513,7 @@ private static IDictionary<string, string> GetServicePackages(CodeGenerator? typ
513513

514514
private static byte[] GetHash(Stream stream)
515515
{
516-
SHA256 algorithm;
517-
try
518-
{
519-
algorithm = SHA256.Create();
520-
}
521-
catch (TargetInvocationException)
522-
{
523-
// SHA256.Create is documented to throw this exception on FIPS-compliant machines. See
524-
// https://msdn.microsoft.com/en-us/library/z08hz7ad Fall back to a FIPS-compliant SHA256 algorithm.
525-
algorithm = new SHA256CryptoServiceProvider();
526-
}
527-
528-
using (algorithm)
516+
using (var algorithm = SHA256.Create())
529517
{
530518
return algorithm.ComputeHash(stream);
531519
}

0 commit comments

Comments
 (0)