Clean up Abstractions / use HashData (#1451)

* Clean up Abstractions / use HashData

- replace usages of GenerateRandom(byte[]) with GenerateRandom(int)
- remove Create() functions and make use of static HashData methods
  on .NET 6+, see https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1850
- Remove unused ReflectionAbstraction

* Use Abstractions for HashData

Co-authored-by: Robert Hague <rh@johnstreetcapital.com>

---------

Co-authored-by: Robert Hague <rh@johnstreetcapital.com>

src/Renci.SshNet/Abstractions/CryptoAbstraction.cs

@@ -1,10 +1,10 @@
-using System;
+using System.Security.Cryptography;
 
 namespace Renci.SshNet.Abstractions
 {
     internal static class CryptoAbstraction
     {
-        private static readonly System.Security.Cryptography.RandomNumberGenerator Randomizer = CreateRandomNumberGenerator();
+        private static readonly RandomNumberGenerator Randomizer = RandomNumberGenerator.Create();
 
         /// <summary>
         /// Generates a <see cref="byte"/> array of the specified length, and fills it with a
@@ -14,51 +14,68 @@ namespace Renci.SshNet.Abstractions
         public static byte[] GenerateRandom(int length)
         {
             var random = new byte[length];
-            GenerateRandom(random);
+            Randomizer.GetBytes(random);
             return random;
         }
 
-        /// <summary>
-        /// Fills an array of bytes with a cryptographically strong random sequence of values.
-        /// </summary>
-        /// <param name="data">The array to fill with cryptographically strong random bytes.</param>
-        /// <exception cref="ArgumentNullException"><paramref name="data"/> is <see langword="null"/>.</exception>
-        /// <remarks>
-        /// The length of the byte array determines how many random bytes are produced.
-        /// </remarks>
-        public static void GenerateRandom(byte[] data)
-        {
-            Randomizer.GetBytes(data);
-        }
-
-        public static System.Security.Cryptography.RandomNumberGenerator CreateRandomNumberGenerator()
-        {
-            return System.Security.Cryptography.RandomNumberGenerator.Create();
-        }
-
-        public static System.Security.Cryptography.MD5 CreateMD5()
+        public static byte[] HashMD5(byte[] source)
         {
-            return System.Security.Cryptography.MD5.Create();
+#if NET
+            return MD5.HashData(source);
+#else
+            using (var md5 = MD5.Create())
+            {
+                return md5.ComputeHash(source);
+            }
+#endif
         }
 
-        public static System.Security.Cryptography.SHA1 CreateSHA1()
+        public static byte[] HashSHA1(byte[] source)
         {
-            return System.Security.Cryptography.SHA1.Create();
+#if NET
+            return SHA1.HashData(source);
+#else
+            using (var sha1 = SHA1.Create())
+            {
+                return sha1.ComputeHash(source);
+            }
+#endif
         }
 
-        public static System.Security.Cryptography.SHA256 CreateSHA256()
+        public static byte[] HashSHA256(byte[] source)
         {
-            return System.Security.Cryptography.SHA256.Create();
+#if NET
+            return SHA256.HashData(source);
+#else
+            using (var sha256 = SHA256.Create())
+            {
+                return sha256.ComputeHash(source);
+            }
+#endif
         }
 
-        public static System.Security.Cryptography.SHA384 CreateSHA384()
+        public static byte[] HashSHA384(byte[] source)
         {
-            return System.Security.Cryptography.SHA384.Create();
+#if NET
+            return SHA384.HashData(source);
+#else
+            using (var sha384 = SHA384.Create())
+            {
+                return sha384.ComputeHash(source);
+            }
+#endif
         }
 
-        public static System.Security.Cryptography.SHA512 CreateSHA512()
+        public static byte[] HashSHA512(byte[] source)
         {
-            return System.Security.Cryptography.SHA512.Create();
+#if NET
+            return SHA512.HashData(source);
+#else
+            using (var sha512 = SHA512.Create())
+            {
+                return sha512.ComputeHash(source);
+            }
+#endif
         }
     }
 }

src/Renci.SshNet/Abstractions/ReflectionAbstraction.cs

@@ -1,16 +0,0 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-
-namespace Renci.SshNet.Abstractions
-{
-    internal static class ReflectionAbstraction
-    {
-        public static IEnumerable<T> GetCustomAttributes<T>(this Type type, bool inherit)
-            where T : Attribute
-        {
-            var attributes = type.GetCustomAttributes(typeof(T), inherit);
-            return attributes.Cast<T>();
-        }
-    }
-}

src/Renci.SshNet/Common/BigInteger.cs

@@ -179,8 +179,7 @@ namespace Renci.SshNet.Common
         /// <returns>A random number of the specified length.</returns>
         public static BigInteger Random(int bitLength)
         {
-            var bytesArray = new byte[(bitLength / 8) + (((bitLength % 8) > 0) ? 1 : 0)];
-            CryptoAbstraction.GenerateRandom(bytesArray);
+            var bytesArray = CryptoAbstraction.GenerateRandom((bitLength / 8) + (((bitLength % 8) > 0) ? 1 : 0));
             bytesArray[bytesArray.Length - 1] = (byte)(bytesArray[bytesArray.Length - 1] & 0x7F); // Ensure not a negative value
             return new BigInteger(bytesArray);
         }

src/Renci.SshNet/Common/HostKeyEventArgs.cs

@@ -100,17 +100,11 @@ namespace Renci.SshNet.Common
             HostKeyName = host.Name;
             KeyLength = host.Key.KeyLength;
 
-            _lazyFingerPrint = new Lazy<byte[]>(() =>
-                {
-                    using var md5 = CryptoAbstraction.CreateMD5();
-                    return md5.ComputeHash(HostKey);
-                });
+            _lazyFingerPrint = new Lazy<byte[]>(() => CryptoAbstraction.HashMD5(HostKey));
 
             _lazyFingerPrintSHA256 = new Lazy<string>(() =>
                 {
-                    using var sha256 = CryptoAbstraction.CreateSHA256();
-
-                    return Convert.ToBase64String(sha256.ComputeHash(HostKey))
+                    return Convert.ToBase64String(CryptoAbstraction.HashSHA256(HostKey))
 #if NET || NETSTANDARD2_1_OR_GREATER
                                   .Replace("=", string.Empty, StringComparison.Ordinal);
 #else

src/Renci.SshNet/Messages/Message.cs

@@ -83,8 +83,7 @@ namespace Renci.SshNet.Messages
                     var paddingLength = GetPaddingLength(paddingMultiplier, excludePacketLengthFieldWhenPadding ? packetLength - 4 : packetLength);
 
                     // add padding bytes
-                    var paddingBytes = new byte[paddingLength];
-                    CryptoAbstraction.GenerateRandom(paddingBytes);
+                    var paddingBytes = CryptoAbstraction.GenerateRandom(paddingLength);
                     sshDataStream.Write(paddingBytes, 0, paddingLength);
 
                     var packetDataLength = GetPacketDataLength(messageLength, paddingLength);
@@ -128,8 +127,7 @@ namespace Renci.SshNet.Messages
                     WriteBytes(sshDataStream);
 
                     // add padding bytes
-                    var paddingBytes = new byte[paddingLength];
-                    CryptoAbstraction.GenerateRandom(paddingBytes);
+                    var paddingBytes = CryptoAbstraction.GenerateRandom(paddingLength);
                     sshDataStream.Write(paddingBytes, 0, paddingLength);
 
                     return sshDataStream.ToArray();

src/Renci.SshNet/Messages/Transport/KeyExchangeInitMessage.cs

@@ -12,9 +12,7 @@ namespace Renci.SshNet.Messages.Transport
         /// </summary>
         public KeyExchangeInitMessage()
         {
-            var cookie = new byte[16];
-            CryptoAbstraction.GenerateRandom(cookie);
-            Cookie = cookie;
+            Cookie = CryptoAbstraction.GenerateRandom(16);
         }
 
         #region Message Properties

src/Renci.SshNet/PrivateKeyFile.cs

@@ -7,7 +7,6 @@ using System.Security.Cryptography;
 using System.Text;
 using System.Text.RegularExpressions;
 
-using Renci.SshNet.Abstractions;
 using Renci.SshNet.Common;
 using Renci.SshNet.Security;
 using Renci.SshNet.Security.Cryptography;
@@ -380,7 +379,8 @@ namespace Renci.SshNet
         {
             var cipherKey = new List<byte>();
 
-            using (var md5 = CryptoAbstraction.CreateMD5())
+#pragma warning disable CA1850 // Prefer static HashData method; We'll reuse the object on lower targets.
+            using (var md5 = MD5.Create())
             {
                 var passwordBytes = Encoding.UTF8.GetBytes(passphrase);
 
@@ -394,6 +394,7 @@ namespace Renci.SshNet
                     cipherKey.AddRange(hash);
                 }
             }
+#pragma warning restore CA1850 // Prefer static HashData method
 
             return cipherKey.ToArray().Take(length);
         }
@@ -426,7 +427,8 @@ namespace Renci.SshNet
 
             var cipherKey = new List<byte>();
 
-            using (var md5 = CryptoAbstraction.CreateMD5())
+#pragma warning disable CA1850 // Prefer static HashData method; We'll reuse the object on lower targets.
+            using (var md5 = MD5.Create())
             {
                 var passwordBytes = Encoding.UTF8.GetBytes(passPhrase);
 
@@ -443,6 +445,7 @@ namespace Renci.SshNet
                     cipherKey.AddRange(hash);
                 }
             }
+#pragma warning restore CA1850 // Prefer static HashData method
 
             var cipher = cipherInfo.Cipher(cipherKey.ToArray(), binarySalt);
 

src/Renci.SshNet/Security/Cryptography/Bcrypt.cs

@@ -852,7 +852,7 @@ namespace Renci.SshNet.Security.Cryptography
         /// <param name="output"></param>
         public void Pbkdf(byte[] password, byte[] salt, int rounds, byte[] output)
         {
-            using (var sha512 = CryptoAbstraction.CreateSHA512())
+            using (var sha512 = SHA512.Create())
             {
                 int nblocks = (output.Length + 31) / 32;
                 byte[] hpass = sha512.ComputeHash(password);

src/Renci.SshNet/Security/Cryptography/DsaDigitalSignature.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Security.Cryptography;
 
-using Renci.SshNet.Abstractions;
 using Renci.SshNet.Common;
 
 namespace Renci.SshNet.Security.Cryptography
@@ -29,7 +28,7 @@ namespace Renci.SshNet.Security.Cryptography
 
             _key = key;
 
-            _hash = CryptoAbstraction.CreateSHA1();
+            _hash = SHA1.Create();
         }
 
         /// <summary>

src/Renci.SshNet/Security/KeyExchangeDiffieHellmanGroupExchangeSha1.cs

@@ -35,10 +35,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha1 = CryptoAbstraction.CreateSHA1())
-            {
-                return sha1.ComputeHash(hashData, 0, hashData.Length);
-            }
+            return CryptoAbstraction.HashSHA1(hashData);
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeDiffieHellmanGroupExchangeSha256.cs

@@ -35,10 +35,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha256 = CryptoAbstraction.CreateSHA256())
-            {
-                return sha256.ComputeHash(hashData);
-            }
+            return CryptoAbstraction.HashSHA256(hashData);
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeDiffieHellmanGroupSha1.cs

@@ -27,10 +27,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha1 = CryptoAbstraction.CreateSHA1())
-            {
-                return sha1.ComputeHash(hashData, 0, hashData.Length);
-            }
+            return CryptoAbstraction.HashSHA1(hashData);
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeDiffieHellmanGroupSha256.cs

@@ -27,10 +27,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha256 = CryptoAbstraction.CreateSHA256())
-            {
-                return sha256.ComputeHash(hashData);
-            }
+            return CryptoAbstraction.HashSHA256(hashData);
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeDiffieHellmanGroupSha512.cs

@@ -27,10 +27,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha512 = CryptoAbstraction.CreateSHA512())
-            {
-                return sha512.ComputeHash(hashData);
-            }
+            return CryptoAbstraction.HashSHA512(hashData);
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeECCurve25519.cs

@@ -68,10 +68,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha256 = CryptoAbstraction.CreateSHA256())
-            {
-                return sha256.ComputeHash(hashData, 0, hashData.Length);
-            }
+            return CryptoAbstraction.HashSHA256(hashData);
         }
 
         private void Session_KeyExchangeEcdhReplyMessageReceived(object sender, MessageEventArgs<KeyExchangeEcdhReplyMessage> e)

src/Renci.SshNet/Security/KeyExchangeECDH256.cs

