fix(iot-svc): Cleanup and deprecation warning of code in CryptoKeyGen…

…erator (#2187) * fix(iot-svc): Add support for GeneratePassword to other targets * Don't add net472 support, but keep code cleanup * Add deprecated attribute * Fix test references by making a local copy for tests
Azure · Oct 4, 2021 · caf7850 · caf7850
1 parent 57629ff
commit caf7850
Show file tree

Hide file tree

Showing 5 changed files with 174 additions and 40 deletions.
diff --git a/e2e/test/helpers/CryptoKeyGenerator.cs b/e2e/test/helpers/CryptoKeyGenerator.cs
@@ -0,0 +1,62 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Security.Cryptography;
+
+#if !NET451
+
+using System.Linq;
+
+#endif
+
+namespace Microsoft.Azure.Devices.E2ETests
+{
+    /// <summary>
+    /// Utility methods for generating cryptographically secure keys and passwords.
+    /// </summary>
+    internal static class CryptoKeyGenerator
+    {
+#if NET451
+        private const int DefaultPasswordLength = 16;
+        private const int GuidLength = 16;
+#endif
+
+        /// <summary>
+        /// Size of the SHA 512 key.
+        /// </summary>
+        internal const int Sha512KeySize = 64;
+
+        /// <summary>
+        /// Generate a key with a specified key size.
+        /// </summary>
+        /// <param name="keySize">The size of the key.</param>
+        /// <returns>Byte array representing the key.</returns>
+        internal static byte[] GenerateKeyBytes(int keySize)
+        {
+#if NET451
+            byte[] keyBytes = new byte[keySize];
+            using var cyptoProvider = new RNGCryptoServiceProvider();
+            cyptoProvider.GetNonZeroBytes(keyBytes);
+#else
+            byte[] keyBytes = new byte[keySize];
+            using var cyptoProvider = RandomNumberGenerator.Create();
+            while (keyBytes.Contains(byte.MinValue))
+            {
+                cyptoProvider.GetBytes(keyBytes);
+            }
+#endif
+            return keyBytes;
+        }
+
+        /// <summary>
+        /// Generates a key of the specified size.
+        /// </summary>
+        /// <param name="keySize">Desired key size.</param>
+        /// <returns>A generated key.</returns>
+        internal static string GenerateKey(int keySize)
+        {
+            return Convert.ToBase64String(GenerateKeyBytes(keySize));
+        }
+    }
+}
diff --git a/e2e/test/provisioning/ProvisioningServiceClientE2ETests.cs b/e2e/test/provisioning/ProvisioningServiceClientE2ETests.cs
@@ -3,10 +3,8 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics.Tracing;
 using System.Net;
 using System.Threading.Tasks;
-using Microsoft.Azure.Devices.Common;
 using Microsoft.Azure.Devices.Provisioning.Security.Samples;
 using Microsoft.Azure.Devices.Provisioning.Service;
 using Microsoft.Azure.Devices.Shared;

diff --git a/iothub/service/src/Common/Security/CryptoKeyGenerator.cs b/iothub/service/src/Common/Security/CryptoKeyGenerator.cs
@@ -1,29 +1,32 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System.Linq;
 using System;
 using System.Text;
+using System.Security.Cryptography;
+
+#if NET451
+
+using System.Web.Security;
+
+#endif
 
 #if !NET451
 
-using System.Security.Cryptography;
+using System.Linq;
 
-#else
-    using System.Web.Security;
-    using System.Security.Cryptography;
 #endif
 
 namespace Microsoft.Azure.Devices.Common
 {
     /// <summary>
     /// Utility methods for generating cryptographically secure keys and passwords.
     /// </summary>
-    static public class CryptoKeyGenerator
+    public static class CryptoKeyGenerator
     {
 #if NET451
-        const int DefaultPasswordLength = 16;
-        const int GuidLength = 16;
+        private const int DefaultPasswordLength = 16;
+        private const int GuidLength = 16;
 #endif
 
         /// <summary>
@@ -36,22 +39,19 @@ static public class CryptoKeyGenerator
         /// </summary>
         /// <param name="keySize">The size of the key.</param>
         /// <returns>Byte array representing the key.</returns>
+        [Obsolete("This method will be deprecated in a future version.")]
         public static byte[] GenerateKeyBytes(int keySize)
         {
-#if !NET451
-            var keyBytes = new byte[keySize];
-            using (var cyptoProvider = RandomNumberGenerator.Create())
-            {
-                while (keyBytes.Contains(byte.MinValue))
-                {
-                    cyptoProvider.GetBytes(keyBytes);
-                }
-            }
+#if NET451
+            byte[] keyBytes = new byte[keySize];
+            using var cyptoProvider = new RNGCryptoServiceProvider();
+            cyptoProvider.GetNonZeroBytes(keyBytes);
 #else
-            var keyBytes = new byte[keySize];
-            using (var cyptoProvider = new RNGCryptoServiceProvider())
+            byte[] keyBytes = new byte[keySize];
+            using var cyptoProvider = RandomNumberGenerator.Create();
+            while (keyBytes.Contains(byte.MinValue))
             {
-                cyptoProvider.GetNonZeroBytes(keyBytes);
+                cyptoProvider.GetBytes(keyBytes);
             }
 #endif
             return keyBytes;
@@ -62,6 +62,7 @@ public static byte[] GenerateKeyBytes(int keySize)
         /// </summary>
         /// <param name="keySize">Desired key size.</param>
         /// <returns>A generated key.</returns>
+        [Obsolete("This method will be deprecated in a future version.")]
         public static string GenerateKey(int keySize)
         {
             return Convert.ToBase64String(GenerateKeyBytes(keySize));
@@ -72,44 +73,54 @@ public static string GenerateKey(int keySize)
         /// Generate a hexadecimal key of the specified size.
         /// </summary>
         /// <param name="keySize">Desired key size.</param>
-        /// <returns>A generated hexadecimal key.</returns>        
+        /// <returns>A generated hexadecimal key.</returns>
+        [Obsolete("This method will not be carried forward to newer .NET targets.")]
         public static string GenerateKeyInHex(int keySize)
         {
-            var keyBytes = new byte[keySize];
-            using (var cyptoProvider = new RNGCryptoServiceProvider())
-            {
-                cyptoProvider.GetNonZeroBytes(keyBytes);
-            }
+            byte[] keyBytes = new byte[keySize];
+            using var cyptoProvider = new RNGCryptoServiceProvider();
+            cyptoProvider.GetNonZeroBytes(keyBytes);
+
             return BitConverter.ToString(keyBytes).Replace("-", "");
         }
 
         /// <summary>
         /// Generate a GUID using random bytes from the framework's cryptograpically strong RNG (Random Number Generator).
         /// </summary>
         /// <returns>A cryptographically secure GUID.</returns>
+        [Obsolete("This method will not be carried forward to newer .NET targets.")]
         public static Guid GenerateGuid()
         {
             byte[] bytes = new byte[GuidLength];
-            using (var rng = new RNGCryptoServiceProvider())
-            {
-                rng.GetBytes(bytes);
-            }
+            using var rng = new RNGCryptoServiceProvider();
+            rng.GetBytes(bytes);
 
-            var time = BitConverter.ToUInt32(bytes, 0);
-            var time_mid = BitConverter.ToUInt16(bytes, 4);
-            var time_hi_and_ver = BitConverter.ToUInt16(bytes, 6);
-            time_hi_and_ver = (ushort)((time_hi_and_ver | 0x4000) & 0x4FFF);
+            uint time = BitConverter.ToUInt32(bytes, 0);
+            ushort timeMid = BitConverter.ToUInt16(bytes, 4);
+            ushort timeHiAndVer = BitConverter.ToUInt16(bytes, 6);
+            timeHiAndVer = (ushort)((timeHiAndVer | 0x4000) & 0x4FFF);
 
             bytes[8] = (byte)((bytes[8] | 0x80) & 0xBF);
 
-            return new Guid(time, time_mid, time_hi_and_ver, bytes[8], bytes[9],
-                bytes[10], bytes[11], bytes[12], bytes[13], bytes[14], bytes[15]);
+            return new Guid(
+                time,
+                timeMid,
+                timeHiAndVer,
+                bytes[8],
+                bytes[9],
+                bytes[10],
+                bytes[11],
+                bytes[12],
+                bytes[13],
+                bytes[14],
+                bytes[15]);
         }
 
         /// <summary>
         /// Generate a unique password with a default length and without converting it to Base64String.
         /// </summary>
         /// <returns>A unique password.</returns>
+        [Obsolete("This method will not be carried forward to newer .NET targets.")]
         public static string GeneratePassword()
         {
             return GeneratePassword(DefaultPasswordLength, false);
@@ -121,9 +132,10 @@ public static string GeneratePassword()
         /// <param name="length">Desired length of the password.</param>
         /// <param name="base64Encoding">Encode the password if set to True. False otherwise.</param>
         /// <returns>A generated password.</returns>
+        [Obsolete("This method will not be carried forward to newer .NET targets.")]
         public static string GeneratePassword(int length, bool base64Encoding)
         {
-            var password = Membership.GeneratePassword(length, length / 2);
+            string password = Membership.GeneratePassword(length, length / 2);
             if (base64Encoding)
             {
                 password = Convert.ToBase64String(Encoding.UTF8.GetBytes(password));

diff --git a/iothub/service/tests/CryptoKeyGenerator.cs b/iothub/service/tests/CryptoKeyGenerator.cs
@@ -0,0 +1,62 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Security.Cryptography;
+
+#if !NET451
+
+using System.Linq;
+
+#endif
+
+namespace Microsoft.Azure.Devices.Tests
+{
+    /// <summary>
+    /// Utility methods for generating cryptographically secure keys and passwords.
+    /// </summary>
+    internal static class CryptoKeyGenerator
+    {
+#if NET451
+        private const int DefaultPasswordLength = 16;
+        private const int GuidLength = 16;
+#endif
+
+        /// <summary>
+        /// Size of the SHA 512 key.
+        /// </summary>
+        internal const int Sha512KeySize = 64;
+
+        /// <summary>
+        /// Generate a key with a specified key size.
+        /// </summary>
+        /// <param name="keySize">The size of the key.</param>
+        /// <returns>Byte array representing the key.</returns>
+        internal static byte[] GenerateKeyBytes(int keySize)
+        {
+#if NET451
+            byte[] keyBytes = new byte[keySize];
+            using var cyptoProvider = new RNGCryptoServiceProvider();
+            cyptoProvider.GetNonZeroBytes(keyBytes);
+#else
+            byte[] keyBytes = new byte[keySize];
+            using var cyptoProvider = RandomNumberGenerator.Create();
+            while (keyBytes.Contains(byte.MinValue))
+            {
+                cyptoProvider.GetBytes(keyBytes);
+            }
+#endif
+            return keyBytes;
+        }
+
+        /// <summary>
+        /// Generates a key of the specified size.
+        /// </summary>
+        /// <param name="keySize">Desired key size.</param>
+        /// <returns>A generated key.</returns>
+        internal static string GenerateKey(int keySize)
+        {
+            return Convert.ToBase64String(GenerateKeyBytes(keySize));
+        }
+    }
+}
diff --git a/iothub/service/tests/DeviceAuthenticationTests.cs b/iothub/service/tests/DeviceAuthenticationTests.cs
@@ -7,7 +7,7 @@
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.Azure.Devices.Common;
+using Microsoft.Azure.Devices.Tests;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;