Clean up some usage of SHA1 and SHA256 in the code base (#24696)

GrabYourPitchforks · web-flow · commit bbf7c8780c42 · 2020-08-10T14:26:41.000-07:00
* 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
diff --git a/src/Antiforgery/src/Internal/AntiforgeryOptionsSetup.cs b/src/Antiforgery/src/Internal/AntiforgeryOptionsSetup.cs
@@ -2,7 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Linq;
+using System.Security.Cryptography;
 using System.Text;
 using Microsoft.AspNetCore.DataProtection;
 using Microsoft.AspNetCore.WebUtilities;
@@ -35,12 +35,8 @@ public void Configure(AntiforgeryOptions options)
 
         private static string ComputeCookieName(string applicationId)
         {
-            using (var sha256 = CryptographyAlgorithms.CreateSHA256())
-            {
-                var hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(applicationId));
-                var subHash = hash.Take(8).ToArray();
-                return WebEncoders.Base64UrlEncode(subHash);
-            }
+            byte[] fullHash = SHA256.HashData(Encoding.UTF8.GetBytes(applicationId));
+            return WebEncoders.Base64UrlEncode(fullHash, 0, 8);
         }
     }
 }
diff --git a/src/Antiforgery/src/Internal/AntiforgerySerializationContext.cs b/src/Antiforgery/src/Internal/AntiforgerySerializationContext.cs
@@ -27,7 +27,6 @@ internal class AntiforgerySerializationContext
         private MemoryStream? _stream;
         private BinaryReader? _reader;
         private BinaryWriter? _writer;
-        private SHA256? _sha256;
 
         public MemoryStream Stream
         {
@@ -82,23 +81,6 @@ private set
             }
         }
 
-        public SHA256 Sha256
-        {
-            get
-            {
-                if (_sha256 == null)
-                {
-                    _sha256 = CryptographyAlgorithms.CreateSHA256();
-                }
-
-                return _sha256;
-            }
-            private set
-            {
-                _sha256 = value;
-            }
-        }
-
         public char[] GetChars(int count)
         {
             if (_chars == null || _chars.Length < count)
diff --git a/src/Antiforgery/src/Internal/CryptographyAlgorithms.cs b/src/Antiforgery/src/Internal/CryptographyAlgorithms.cs
diff --git a/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs b/src/Antiforgery/src/Internal/DefaultClaimUidExtractor.cs
@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Security.Claims;
+using System.Security.Cryptography;
 using Microsoft.Extensions.ObjectPool;
 
 namespace Microsoft.AspNetCore.Antiforgery
@@ -134,9 +135,13 @@ private byte[] ComputeSha256(IEnumerable<string> parameters)
 
                 writer.Flush();
 
-                var sha256 = serializationContext.Sha256;
-                var stream = serializationContext.Stream;
-                var bytes = sha256.ComputeHash(stream.ToArray(), 0, checked((int)stream.Length));
+                bool success = serializationContext.Stream.TryGetBuffer(out ArraySegment<byte> buffer);
+                if (!success)
+                {
+                    throw new InvalidOperationException();
+                }
+
+                var bytes = SHA256.HashData(buffer);
 
                 return bytes;
             }
diff --git a/src/Middleware/WebSockets/src/HandshakeHelpers.cs b/src/Middleware/WebSockets/src/HandshakeHelpers.cs
@@ -110,23 +110,20 @@ public static string CreateResponseKey(string requestKey)
             // this concatenated value to obtain a 20-byte value and base64-encoding"
             // https://tools.ietf.org/html/rfc6455#section-4.2.2
 
