From 298cf264c00071312f6bd29146a9d6d627e5c3b6 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 17 Aug 2020 11:24:48 -0600 Subject: [PATCH] Update IDeriveBytes to allow specifying hash algorithm This makes a couple tests fail on UWP because UWP is consuming the netstandard2.0 build of the library. Adding a uap10.0 target hits https://github.com/novotnyllc/MSBuildSdkExtras/issues/237. --- .../ECDiffieHellmanCngPublicKeyFactory.cs | 2 +- src/PCLCrypto/ECDiffieHellmanFactory.cs | 2 + src/PCLCrypto/IDeriveBytes.cs | 19 ++------ src/PCLCrypto/NetFx/DeriveBytes.cs | 33 +++++++++---- .../NetFx/KeyDerivationCryptographicKey.cs | 31 ++++++++++--- ...ImplementedByReferenceAssemblyException.cs | 2 +- src/PCLCrypto/PCLCrypto.csproj | 4 +- src/PCLCrypto/desktop/ECDiffieHellman.cs | 2 +- .../DeriveBytesTests.cs | 15 +++--- .../KeyDerivationAlgorithmProviderTests.cs | 46 ++++++++----------- .../PCLCrypto.Tests.Shared/PlatformSupport.cs | 8 ++-- test/PCLCrypto.Tests/PCLCrypto.Tests.csproj | 2 +- 12 files changed, 94 insertions(+), 72 deletions(-) diff --git a/src/PCLCrypto/ECDiffieHellmanCngPublicKeyFactory.cs b/src/PCLCrypto/ECDiffieHellmanCngPublicKeyFactory.cs index c5e8b39e..1abb4a27 100644 --- a/src/PCLCrypto/ECDiffieHellmanCngPublicKeyFactory.cs +++ b/src/PCLCrypto/ECDiffieHellmanCngPublicKeyFactory.cs @@ -22,7 +22,7 @@ public IECDiffieHellmanPublicKey FromByteArray(byte[] publicKey) #if NETSTANDARD2_0 throw new NotImplementedByReferenceAssemblyException(); -#elif __IOS__ || __ANDROID__ +#elif __IOS__ || __ANDROID__ || WINDOWS_UWP throw new PlatformNotSupportedException(); #else return new ECDiffieHellmanPublicKey( diff --git a/src/PCLCrypto/ECDiffieHellmanFactory.cs b/src/PCLCrypto/ECDiffieHellmanFactory.cs index b4523978..0e7c7ffb 100644 --- a/src/PCLCrypto/ECDiffieHellmanFactory.cs +++ b/src/PCLCrypto/ECDiffieHellmanFactory.cs @@ -19,6 +19,8 @@ public IECDiffieHellman Create() { #if NETSTANDARD2_0 throw new NotImplementedByReferenceAssemblyException(); +#elif WINDOWS_UWP + throw new PlatformNotSupportedException(); #else return new ECDiffieHellman(Platform.ECDiffieHellman.Create()); #endif diff --git a/src/PCLCrypto/IDeriveBytes.cs b/src/PCLCrypto/IDeriveBytes.cs index 1cd51487..0c70c498 100644 --- a/src/PCLCrypto/IDeriveBytes.cs +++ b/src/PCLCrypto/IDeriveBytes.cs @@ -3,10 +3,7 @@ namespace PCLCrypto { - using System; - using System.Collections.Generic; - using System.Linq; - using System.Text; + using System.Security.Cryptography; /// /// Provides fixed-length key derivation from passwords or byte buffers of arbitrary size. @@ -20,17 +17,11 @@ public interface IDeriveBytes /// The salt. /// The rounds of computation to use in deriving a stronger key. The larger this is, the longer attacks will take. /// The desired key size in bytes. + /// The hash algorithm to use. /// The generated key.byte[] GetBytes(string keyMaterial, byte[] salt, int iterations, int countBytes); - byte[] GetBytes(string keyMaterial, byte[] salt, int iterations, int countBytes); + byte[] GetBytes(string keyMaterial, byte[] salt, int iterations, int countBytes, HashAlgorithmName hashAlgorithm); - /// - /// Derives a cryptographically strong key from the specified bytes. - /// - /// The user-supplied password. - /// The salt. - /// The rounds of computation to use in deriving a stronger key. The larger this is, the longer attacks will take. - /// The desired key size in bytes. - /// The generated key. - byte[] GetBytes(byte[] keyMaterial, byte[] salt, int iterations, int countBytes); + /// + byte[] GetBytes(byte[] keyMaterial, byte[] salt, int iterations, int countBytes, HashAlgorithmName hashAlgorithm); } } diff --git a/src/PCLCrypto/NetFx/DeriveBytes.cs b/src/PCLCrypto/NetFx/DeriveBytes.cs index 84d85420..c9963069 100644 --- a/src/PCLCrypto/NetFx/DeriveBytes.cs +++ b/src/PCLCrypto/NetFx/DeriveBytes.cs @@ -11,40 +11,55 @@ namespace PCLCrypto using System.Threading.Tasks; using Microsoft; +#pragma warning disable CA5379 // Do Not Use Weak Key Derivation Function Algorithm + /// /// Exposes the .NET Framework implementation of . /// internal class DeriveBytes : IDeriveBytes { /// - public byte[] GetBytes(string keyMaterial, byte[] salt, int iterations, int countBytes) + public byte[] GetBytes(string keyMaterial, byte[] salt, int iterations, int countBytes, HashAlgorithmName hashAlgorithm) { Requires.NotNullOrEmpty(keyMaterial, "keyMaterial"); Requires.NotNull(salt, nameof(salt)); Requires.Range(iterations > 0, "iterations"); Requires.Range(countBytes > 0, "countBytes"); - var keyStrengthening = new Rfc2898DeriveBytes(keyMaterial, salt, iterations); - try +#if NETSTANDARD2_0 + if (hashAlgorithm == HashAlgorithmName.SHA1) { + using var keyStrengthening = new Rfc2898DeriveBytes(keyMaterial, salt, iterations); return keyStrengthening.GetBytes(countBytes); } - finally - { - (keyStrengthening as IDisposable)?.Dispose(); - } + + throw new NotImplementedByReferenceAssemblyException(); +#else + using var keyStrengthening = new Rfc2898DeriveBytes(keyMaterial, salt, iterations, hashAlgorithm); + return keyStrengthening.GetBytes(countBytes); +#endif } /// - public byte[] GetBytes(byte[] keyMaterial, byte[] salt, int iterations, int countBytes) + public byte[] GetBytes(byte[] keyMaterial, byte[] salt, int iterations, int countBytes, HashAlgorithmName hashAlgorithm) { Requires.NotNullOrEmpty(keyMaterial, "keyMaterial"); Requires.NotNull(salt, nameof(salt)); Requires.Range(iterations > 0, "iterations"); Requires.Range(countBytes > 0, "countBytes"); - using var keyStrengthening = new Rfc2898DeriveBytes(keyMaterial, salt, iterations); +#if NETSTANDARD2_0 + if (hashAlgorithm == HashAlgorithmName.SHA1) + { + using var keyStrengthening = new Rfc2898DeriveBytes(keyMaterial, salt, iterations); + return keyStrengthening.GetBytes(countBytes); + } + + throw new NotImplementedByReferenceAssemblyException(); +#else + using var keyStrengthening = new Rfc2898DeriveBytes(keyMaterial, salt, iterations, hashAlgorithm); return keyStrengthening.GetBytes(countBytes); +#endif } } } diff --git a/src/PCLCrypto/NetFx/KeyDerivationCryptographicKey.cs b/src/PCLCrypto/NetFx/KeyDerivationCryptographicKey.cs index 87fc5dc3..37c75469 100644 --- a/src/PCLCrypto/NetFx/KeyDerivationCryptographicKey.cs +++ b/src/PCLCrypto/NetFx/KeyDerivationCryptographicKey.cs @@ -4,10 +4,7 @@ namespace PCLCrypto { using System; - using System.Collections.Generic; - using System.Linq; - using System.Text; - using System.Threading.Tasks; + using System.Security.Cryptography; using Microsoft; using Platform = System.Security.Cryptography; @@ -84,6 +81,7 @@ protected internal override byte[] DeriveKeyMaterial(IKeyDerivationParameters pa // more parameter types than just BuildForPbkdf2, we might need to adjust this code // to handle each type of parameter. byte[] salt = parameters.KdfGenericBinary; +#pragma warning disable CA5379 // Do Not Use Weak Key Derivation Function Algorithm switch (this.Algorithm) { case KeyDerivationAlgorithm.Pbkdf2Sha1: @@ -93,10 +91,29 @@ protected internal override byte[] DeriveKeyMaterial(IKeyDerivationParameters pa } default: - // TODO: consider using Platform.PasswordDeriveBytes if it can - // support some more of these algorithms. - throw new NotSupportedException("Only KeyDerivationAlgorithm.Pbkdf2Sha1 is supported for this platform."); +#if NETSTANDARD2_0 + throw new NotImplementedByReferenceAssemblyException(); +#else + using (var deriveBytes = new Platform.Rfc2898DeriveBytes(this.Key, salt, parameters.IterationCount, GetHashAlgorithm(this.Algorithm))) + { + return deriveBytes.GetBytes(desiredKeySize); + } +#endif } +#pragma warning restore CA5379 // Do Not Use Weak Key Derivation Function Algorithm + } + + private static HashAlgorithmName GetHashAlgorithm(KeyDerivationAlgorithm keyDerivationAlgorithm) + { + return keyDerivationAlgorithm switch + { + KeyDerivationAlgorithm.Pbkdf2Md5 => HashAlgorithmName.MD5, + KeyDerivationAlgorithm.Pbkdf2Sha1 => HashAlgorithmName.SHA1, + KeyDerivationAlgorithm.Pbkdf2Sha256 => HashAlgorithmName.SHA256, + KeyDerivationAlgorithm.Pbkdf2Sha384 => HashAlgorithmName.SHA384, + KeyDerivationAlgorithm.Pbkdf2Sha512 => HashAlgorithmName.SHA512, + _ => throw new NotSupportedException($"{keyDerivationAlgorithm} is not supported on this platform."), + }; } } } diff --git a/src/PCLCrypto/NotImplementedByReferenceAssemblyException.cs b/src/PCLCrypto/NotImplementedByReferenceAssemblyException.cs index 4a68ab74..bf8f46e0 100644 --- a/src/PCLCrypto/NotImplementedByReferenceAssemblyException.cs +++ b/src/PCLCrypto/NotImplementedByReferenceAssemblyException.cs @@ -1,7 +1,7 @@ // Copyright (c) Andrew Arnott. All rights reserved. // Licensed under the Microsoft Public License (Ms-PL) license. See LICENSE file in the project root for full license information. -#if NETSTANDARD +#if NETSTANDARD2_0 namespace PCLCrypto { diff --git a/src/PCLCrypto/PCLCrypto.csproj b/src/PCLCrypto/PCLCrypto.csproj index 6f4a7bf0..478c18ba 100644 --- a/src/PCLCrypto/PCLCrypto.csproj +++ b/src/PCLCrypto/PCLCrypto.csproj @@ -1,6 +1,6 @@ - + - net472;netstandard2.0;MonoAndroid9;XamariniOS;netcoreapp2.1 + net472;netstandard2.0;netstandard2.1;MonoAndroid9;XamariniOS;netcoreapp2.1 True PCL Crypto - Portable Crypto APIs diff --git a/src/PCLCrypto/desktop/ECDiffieHellman.cs b/src/PCLCrypto/desktop/ECDiffieHellman.cs index 6d73a362..ae7682e0 100644 --- a/src/PCLCrypto/desktop/ECDiffieHellman.cs +++ b/src/PCLCrypto/desktop/ECDiffieHellman.cs @@ -1,7 +1,7 @@ // Copyright (c) Andrew Arnott. All rights reserved. // Licensed under the Microsoft Public License (Ms-PL) license. See LICENSE file in the project root for full license information. -#if !NETSTANDARD2_0 +#if !(NETSTANDARD2_0 || WINDOWS_UWP) namespace PCLCrypto { diff --git a/test/PCLCrypto.Tests.Shared/DeriveBytesTests.cs b/test/PCLCrypto.Tests.Shared/DeriveBytesTests.cs index da11d143..640c008f 100644 --- a/test/PCLCrypto.Tests.Shared/DeriveBytesTests.cs +++ b/test/PCLCrypto.Tests.Shared/DeriveBytesTests.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography; using System.Text; using PCLCrypto; using Xunit; @@ -20,36 +21,36 @@ public class DeriveBytesTests [Fact] public void GetBytes() { - byte[] keyFromPassword = NetFxCrypto.DeriveBytes.GetBytes(Password1, Salt1, 5, 10); - byte[] keyFromBytes = NetFxCrypto.DeriveBytes.GetBytes(Encoding.UTF8.GetBytes(Password1), Salt1, 5, 10); + byte[] keyFromPassword = NetFxCrypto.DeriveBytes.GetBytes(Password1, Salt1, 5, 10, HashAlgorithmName.SHA1); + byte[] keyFromBytes = NetFxCrypto.DeriveBytes.GetBytes(Encoding.UTF8.GetBytes(Password1), Salt1, 5, 10, HashAlgorithmName.SHA1); CollectionAssertEx.AreEqual(keyFromPassword, keyFromBytes); Assert.Equal(DerivedKey, Convert.ToBase64String(keyFromPassword)); - byte[] keyWithOtherSalt = NetFxCrypto.DeriveBytes.GetBytes(Password1, Salt2, 5, 10); + byte[] keyWithOtherSalt = NetFxCrypto.DeriveBytes.GetBytes(Password1, Salt2, 5, 10, HashAlgorithmName.SHA1); CollectionAssertEx.AreNotEqual(keyFromPassword, keyWithOtherSalt); } [Fact] public void GetBytes_NullBytes() { - Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes((byte[])null!, Salt1, 5, 10)); + Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes((byte[])null!, Salt1, 5, 10, HashAlgorithmName.SHA1)); } [Fact] public void GetBytes_NullPassword() { - Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes((string)null!, Salt1, 5, 10)); + Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes((string)null!, Salt1, 5, 10, HashAlgorithmName.SHA1)); } [Fact] public void GetBytes_Password_NullSalt() { - Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes(Password1, null!, 5, 10)); + Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes(Password1, null!, 5, 10, HashAlgorithmName.SHA1)); } [Fact] public void GetBytes_Bytes_NullSalt() { - Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes(Encoding.UTF8.GetBytes(Password1), null!, 5, 10)); + Assert.Throws(() => NetFxCrypto.DeriveBytes.GetBytes(Encoding.UTF8.GetBytes(Password1), null!, 5, 10, HashAlgorithmName.SHA1)); } } diff --git a/test/PCLCrypto.Tests.Shared/KeyDerivationAlgorithmProviderTests.cs b/test/PCLCrypto.Tests.Shared/KeyDerivationAlgorithmProviderTests.cs index 65b9ec3c..dd6f8c65 100644 --- a/test/PCLCrypto.Tests.Shared/KeyDerivationAlgorithmProviderTests.cs +++ b/test/PCLCrypto.Tests.Shared/KeyDerivationAlgorithmProviderTests.cs @@ -17,11 +17,6 @@ public class KeyDerivationAlgorithmProviderTests private readonly byte[] originalKey = new byte[] { 0x1, 0x2, 0x3, 0x5 }; private readonly byte[] salt = new byte[8]; private readonly int iterations = 100; - private readonly Dictionary stretchedKeyBase64 = new Dictionary - { - { KeyDerivationAlgorithm.Pbkdf2Sha1, "3HWzwI225INl7y6+G9Jv7Af8UGE=" }, - { KeyDerivationAlgorithm.Pbkdf2Sha256, "t420R6yC8H2CDK/0sSGmwKHLooM=" }, - }; private readonly ITestOutputHelper logger; @@ -55,30 +50,29 @@ public void CreateKey_InvalidInputs() () => algorithm.CreateKey(null!)); } - [Fact] - public void CreateKey() + [Theory] + [InlineData(KeyDerivationAlgorithm.Pbkdf2Sha1, "3HWzwI225INl7y6+G9Jv7Af8UGE=")] + [InlineData(KeyDerivationAlgorithm.Pbkdf2Sha256, "t420R6yC8H2CDK/0sSGmwKHLooM=")] + public void CreateKey(KeyDerivationAlgorithm algorithmName, string result) { - foreach (KeyValuePair algorithmAndExpectedResult in this.stretchedKeyBase64) - { - this.logger.WriteLine("Testing algorithm: {0}", algorithmAndExpectedResult.Key); - IKeyDerivationAlgorithmProvider? algorithm = WinRTCrypto.KeyDerivationAlgorithmProvider.OpenAlgorithm(algorithmAndExpectedResult.Key); - ICryptographicKey key = algorithm.CreateKey(this.originalKey); - Assert.NotNull(key); - Assert.Equal(this.originalKey.Length * 8, key.KeySize); + this.logger.WriteLine("Testing algorithm: {0}", algorithmName); + IKeyDerivationAlgorithmProvider? algorithm = WinRTCrypto.KeyDerivationAlgorithmProvider.OpenAlgorithm(algorithmName); + ICryptographicKey key = algorithm.CreateKey(this.originalKey); + Assert.NotNull(key); + Assert.Equal(this.originalKey.Length * 8, key.KeySize); - IKeyDerivationParameters parameters = WinRTCrypto.KeyDerivationParameters.BuildForPbkdf2(this.salt, this.iterations); - Assert.Equal(this.iterations, parameters.IterationCount); - CollectionAssertEx.AreEqual(this.salt, parameters.KdfGenericBinary); + IKeyDerivationParameters parameters = WinRTCrypto.KeyDerivationParameters.BuildForPbkdf2(this.salt, this.iterations); + Assert.Equal(this.iterations, parameters.IterationCount); + CollectionAssertEx.AreEqual(this.salt, parameters.KdfGenericBinary); - try - { - byte[] keyMaterial = WinRTCrypto.CryptographicEngine.DeriveKeyMaterial(key, parameters, 20); - Assert.Equal(algorithmAndExpectedResult.Value, Convert.ToBase64String(keyMaterial)); - } - catch (NotSupportedException) - { - this.logger.WriteLine(" - Not supported on this platform"); - } + try + { + byte[] keyMaterial = WinRTCrypto.CryptographicEngine.DeriveKeyMaterial(key, parameters, 20); + Assert.Equal(result, Convert.ToBase64String(keyMaterial)); + } + catch (NotSupportedException) + { + this.logger.WriteLine(" - Not supported on this platform"); } } } diff --git a/test/PCLCrypto.Tests.Shared/PlatformSupport.cs b/test/PCLCrypto.Tests.Shared/PlatformSupport.cs index 4f9249b2..5922d980 100644 --- a/test/PCLCrypto.Tests.Shared/PlatformSupport.cs +++ b/test/PCLCrypto.Tests.Shared/PlatformSupport.cs @@ -108,10 +108,12 @@ public void PublicKeyFormat(CryptographicPublicKeyBlobType format) Assert.NotEmpty(serialized); } - [SkippableFact(typeof(NotSupportedException), typeof(PlatformNotSupportedException))] - public void KeyDerivation_Pbkdf2() + [SkippableTheory(typeof(NotSupportedException), typeof(PlatformNotSupportedException))] + [InlineData(KeyDerivationAlgorithm.Pbkdf2Sha1)] + [InlineData(KeyDerivationAlgorithm.Pbkdf2Sha256)] + public void KeyDerivation_Pbkdf2(KeyDerivationAlgorithm algorithmName) { - IKeyDerivationAlgorithmProvider? provider = WinRTCrypto.KeyDerivationAlgorithmProvider.OpenAlgorithm(KeyDerivationAlgorithm.Pbkdf2Sha256); + IKeyDerivationAlgorithmProvider? provider = WinRTCrypto.KeyDerivationAlgorithmProvider.OpenAlgorithm(algorithmName); ICryptographicKey? originalKey = provider.CreateKey(Encoding.UTF8.GetBytes("my secret")); var salt = WinRTCrypto.CryptographicBuffer.GenerateRandom(32); const int iterationCount = 2; diff --git a/test/PCLCrypto.Tests/PCLCrypto.Tests.csproj b/test/PCLCrypto.Tests/PCLCrypto.Tests.csproj index 04f8c318..6044fb6b 100644 --- a/test/PCLCrypto.Tests/PCLCrypto.Tests.csproj +++ b/test/PCLCrypto.Tests/PCLCrypto.Tests.csproj @@ -1,6 +1,6 @@  - net472;netcoreapp2.1 + net472;netcoreapp2.1;netcoreapp3.1