From caf785072330a61b33518ce49a7efcad080f60c9 Mon Sep 17 00:00:00 2001 From: "David R. Williamson" Date: Mon, 4 Oct 2021 16:25:49 -0700 Subject: [PATCH] fix(iot-svc): Cleanup and deprecation warning of code in CryptoKeyGenerator (#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 --- e2e/test/helpers/CryptoKeyGenerator.cs | 62 +++++++++++++ .../ProvisioningServiceClientE2ETests.cs | 2 - .../src/Common/Security/CryptoKeyGenerator.cs | 86 +++++++++++-------- iothub/service/tests/CryptoKeyGenerator.cs | 62 +++++++++++++ .../tests/DeviceAuthenticationTests.cs | 2 +- 5 files changed, 174 insertions(+), 40 deletions(-) create mode 100644 e2e/test/helpers/CryptoKeyGenerator.cs create mode 100644 iothub/service/tests/CryptoKeyGenerator.cs diff --git a/e2e/test/helpers/CryptoKeyGenerator.cs b/e2e/test/helpers/CryptoKeyGenerator.cs new file mode 100644 index 0000000000..8c4f9c82e2 --- /dev/null +++ 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 +{ + /// + /// Utility methods for generating cryptographically secure keys and passwords. + /// + internal static class CryptoKeyGenerator + { +#if NET451 + private const int DefaultPasswordLength = 16; + private const int GuidLength = 16; +#endif + + /// + /// Size of the SHA 512 key. + /// + internal const int Sha512KeySize = 64; + + /// + /// Generate a key with a specified key size. + /// + /// The size of the key. + /// Byte array representing the key. + 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; + } + + /// + /// Generates a key of the specified size. + /// + /// Desired key size. + /// A generated key. + 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 index 72ee57237b..3757ae6825 100644 --- 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 index a5323235e2..03b082ba10 100644 --- a/iothub/service/src/Common/Security/CryptoKeyGenerator.cs +++ b/iothub/service/src/Common/Security/CryptoKeyGenerator.cs @@ -1,17 +1,20 @@ // 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 @@ -19,11 +22,11 @@ namespace Microsoft.Azure.Devices.Common /// /// Utility methods for generating cryptographically secure keys and passwords. /// - 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 /// @@ -36,22 +39,19 @@ static public class CryptoKeyGenerator /// /// The size of the key. /// Byte array representing the key. + [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) /// /// Desired key size. /// A generated key. + [Obsolete("This method will be deprecated in a future version.")] public static string GenerateKey(int keySize) { return Convert.ToBase64String(GenerateKeyBytes(keySize)); @@ -72,14 +73,14 @@ public static string GenerateKey(int keySize) /// Generate a hexadecimal key of the specified size. /// /// Desired key size. - /// A generated hexadecimal key. + /// A generated hexadecimal key. + [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("-", ""); } @@ -87,29 +88,39 @@ public static string GenerateKeyInHex(int keySize) /// Generate a GUID using random bytes from the framework's cryptograpically strong RNG (Random Number Generator). /// /// A cryptographically secure GUID. + [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]); } /// /// Generate a unique password with a default length and without converting it to Base64String. /// /// A unique password. + [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() /// Desired length of the password. /// Encode the password if set to True. False otherwise. /// A generated password. + [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 new file mode 100644 index 0000000000..30cd86c15d --- /dev/null +++ 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 +{ + /// + /// Utility methods for generating cryptographically secure keys and passwords. + /// + internal static class CryptoKeyGenerator + { +#if NET451 + private const int DefaultPasswordLength = 16; + private const int GuidLength = 16; +#endif + + /// + /// Size of the SHA 512 key. + /// + internal const int Sha512KeySize = 64; + + /// + /// Generate a key with a specified key size. + /// + /// The size of the key. + /// Byte array representing the key. + 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; + } + + /// + /// Generates a key of the specified size. + /// + /// Desired key size. + /// A generated key. + 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 index ac933dc3d7..4c2bf8531c 100644 --- 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;