-            using (var algorithm = SHA1.Create())
-            {
-                // requestKey is already verified to be small (24 bytes) by 'IsRequestKeyValid()' and everything is 1:1 mapping to UTF8 bytes
-                // so this can be hardcoded to 60 bytes for the requestKey + static websocket string
-                Span<byte> mergedBytes = stackalloc byte[60];
-                Encoding.UTF8.GetBytes(requestKey, mergedBytes);
-                EncodedWebSocketKey.CopyTo(mergedBytes.Slice(24));
-
-                Span<byte> hashedBytes = stackalloc byte[20];
-                var success = algorithm.TryComputeHash(mergedBytes, hashedBytes, out var written);
-                if (!success || written != 20)
-                {
-                    throw new InvalidOperationException("Could not compute the hash for the 'Sec-WebSocket-Accept' header.");
-                }
+            // requestKey is already verified to be small (24 bytes) by 'IsRequestKeyValid()' and everything is 1:1 mapping to UTF8 bytes
+            // so this can be hardcoded to 60 bytes for the requestKey + static websocket string
+            Span<byte> mergedBytes = stackalloc byte[60];
+            Encoding.UTF8.GetBytes(requestKey, mergedBytes);
+            EncodedWebSocketKey.CopyTo(mergedBytes.Slice(24));
 
-                return Convert.ToBase64String(hashedBytes);
+            Span<byte> hashedBytes = stackalloc byte[20];
+            var written = SHA1.HashData(mergedBytes, hashedBytes);
+            if (written != 20)
+            {
+                throw new InvalidOperationException("Could not compute the hash for the 'Sec-WebSocket-Accept' header.");
             }
+
+            return Convert.ToBase64String(hashedBytes);
         }
     }
 }
diff --git a/src/Mvc/Mvc.Razor/src/Infrastructure/CryptographyAlgorithms.cs b/src/Mvc/Mvc.Razor/src/Infrastructure/CryptographyAlgorithms.cs
diff --git a/src/Mvc/Mvc.Razor/src/Infrastructure/DefaultFileVersionProvider.cs b/src/Mvc/Mvc.Razor/src/Infrastructure/DefaultFileVersionProvider.cs
@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Security.Cryptography;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.ViewFeatures;
@@ -97,7 +98,7 @@ public string AddFileVersionToPath(PathString requestPathBase, string path)
 
         private static string GetHashForFile(IFileInfo fileInfo)
         {
-            using (var sha256 = CryptographyAlgorithms.CreateSHA256())
+            using (var sha256 = SHA256.Create())
             {
                 using (var readStream = fileInfo.CreateReadStream())
                 {
@@ -107,4 +108,4 @@ private static string GetHashForFile(IFileInfo fileInfo)
             }
         }
     }
-}
+}
diff --git a/src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs b/src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs
@@ -4,12 +4,11 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
+using System.Security.Cryptography;
 using System.Text;
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Mvc.Razor.Infrastructure;
 using Microsoft.AspNetCore.Razor.TagHelpers;
 using Microsoft.AspNetCore.Routing;
-using Microsoft.Extensions.Internal;
 using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.Mvc.TagHelpers.Cache
@@ -180,12 +179,9 @@ public string GenerateHashedKey()
             // The key is typically too long to be useful, so we use a cryptographic hash
             // as the actual key (better randomization and key distribution, so small vary
             // values will generate dramatically different keys).
-            using (var sha256 = CryptographyAlgorithms.CreateSHA256())
-            {
-                var contentBytes = Encoding.UTF8.GetBytes(key);
-                var hashedBytes = sha256.ComputeHash(contentBytes);
-                return Convert.ToBase64String(hashedBytes);
-            }
+            var contentBytes = Encoding.UTF8.GetBytes(key);
+            var hashedBytes = SHA256.HashData(contentBytes);
+            return Convert.ToBase64String(hashedBytes);
         }
 
         /// <inheritdoc />
diff --git a/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountHandler.cs b/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountHandler.cs
@@ -68,8 +68,7 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
                 // Store this for use during the code redemption.
                 properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
 
-                using var sha256 = SHA256.Create();
-                var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
+                var challengeBytes = SHA256.HashData(Encoding.UTF8.GetBytes(codeVerifier));
                 var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes);
 
                 queryStrings[OAuthConstants.CodeChallengeKey] = codeChallenge;
diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
@@ -280,8 +280,7 @@ protected virtual string BuildChallengeUrl(AuthenticationProperties properties,
                 // Store this for use during the code redemption.
                 properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
 
-                using var sha256 = SHA256.Create();
-                var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
+                var challengeBytes = SHA256.HashData(Encoding.UTF8.GetBytes(codeVerifier));
                 var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes);
 
                 parameters[OAuthConstants.CodeChallengeKey] = codeChallenge;
diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs
@@ -375,8 +375,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert
                 // Store this for use during the code redemption. See RunAuthorizationCodeReceivedEventAsync.
                 properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
 
-                using var sha256 = SHA256.Create();
-                var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
+                var challengeBytes = SHA256.HashData(Encoding.UTF8.GetBytes(codeVerifier));
                 var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes);
 
                 message.Parameters.Add(OAuthConstants.CodeChallengeKey, codeChallenge);
diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs
@@ -23,8 +23,6 @@ internal partial class HttpConnectionManager
         // TODO: Consider making this configurable? At least for testing?
         private static readonly TimeSpan _heartbeatTickRate = TimeSpan.FromSeconds(1);
 
-        private static readonly RNGCryptoServiceProvider _keyGenerator = new RNGCryptoServiceProvider();
-
         private readonly ConcurrentDictionary<string, (HttpConnectionContext Connection, ValueStopwatch Timer)> _connections =
             new ConcurrentDictionary<string, (HttpConnectionContext Connection, ValueStopwatch Timer)>(StringComparer.Ordinal);
         private readonly TimerAwaitable _nextHeartbeat;
@@ -113,7 +111,7 @@ private static string MakeNewConnectionId()
             // 128 bit buffer / 8 bits per byte = 16 bytes
             Span<byte> buffer = stackalloc byte[16];
             // Generate the id with RNGCrypto because we want a cryptographically random id, which GUID is not
-            _keyGenerator.GetBytes(buffer);
+            RandomNumberGenerator.Fill(buffer);
             return WebEncoders.Base64UrlEncode(buffer);
         }
 
diff --git a/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs b/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs
@@ -513,19 +513,7 @@ private static IDictionary<string, string> GetServicePackages(CodeGenerator? typ
 
         private static byte[] GetHash(Stream stream)
         {
-            SHA256 algorithm;
-            try
-            {
-                algorithm = SHA256.Create();
-            }
-            catch (TargetInvocationException)
-            {
-                // SHA256.Create is documented to throw this exception on FIPS-compliant machines. See
-                // https://msdn.microsoft.com/en-us/library/z08hz7ad Fall back to a FIPS-compliant SHA256 algorithm.
-                algorithm = new SHA256CryptoServiceProvider();
-            }
-
-            using (algorithm)
+            using (var algorithm = SHA256.Create())
             {
                 return algorithm.ComputeHash(stream);
             }

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@`
`2`	`2`	`// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.`
`3`	`3`
`4`	`4`	`using System;`
`5`		`-using System.Linq;`
	`5`	`+using System.Security.Cryptography;`
`6`	`6`	`using System.Text;`
`7`	`7`	`using Microsoft.AspNetCore.DataProtection;`
`8`	`8`	`using Microsoft.AspNetCore.WebUtilities;`
`@@ -35,12 +35,8 @@ public void Configure(AntiforgeryOptions options)`
`35`	`35`
`36`	`36`	`private static string ComputeCookieName(string applicationId)`
`37`	`37`	`{`
`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);`
`44`	`40`	`}`
`45`	`41`	`}`
`46`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@`
`2`	`2`	`// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.`
`3`	`3`
`4`	`4`	`using System;`
	`5`	`+using System.Security.Cryptography;`
`5`	`6`	`using Microsoft.AspNetCore.Hosting;`
`6`	`7`	`using Microsoft.AspNetCore.Http;`
`7`	`8`	`using Microsoft.AspNetCore.Mvc.ViewFeatures;`
`@@ -97,7 +98,7 @@ public string AddFileVersionToPath(PathString requestPathBase, string path)`
`97`	`98`
`98`	`99`	`private static string GetHashForFile(IFileInfo fileInfo)`
`99`	`100`	`{`
`100`		`- using (var sha256 = CryptographyAlgorithms.CreateSHA256())`
	`101`	`+ using (var sha256 = SHA256.Create())`
`101`	`102`	`{`
`102`	`103`	`using (var readStream = fileInfo.CreateReadStream())`
`103`	`104`	`{`
`@@ -107,4 +108,4 @@ private static string GetHashForFile(IFileInfo fileInfo)`
`107`	`108`	`}`
`108`	`109`	`}`
`109`	`110`	`}`
`110`		`-}`
	`111`	`+}`