@@ -1,4 +1,4 @@
-using Org.BouncyCastle.Asn1.Sec;
+using Org.BouncyCastle.Asn1.Sec;
 using Org.BouncyCastle.Asn1.X9;
 
 using Renci.SshNet.Abstractions;
@@ -46,10 +46,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha256 = CryptoAbstraction.CreateSHA256())
-            {
-                return sha256.ComputeHash(hashData, 0, hashData.Length);
-            }
+            return CryptoAbstraction.HashSHA256(hashData);
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeECDH384.cs

@@ -1,4 +1,4 @@
-using Org.BouncyCastle.Asn1.Sec;
+using Org.BouncyCastle.Asn1.Sec;
 using Org.BouncyCastle.Asn1.X9;
 
 using Renci.SshNet.Abstractions;
@@ -46,10 +46,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha384 = CryptoAbstraction.CreateSHA384())
-            {
-                return sha384.ComputeHash(hashData, 0, hashData.Length);
-            }
+            return CryptoAbstraction.HashSHA384(hashData);
         }
     }
 }

src/Renci.SshNet/Security/KeyExchangeECDH521.cs

@@ -1,4 +1,4 @@
-using Org.BouncyCastle.Asn1.Sec;
+using Org.BouncyCastle.Asn1.Sec;
 using Org.BouncyCastle.Asn1.X9;
 
 using Renci.SshNet.Abstractions;
@@ -46,10 +46,7 @@ namespace Renci.SshNet.Security
         /// </returns>
         protected override byte[] Hash(byte[] hashData)
         {
-            using (var sha512 = CryptoAbstraction.CreateSHA512())
-            {
-                return sha512.ComputeHash(hashData, 0, hashData.Length);
-            }
+            return CryptoAbstraction.HashSHA512(hashData);
         }
     }
 }

test/Renci.SshNet.Tests/Classes/SessionTest_ConnectingBase.cs

@@ -9,6 +9,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 using Moq;
 
+using Renci.SshNet.Abstractions;
 using Renci.SshNet.Common;
 using Renci.SshNet.Compression;
 using Renci.SshNet.Connection;
@@ -181,7 +182,8 @@ namespace Renci.SshNet.Tests.Classes
                     {
                         var serviceAcceptMessage = ServiceAcceptMessageBuilder.Create(ServiceName.UserAuthentication)
                                                                               .Build(ServerOutboundPacketSequence);
-                        var hash = Abstractions.CryptoAbstraction.CreateSHA256().ComputeHash(serviceAcceptMessage);
+
+                        var hash = CryptoAbstraction.HashSHA256(serviceAcceptMessage);
 
                         var packet = new byte[serviceAcceptMessage.Length - 4 + hash.Length];