From 5f15fdf20c052b5c99ef6470dff19e9c66097f14 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 8 Feb 2021 14:16:54 -0500 Subject: [PATCH 01/29] Basic implementation of one-shot PBKDF2 for all platforms. --- .../Interop.Pbkdf2.cs | 62 ++++++++ .../Interop.EVP.cs | 35 ++++- .../BCrypt/Interop.BCryptAlgPseudoHandle.cs | 28 ++++ .../BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs | 38 +++++ .../Windows/BCrypt/Interop.BCryptHash.cs | 11 -- .../CMakeLists.txt | 1 + .../entrypoints.c | 1 + .../pal_keyderivation.c | 64 +++++++++ .../pal_keyderivation.h | 19 +++ .../entrypoints.c | 1 + .../opensslshim.h | 2 + .../pal_evp.c | 19 +++ .../pal_evp.h | 9 ++ ...System.Security.Cryptography.Algorithms.cs | 7 +- .../HashProviderDispenser.Unix.cs | 2 +- .../Cryptography/Pbkdf2Implementation.OSX.cs | 30 ++++ .../Cryptography/Pbkdf2Implementation.Unix.cs | 30 ++++ .../Pbkdf2Implementation.Windows.cs | 51 +++++++ .../src/Resources/Strings.resx | 9 ++ ...em.Security.Cryptography.Algorithms.csproj | 10 ++ .../Rfc2898DeriveBytes.OneShot.cs | 134 ++++++++++++++++++ .../Cryptography/Rfc2898DeriveBytes.cs | 2 +- .../tests/Rfc2898OneShotTests.cs | 72 ++++++++++ ...urity.Cryptography.Algorithms.Tests.csproj | 1 + 24 files changed, 623 insertions(+), 15 deletions(-) create mode 100644 src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs create mode 100644 src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs create mode 100644 src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs create mode 100644 src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c create mode 100644 src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h create mode 100644 src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs create mode 100644 src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs create mode 100644 src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs create mode 100644 src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs create mode 100644 src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs new file mode 100644 index 00000000000000..33b6ed7e760fce --- /dev/null +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Security.Cryptography; + +internal static partial class Interop +{ + internal static partial class AppleCrypto + { + internal static unsafe void KeyDerivationPBKDF( + PAL_HashAlgorithm prfAlgorithm, + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + Span destination) + { + fixed (byte* pPassword = password) + fixed (byte* pSalt = salt) + fixed (byte* pDestination = destination) + { + int ret = AppleCryptoNative_KeyDerivationPBKDF( + prfAlgorithm, + pPassword, + password.Length, + pSalt, + salt.Length, + iterations, + pDestination, + destination.Length, + out int ccStatus); + + if (ret == 0) + { + throw Interop.AppleCrypto.CreateExceptionForCCError( + ccStatus, + Interop.AppleCrypto.CCCryptorStatus); + } + + if (ret != 1) + { + Debug.Fail($"KeyDerivationPBKDF failed with invalid input {ret}"); + throw new CryptographicException(); + } + } + } + + [DllImport(Libraries.AppleCryptoNative)] + private static extern unsafe int AppleCryptoNative_KeyDerivationPBKDF( + PAL_HashAlgorithm prfAlgorithm, + byte* password, + int passwordLen, + byte* salt, + int saltLen, + int iterations, + byte* derivedKey, + int derivedKeyLen, + out int errorCode); + } +} diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs index ab184581dbcbf6..096387f893a7e6 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs @@ -51,10 +51,43 @@ internal static int EvpDigestUpdate(SafeEvpMdCtxHandle ctx, ReadOnlySpan d [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpSha512")] internal static extern IntPtr EvpSha512(); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetMaxMdSize")] private static extern int GetMaxMdSize(); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_Pkcs5Pbkdf2Hmac")] + private static unsafe extern int Pkcs5Pbkdf2Hmac( + byte* pPassword, + int passwordLength, + byte* pSalt, + int saltLength, + int iterations, + IntPtr digestEvp, + byte* pDestination, + int destinationLength); + + internal static unsafe int Pkcs5Pbkdf2Hmac( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + IntPtr digestEvp, + Span destination) + { + fixed (byte* pPassword = password) + fixed (byte* pSalt = salt) + fixed (byte* pDestination = destination) + { + return Pkcs5Pbkdf2Hmac( + pPassword, + password.Length, + pSalt, + salt.Length, + iterations, + digestEvp, + pDestination, + destination.Length); + } + } + internal static readonly int EVP_MAX_MD_SIZE = GetMaxMdSize(); } } diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs new file mode 100644 index 00000000000000..c56ab953e5d194 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal partial class Interop +{ + internal partial class BCrypt + { + // Pseudo-handles, as defined in bcrypt.h + // TODO: This really should be backed by 'nuint' (see https://github.com/dotnet/roslyn/issues/44110) + public enum BCryptAlgPseudoHandle : uint + { + BCRYPT_MD5_ALG_HANDLE = 0x00000021, + BCRYPT_SHA1_ALG_HANDLE = 0x00000031, + BCRYPT_SHA256_ALG_HANDLE = 0x00000041, + BCRYPT_SHA384_ALG_HANDLE = 0x00000051, + BCRYPT_SHA512_ALG_HANDLE = 0x00000061, + + BCRYPT_HMAC_MD5_ALG_HANDLE = 0x00000091, + BCRYPT_HMAC_SHA1_ALG_HANDLE = 0x000000A1, + BCRYPT_HMAC_SHA256_ALG_HANDLE = 0x000000B1, + BCRYPT_HMAC_SHA384_ALG_HANDLE = 0x000000C1, + BCRYPT_HMAC_SHA512_ALG_HANDLE = 0x000000D1, + } + } +} diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs new file mode 100644 index 00000000000000..c38596757bebb5 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; + +using Microsoft.Win32.SafeHandles; + +internal partial class Interop +{ + internal partial class BCrypt + { + [DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)] + internal static extern unsafe NTSTATUS BCryptDeriveKeyPBKDF2( + SafeBCryptAlgorithmHandle hPrf, + byte* pbPassword, + int cbPassword, + byte* pbSalt, + int cbSalt, + ulong cIterations, + byte* pbDerivedKey, + int cbDerivedKey, + int dwFlags); + + [DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)] + internal static extern unsafe NTSTATUS BCryptDeriveKeyPBKDF2( + nuint hPrf, + byte* pbPassword, + int cbPassword, + byte* pbSalt, + int cbSalt, + ulong cIterations, + byte* pbDerivedKey, + int cbDerivedKey, + int dwFlags); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptHash.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptHash.cs index e018062b9c7d1c..530e7b2c97dafc 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptHash.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptHash.cs @@ -10,16 +10,5 @@ internal partial class BCrypt { [DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)] internal static unsafe extern NTSTATUS BCryptHash(nuint hAlgorithm, byte* pbSecret, int cbSecret, byte* pbInput, int cbInput, byte* pbOutput, int cbOutput); - - // Pseudo-handles, as defined in bcrypt.h - // TODO: This really should be backed by 'nuint' (see https://github.com/dotnet/roslyn/issues/44110) - public enum BCryptAlgPseudoHandle : uint - { - BCRYPT_MD5_ALG_HANDLE = 0x00000021, - BCRYPT_SHA1_ALG_HANDLE = 0x00000031, - BCRYPT_SHA256_ALG_HANDLE = 0x00000041, - BCRYPT_SHA384_ALG_HANDLE = 0x00000051, - BCRYPT_SHA512_ALG_HANDLE = 0x00000061, - } } } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/CMakeLists.txt b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/CMakeLists.txt index e1e4c47a6d861f..a22275331045cb 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/CMakeLists.txt +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/CMakeLists.txt @@ -10,6 +10,7 @@ set(NATIVECRYPTO_SOURCES pal_ecc.c pal_hmac.c pal_keyagree.c + pal_keyderivation.c pal_keychain.c pal_random.c pal_rsa.c diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c index 3c00bb540e2ce4..6aec84c2c2a436 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c @@ -103,6 +103,7 @@ static const Entry s_cryptoAppleNative[] = DllImportEntry(AppleCryptoNative_X509ChainGetStatusAtIndex) DllImportEntry(AppleCryptoNative_GetOSStatusForChainStatus) DllImportEntry(AppleCryptoNative_X509ChainSetTrustAnchorCertificates) + DllImportEntry(AppleCryptoNative_KeyDerivationPBKDF) }; EXTERN_C const void* CryptoAppleResolveDllImport(const char* name); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c new file mode 100644 index 00000000000000..38a53705c88cf4 --- /dev/null +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c @@ -0,0 +1,64 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "pal_keyderivation.h" + +#if !defined(TARGET_IOS) && !defined(TARGET_TVOS) + +static int32_t PrfAlgorithmFromHashAlgorithm(PAL_HashAlgorithm hashAlgorithm, CCPseudoRandomAlgorithm* algorithm) +{ + if (algorithm == NULL) + return 0; + + switch (hashAlgorithm) + { + case PAL_SHA1: + *algorithm = kCCPRFHmacAlgSHA1; + return 1; + case PAL_SHA256: + *algorithm = kCCPRFHmacAlgSHA256; + return 1; + case PAL_SHA384: + *algorithm = kCCPRFHmacAlgSHA384; + return 1; + case PAL_SHA512: + *algorithm = kCCPRFHmacAlgSHA512; + return 1; + default: + *algorithm = 0; + return 0; + } +} + +int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, + const char* password, + int32_t passwordLen, + const uint8_t* salt, + int32_t saltLen, + int32_t iterations, + uint8_t* derivedKey, + uint32_t derivedKeyLen, + int32_t* errorCode) +{ + if (errorCode != NULL) + *errorCode = noErr; + + if (password == NULL || passwordLen < 0 || salt == NULL || saltLen < 0 || + iterations < 0 || derivedKey == NULL || derivedKeyLen < 0 || errorCode == NULL) + { + return -1; + } + + CCPseudoRandomAlgorithm prf; + + if (!PrfAlgorithmFromHashAlgorithm(prfAlgorithm, &prf)) + { + return -2; + } + + CCStatus result = CCKeyDerivationPBKDF(kCCPBKDF2, password, passwordLen, salt, + saltLen, prf, iterations, derivedKey, derivedKeyLen); + *errorCode = result; + return result == kCCSuccess ? 1 : 0; +} +#endif diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h new file mode 100644 index 00000000000000..c707b05788b438 --- /dev/null +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#pragma once + +#include "pal_digest.h" +#include + +#if !defined(TARGET_IOS) && !defined(TARGET_TVOS) +PALEXPORT int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, + const char* password, + int32_t passwordLen, + const uint8_t* salt, + int32_t saltLen, + int32_t iterations, + uint8_t* derivedKey, + uint32_t derivedKeyLen, + int32_t* errorCode); +#endif diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index 9856a0b10f1f05..455077551454e7 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -214,6 +214,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_PemReadX509FromBio) DllImportEntry(CryptoNative_PemReadX509FromBioAux) DllImportEntry(CryptoNative_PemWriteBioX509Crl) + DllImportEntry(CryptoNative_Pkcs5Pbkdf2Hmac) DllImportEntry(CryptoNative_Pkcs7CreateCertificateCollection) DllImportEntry(CryptoNative_Pkcs7Destroy) DllImportEntry(CryptoNative_PushX509StackField) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 9e6fa85a4949e3..5490d1a83666ce 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -437,6 +437,7 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi REQUIRED_FUNCTION(PEM_read_bio_X509_AUX) \ REQUIRED_FUNCTION(PEM_read_bio_X509_CRL) \ REQUIRED_FUNCTION(PEM_write_bio_X509_CRL) \ + REQUIRED_FUNCTION(PKCS5_PBKDF2_HMAC) \ REQUIRED_FUNCTION(PKCS12_free) \ REQUIRED_FUNCTION(PKCS12_parse) \ REQUIRED_FUNCTION(PKCS7_sign) \ @@ -847,6 +848,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define PEM_read_bio_X509_AUX PEM_read_bio_X509_AUX_ptr #define PEM_read_bio_X509_CRL PEM_read_bio_X509_CRL_ptr #define PEM_write_bio_X509_CRL PEM_write_bio_X509_CRL_ptr +#define PKCS5_PBKDF2_HMAC PKCS5_PBKDF2_HMAC_ptr #define PKCS12_free PKCS12_free_ptr #define PKCS12_parse PKCS12_parse_ptr #define PKCS7_sign PKCS7_sign_ptr diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c index 4e67c450e9e888..fd622b78232a6a 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c @@ -151,3 +151,22 @@ int32_t CryptoNative_GetMaxMdSize() { return EVP_MAX_MD_SIZE; } + +int32_t CryptoNative_Pkcs5Pbkdf2Hmac(const char* password, + int32_t passwordLength, + const unsigned char* salt, + int32_t saltLength, + int32_t iterations, + const EVP_MD* digest, + unsigned char* destination, + int32_t destinationLength) +{ + if (password == NULL || passwordLength < 0 || salt == NULL || saltLength < 0 || + iterations <= 0 || digest == NULL || destination == NULL || destinationLength < 0) + { + return -1; + } + + return PKCS5_PBKDF2_HMAC( + password, passwordLength, salt, saltLength, iterations, digest, destinationLength, destination); +} diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h index 3a546c9f92322c..0eb067bff72eb2 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h @@ -120,3 +120,12 @@ GetMaxMdSize Returns the maxium bytes for a message digest. */ PALEXPORT int32_t CryptoNative_GetMaxMdSize(void); + +PALEXPORT int32_t CryptoNative_Pkcs5Pbkdf2Hmac(const char* password, + int32_t passwordLength, + const unsigned char* salt, + int32_t saltLength, + int32_t iterations, + const EVP_MD* digest, + unsigned char* destination, + int32_t destinationLength); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs b/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs index 8e9d12de19a4e8..e5205d90f3726c 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs @@ -587,9 +587,14 @@ public Rfc2898DeriveBytes(string password, int saltSize, int iterations, System. public byte[] CryptDeriveKey(string algname, string alghashname, int keySize, byte[] rgbIV) { throw null; } protected override void Dispose(bool disposing) { } public override byte[] GetBytes(int cb) { throw null; } + public static byte[] Pbkdf2DeriveBytes(byte[] password, byte[] salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } + public static byte[] Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } + public static void Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, System.Span destination) { } + public static byte[] Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } + public static void Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, System.Span destination) { } + public static byte[] Pbkdf2DeriveBytes(string password, byte[] salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } public override void Reset() { } } - [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public abstract partial class Rijndael : System.Security.Cryptography.SymmetricAlgorithm { diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.Unix.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.Unix.cs index a67223334986f6..21647eca04f53c 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.Unix.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.Unix.cs @@ -29,7 +29,7 @@ public static HashProvider CreateMacProvider(string hashAlgorithmId, ReadOnlySpa return new HmacHashProvider(evpType, key); } - private static IntPtr HashAlgorithmToEvp(string hashAlgorithmId) => hashAlgorithmId switch { + public static IntPtr HashAlgorithmToEvp(string hashAlgorithmId) => hashAlgorithmId switch { HashAlgorithmNames.SHA1 => s_evpSha1 == IntPtr.Zero ? (s_evpSha1 = Interop.Crypto.EvpSha1()) : s_evpSha1, HashAlgorithmNames.SHA256 => s_evpSha256 == IntPtr.Zero ? (s_evpSha256 = Interop.Crypto.EvpSha256()) : s_evpSha256, HashAlgorithmNames.SHA384 => s_evpSha384 == IntPtr.Zero ? (s_evpSha384 = Interop.Crypto.EvpSha384()) : s_evpSha384, diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs new file mode 100644 index 00000000000000..13394b662468bd --- /dev/null +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Security.Cryptography; +using PAL_HashAlgorithm = Interop.AppleCrypto.PAL_HashAlgorithm; + +namespace Internal.Cryptography +{ + internal partial class Pbkdf2Implementation + { + public static unsafe void Fill( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + string hashAlgorithmName, + Span destination) + { + PAL_HashAlgorithm prfAlgorithm = hashAlgorithmName switch { + "SHA1" => PAL_HashAlgorithm.Sha1, + "SHA256" => PAL_HashAlgorithm.Sha256, + "SHA384" => PAL_HashAlgorithm.Sha384, + "SHA512" => PAL_HashAlgorithm.Sha512, + _ => throw new CryptographicException() // Should have been validated before getting here. + }; + + Interop.AppleCrypto.KeyDerivationPBKDF(prfAlgorithm, password, salt, iterations, destination); + } + } +} diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs new file mode 100644 index 00000000000000..0e34ff910690fb --- /dev/null +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Security.Cryptography; + +namespace Internal.Cryptography +{ + internal partial class Pbkdf2Implementation + { + public static unsafe void Fill( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + string hashAlgorithmName, + Span destination) + { + IntPtr evpHashType = HashProviderDispenser.HashAlgorithmToEvp(hashAlgorithmName); + int result = Interop.Crypto.Pkcs5Pbkdf2Hmac(password, salt, iterations, evpHashType, destination); + const int Success = 1; + + if (result != Success) + { + Debug.Assert(result == 0, $"Unexpected result {result}"); + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + } + } +} diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs new file mode 100644 index 00000000000000..c6fa126d4de264 --- /dev/null +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -0,0 +1,51 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Security.Cryptography; +using Microsoft.Win32.SafeHandles; +using BCryptOpenAlgorithmProviderFlags = Interop.BCrypt.BCryptOpenAlgorithmProviderFlags; +using NTSTATUS = Interop.BCrypt.NTSTATUS; + +namespace Internal.Cryptography +{ + internal partial class Pbkdf2Implementation + { + public static unsafe void Fill( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + string hashAlgorithmName, + Span destination) + { + Debug.Assert(iterations >= 0); + const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; + + // Do not dispose handle since it is shared and cached. + SafeBCryptAlgorithmHandle handle = + Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName, OpenAlgorithmFlags, out _); + + fixed (byte* pPassword = password) + fixed (byte* pSalt = salt) + fixed (byte* pDestination = destination) + { + NTSTATUS ntStatus = Interop.BCrypt.BCryptDeriveKeyPBKDF2( + handle, + pPassword, + password.Length, + pSalt, + salt.Length, + (ulong)iterations, + pDestination, + destination.Length, + dwFlags: 0); + + if (ntStatus != NTSTATUS.STATUS_SUCCESS) + { + throw Interop.BCrypt.CreateCryptographicException(ntStatus); + } + } + } + } +} diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx index a0008c802c9915..8d35ae31c3d5a3 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx @@ -90,6 +90,15 @@ An encrypted key was found, but no password was provided. Use ImportFromEncryptedPem to import this key. + + The specified salt is too short. The minimum salt size is {0} bytes. + + + The specified number of iterations is too small. The minimum is {0}. + + + The specified number of bytes to extract is too small. The minimum is {0}. + Error occurred during a cryptographic operation. diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj b/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj index 4bc44931c4420c..eb1f604843e9fd 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj @@ -77,6 +77,7 @@ + @@ -292,6 +293,7 @@ + + + + @@ -539,6 +546,8 @@ Link="Common\Interop\OSX\System.Security.Cryptography.Native.Apple\Interop.SecKeyRef.Export.cs" /> + + diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs new file mode 100644 index 00000000000000..dad709f3f22747 --- /dev/null +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -0,0 +1,134 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text; +using Internal.Cryptography; + +namespace System.Security.Cryptography +{ + public partial class Rfc2898DeriveBytes + { + private const int OneShotMinIterations = 1000; + private const int OneShotMinSaltLength = 128 / 8; // 128-bits + private const int OneShotMinExtractLength = 112 / 8; // 114-bits + + public static byte[] Pbkdf2DeriveBytes( + byte[] password, + byte[] salt, + int iterations, + int length, + HashAlgorithmName hashAlgorithm) + { + if (password is null) + throw new ArgumentNullException(nameof(password)); + if (salt is null) + throw new ArgumentNullException(nameof(salt)); + + return Pbkdf2DeriveBytes(new ReadOnlySpan(password), new ReadOnlySpan(salt), iterations, length, hashAlgorithm); + } + + public static byte[] Pbkdf2DeriveBytes( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + int length, + HashAlgorithmName hashAlgorithm) + { + if (length < OneShotMinExtractLength) + throw new ArgumentOutOfRangeException(nameof(length), SR.Format(SR.Argument_Rfc2898_MinLength, OneShotMinExtractLength)); + + byte[] result = new byte[length]; + Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithm, result); + return result; + } + + public static void Pbkdf2DeriveBytes( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + HashAlgorithmName hashAlgorithm, + Span destination) + { + Pbkdf2DeriveBytesCore(password, salt, iterations, hashAlgorithm, destination); + } + + public static byte[] Pbkdf2DeriveBytes( + string password, + byte[] salt, + int iterations, + int length, + HashAlgorithmName hashAlgorithm) + { + if (password is null) + throw new ArgumentNullException(nameof(password)); + if (salt is null) + throw new ArgumentNullException(nameof(salt)); + + return Pbkdf2DeriveBytes(password.AsSpan(), new ReadOnlySpan(salt), iterations, length, hashAlgorithm); + } + + public static byte[] Pbkdf2DeriveBytes( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + int length, + HashAlgorithmName hashAlgorithm) + { + if (length < OneShotMinExtractLength) + throw new ArgumentOutOfRangeException(nameof(length), SR.Format(SR.Argument_Rfc2898_MinLength, OneShotMinExtractLength)); + + byte[] result = new byte[length]; + Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithm, result); + return result; + } + + public static void Pbkdf2DeriveBytes( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + HashAlgorithmName hashAlgorithm, + Span destination) + { + byte[] passwordBytes = CryptoPool.Rent(Encoding.UTF8.GetMaxByteCount(password.Length)); + int passwordBytesWritten = Encoding.UTF8.GetBytes(password, passwordBytes); + + Pbkdf2DeriveBytesCore(passwordBytes, salt, iterations, hashAlgorithm, destination); + CryptoPool.Return(passwordBytes, clearSize: passwordBytesWritten); + } + + private static void Pbkdf2DeriveBytesCore( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + HashAlgorithmName hashAlgorithm, + Span destination) + { + if (salt.Length < OneShotMinSaltLength) + throw new ArgumentOutOfRangeException(nameof(salt), SR.Format(SR.Argument_Rfc2898_SaltTooShort, OneShotMinSaltLength)); + if (iterations < OneShotMinIterations) + throw new ArgumentOutOfRangeException(nameof(iterations), SR.Format(SR.Argument_Rfc2898_MinIterations, OneShotMinIterations)); + if (destination.Length < OneShotMinExtractLength) + throw new ArgumentOutOfRangeException(nameof(destination), SR.Format(SR.Argument_Rfc2898_MinLength, OneShotMinExtractLength)); + if (string.IsNullOrEmpty(hashAlgorithm.Name)) + throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm)); + + string hashAlgorithmName = hashAlgorithm.Name; + + // MD5 intentionally left out. + if (hashAlgorithmName != HashAlgorithmName.SHA1.Name && + hashAlgorithmName != HashAlgorithmName.SHA256.Name && + hashAlgorithmName != HashAlgorithmName.SHA384.Name && + hashAlgorithmName != HashAlgorithmName.SHA512.Name) + { + throw new CryptographicException(SR.Format(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithmName)); + } + + if (destination.IsEmpty) + { + return; + } + + Pbkdf2Implementation.Fill(password, salt, iterations, hashAlgorithm.Name, destination); + } + } +} diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs index bf833303ecc90b..7c150159dc37a6 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.cs @@ -12,7 +12,7 @@ namespace System.Security.Cryptography { [UnsupportedOSPlatform("browser")] - public class Rfc2898DeriveBytes : DeriveBytes + public partial class Rfc2898DeriveBytes : DeriveBytes { private const int MinimumSaltSize = 8; diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs new file mode 100644 index 00000000000000..f3206b6a28aed5 --- /dev/null +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -0,0 +1,72 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Text; +using Xunit; + +namespace System.Security.Cryptography.DeriveBytesTests +{ + [SkipOnMono("Not supported on Browser", TestPlatforms.Browser)] + public static class Rfc2898OneShotTests + { + private const string Password = "tired"; + + private static readonly byte[] s_passwordBytes = Encoding.UTF8.GetBytes(Password); + private static readonly byte[] s_salt = new byte[] + { + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 + }; + private static readonly int s_extractLength = 14; + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_NullPassword() + { + AssertExtensions.Throws("password", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + password: (byte[])null, s_salt, iterations: 1000, s_extractLength, HashAlgorithmName.SHA256) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_NullSalt() + { + AssertExtensions.Throws("salt", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, salt: (byte[])null, iterations: 1000, s_extractLength, HashAlgorithmName.SHA256) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_TooShort() + { + ArgumentOutOfRangeException ex = AssertExtensions.Throws("salt", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, salt: new byte[0], iterations: 1000, s_extractLength, HashAlgorithmName.SHA256) + ); + + Assert.Contains("16", ex.Message); // Exception should be formatted with the minimum salt length. + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_IterationsTooLow() + { + ArgumentOutOfRangeException ex = AssertExtensions.Throws("iterations", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, s_salt, iterations: 999, s_extractLength, HashAlgorithmName.SHA256) + ); + + Assert.Contains("1000", ex.Message); // Exception should be formatted with the minimum iterations. + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_Success() + { + using Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(s_passwordBytes, s_salt, iterations: 1000, HashAlgorithmName.SHA256); + byte[] key1 = instanceKdf.GetBytes(s_extractLength); + byte[] key2 = Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, s_salt, iterations: 1000, s_extractLength, HashAlgorithmName.SHA256); + Assert.Equal(key1, key2); + } + } +} diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/System.Security.Cryptography.Algorithms.Tests.csproj b/src/libraries/System.Security.Cryptography.Algorithms/tests/System.Security.Cryptography.Algorithms.Tests.csproj index 3c3183281c985a..f48d669b029416 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/System.Security.Cryptography.Algorithms.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/System.Security.Cryptography.Algorithms.Tests.csproj @@ -85,6 +85,7 @@ + From c77b4e7ec6713b857c3668af6fba47114267b5db Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 08:40:25 -0500 Subject: [PATCH 02/29] Use HashAlgorithmName to PAL --- .../src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs | 4 ++-- .../src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs | 5 +++-- .../Internal/Cryptography/Pbkdf2Implementation.Windows.cs | 5 +++-- .../Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs index 13394b662468bd..e34ae4bc0f1fc6 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs @@ -13,10 +13,10 @@ public static unsafe void Fill( ReadOnlySpan password, ReadOnlySpan salt, int iterations, - string hashAlgorithmName, + HashAlgorithmName hashAlgorithmName, Span destination) { - PAL_HashAlgorithm prfAlgorithm = hashAlgorithmName switch { + PAL_HashAlgorithm prfAlgorithm = hashAlgorithmName.Name switch { "SHA1" => PAL_HashAlgorithm.Sha1, "SHA256" => PAL_HashAlgorithm.Sha256, "SHA384" => PAL_HashAlgorithm.Sha384, diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs index 0e34ff910690fb..733a39a7a9047a 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs @@ -13,10 +13,11 @@ public static unsafe void Fill( ReadOnlySpan password, ReadOnlySpan salt, int iterations, - string hashAlgorithmName, + HashAlgorithmName hashAlgorithmName, Span destination) { - IntPtr evpHashType = HashProviderDispenser.HashAlgorithmToEvp(hashAlgorithmName); + Debug.Assert(hashAlgorithmName.Name is not null); + IntPtr evpHashType = HashProviderDispenser.HashAlgorithmToEvp(hashAlgorithmName.Name); int result = Interop.Crypto.Pkcs5Pbkdf2Hmac(password, salt, iterations, evpHashType, destination); const int Success = 1; diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index c6fa126d4de264..2092ce7fc2a8e2 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -16,15 +16,16 @@ public static unsafe void Fill( ReadOnlySpan password, ReadOnlySpan salt, int iterations, - string hashAlgorithmName, + HashAlgorithmName hashAlgorithmName, Span destination) { Debug.Assert(iterations >= 0); + Debug.Assert(hashAlgorithmName.Name is not null); const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; // Do not dispose handle since it is shared and cached. SafeBCryptAlgorithmHandle handle = - Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName, OpenAlgorithmFlags, out _); + Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName.Name, OpenAlgorithmFlags, out _); fixed (byte* pPassword = password) fixed (byte* pSalt = salt) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index dad709f3f22747..2172500d679aa7 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -128,7 +128,7 @@ private static void Pbkdf2DeriveBytesCore( return; } - Pbkdf2Implementation.Fill(password, salt, iterations, hashAlgorithm.Name, destination); + Pbkdf2Implementation.Fill(password, salt, iterations, hashAlgorithm, destination); } } } From 1bf9d93f6936e03ef211ea937397bd0c977c4c14 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 09:12:00 -0500 Subject: [PATCH 03/29] Use psuedo handles if available on the Windows platform. --- .../Windows/BCrypt/Interop.NTSTATUS.cs | 1 + .../Pbkdf2Implementation.Windows.cs | 58 ++++++++++++++++--- .../tests/Rfc2898OneShotTests.cs | 13 +++-- 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs index 051f21938984b8..87b8225ebbe02b 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs @@ -12,6 +12,7 @@ internal enum NTSTATUS : uint STATUS_INVALID_PARAMETER = 0xc000000d, STATUS_NO_MEMORY = 0xc0000017, STATUS_AUTH_TAG_MISMATCH = 0xc000a002, + STATUS_INVALID_HANDLE = 0xc0000008, } } } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index 2092ce7fc2a8e2..650c1e675fbfdb 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Security.Cryptography; using Microsoft.Win32.SafeHandles; +using BCryptAlgPseudoHandle = Interop.BCrypt.BCryptAlgPseudoHandle; using BCryptOpenAlgorithmProviderFlags = Interop.BCrypt.BCryptOpenAlgorithmProviderFlags; using NTSTATUS = Interop.BCrypt.NTSTATUS; @@ -12,6 +13,8 @@ namespace Internal.Cryptography { internal partial class Pbkdf2Implementation { + private static volatile bool s_useCompatOneShot; + public static unsafe void Fill( ReadOnlySpan password, ReadOnlySpan salt, @@ -21,17 +24,47 @@ public static unsafe void Fill( { Debug.Assert(iterations >= 0); Debug.Assert(hashAlgorithmName.Name is not null); - const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; - // Do not dispose handle since it is shared and cached. - SafeBCryptAlgorithmHandle handle = - Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName.Name, OpenAlgorithmFlags, out _); + const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; + NTSTATUS status; fixed (byte* pPassword = password) fixed (byte* pSalt = salt) fixed (byte* pDestination = destination) { - NTSTATUS ntStatus = Interop.BCrypt.BCryptDeriveKeyPBKDF2( + if (!s_useCompatOneShot) + { + BCryptAlgPseudoHandle psuedoHandle = PseudoHandleFromIdentifier(hashAlgorithmName.Name); + status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( + (nuint)psuedoHandle, + pPassword, + password.Length, + pSalt, + salt.Length, + (ulong)iterations, + pDestination, + destination.Length, + dwFlags: 0); + + switch (status) + { + case NTSTATUS.STATUS_SUCCESS: + return; + case NTSTATUS.STATUS_INVALID_HANDLE: + s_useCompatOneShot = true; + break; + default: + throw Interop.BCrypt.CreateCryptographicException(status); + } + } + + Debug.Assert(s_useCompatOneShot); + + // Do not dispose handle since it is shared and cached. + SafeBCryptAlgorithmHandle handle = + Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName.Name, OpenAlgorithmFlags, out _); + + status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( handle, pPassword, password.Length, @@ -42,11 +75,22 @@ public static unsafe void Fill( destination.Length, dwFlags: 0); - if (ntStatus != NTSTATUS.STATUS_SUCCESS) + if (status != NTSTATUS.STATUS_SUCCESS) { - throw Interop.BCrypt.CreateCryptographicException(ntStatus); + throw Interop.BCrypt.CreateCryptographicException(status); } } } + + private static BCryptAlgPseudoHandle PseudoHandleFromIdentifier(string hashAlgorithmId) + { + return hashAlgorithmId switch { + HashAlgorithmNames.SHA1 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA1_ALG_HANDLE, + HashAlgorithmNames.SHA256 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA256_ALG_HANDLE, + HashAlgorithmNames.SHA384 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA384_ALG_HANDLE, + HashAlgorithmNames.SHA512 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA512_ALG_HANDLE, + _ => throw new CryptographicException() + }; + } } } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index f3206b6a28aed5..460235756d7679 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -59,13 +59,18 @@ public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_IterationsTooLow() Assert.Contains("1000", ex.Message); // Exception should be formatted with the minimum iterations. } - [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_Success() + [Theory] + [InlineData(nameof(HashAlgorithmName.SHA1))] + [InlineData(nameof(HashAlgorithmName.SHA256))] + [InlineData(nameof(HashAlgorithmName.SHA384))] + [InlineData(nameof(HashAlgorithmName.SHA512))] + public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_Success(string hashAlgorithm) { - using Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(s_passwordBytes, s_salt, iterations: 1000, HashAlgorithmName.SHA256); + HashAlgorithmName hashAlgorithmName = new HashAlgorithmName(hashAlgorithm); + using Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(s_passwordBytes, s_salt, iterations: 1000, hashAlgorithmName); byte[] key1 = instanceKdf.GetBytes(s_extractLength); byte[] key2 = Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, s_salt, iterations: 1000, s_extractLength, HashAlgorithmName.SHA256); + s_passwordBytes, s_salt, iterations: 1000, s_extractLength, hashAlgorithmName); Assert.Equal(key1, key2); } } From 22ead6477916d3f3a92b7d82b98f3b47ad599668 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 09:54:35 -0500 Subject: [PATCH 04/29] Fix compilation --- .../Unix/System.Security.Cryptography.Native.Apple/entrypoints.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c index 6aec84c2c2a436..82f6f67cb6b72e 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c @@ -20,6 +20,7 @@ #include "pal_trust.h" #include "pal_x509.h" #include "pal_x509chain.h" +#include "pal_keyderivation.h" static const Entry s_cryptoAppleNative[] = { From abe4c36e99f75e77b085a3678a80ce076d4d356f Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 09:56:47 -0500 Subject: [PATCH 05/29] Undo inadvertent change in ref file. --- .../ref/System.Security.Cryptography.Algorithms.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs b/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs index e5205d90f3726c..f173f440dd7939 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs @@ -595,6 +595,7 @@ public static void Pbkdf2DeriveBytes(System.ReadOnlySpan password, System. public static byte[] Pbkdf2DeriveBytes(string password, byte[] salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } public override void Reset() { } } + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public abstract partial class Rijndael : System.Security.Cryptography.SymmetricAlgorithm { From 859112e12c5feaa5bff6b53f7479acb9c30e865e Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 13:02:01 -0500 Subject: [PATCH 06/29] Detect Windows 7 instead of relying on the NTSTATUS. --- .../Windows/BCrypt/Interop.NTSTATUS.cs | 1 - .../Pbkdf2Implementation.Windows.cs | 55 ++++++++----------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs index 87b8225ebbe02b..051f21938984b8 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.NTSTATUS.cs @@ -12,7 +12,6 @@ internal enum NTSTATUS : uint STATUS_INVALID_PARAMETER = 0xc000000d, STATUS_NO_MEMORY = 0xc0000017, STATUS_AUTH_TAG_MISMATCH = 0xc000a002, - STATUS_INVALID_HANDLE = 0xc0000008, } } } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index 650c1e675fbfdb..4daa0cda22d619 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -13,7 +13,9 @@ namespace Internal.Cryptography { internal partial class Pbkdf2Implementation { - private static volatile bool s_useCompatOneShot; + // BCryptDeriveKeyPBKDF2 on Windows 7 will crash the process with an access violation + // when given a pseudo handle, so we can't detect based on the NTSTATUS of the native call. + private static readonly bool s_usePseudoHandles = OperatingSystem.IsWindowsVersionAtLeast(10, 0, 0); public static unsafe void Fill( ReadOnlySpan password, @@ -25,18 +27,17 @@ public static unsafe void Fill( Debug.Assert(iterations >= 0); Debug.Assert(hashAlgorithmName.Name is not null); - const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; NTSTATUS status; fixed (byte* pPassword = password) fixed (byte* pSalt = salt) fixed (byte* pDestination = destination) { - if (!s_useCompatOneShot) + if (s_usePseudoHandles) { - BCryptAlgPseudoHandle psuedoHandle = PseudoHandleFromIdentifier(hashAlgorithmName.Name); + BCryptAlgPseudoHandle pseudoHandle = PseudoHandleFromIdentifier(hashAlgorithmName.Name); status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( - (nuint)psuedoHandle, + (nuint)pseudoHandle, pPassword, password.Length, pSalt, @@ -45,35 +46,25 @@ public static unsafe void Fill( pDestination, destination.Length, dwFlags: 0); - - switch (status) - { - case NTSTATUS.STATUS_SUCCESS: - return; - case NTSTATUS.STATUS_INVALID_HANDLE: - s_useCompatOneShot = true; - break; - default: - throw Interop.BCrypt.CreateCryptographicException(status); - } } + else + { + const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; + // Do not dispose handle since it is shared and cached. + SafeBCryptAlgorithmHandle handle = + Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName.Name, OpenAlgorithmFlags, out _); - Debug.Assert(s_useCompatOneShot); - - // Do not dispose handle since it is shared and cached. - SafeBCryptAlgorithmHandle handle = - Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName.Name, OpenAlgorithmFlags, out _); - - status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( - handle, - pPassword, - password.Length, - pSalt, - salt.Length, - (ulong)iterations, - pDestination, - destination.Length, - dwFlags: 0); + status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( + handle, + pPassword, + password.Length, + pSalt, + salt.Length, + (ulong)iterations, + pDestination, + destination.Length, + dwFlags: 0); + } if (status != NTSTATUS.STATUS_SUCCESS) { From 62d0ec23442bdff21d4ed8fb0bb5b4dc61f2332c Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 16:25:59 -0500 Subject: [PATCH 07/29] Loosen restrictions and add deeper tests --- .../pal_keyderivation.c | 4 +- .../pal_evp.c | 2 +- .../Cryptography/Pbkdf2Implementation.OSX.cs | 3 + .../Cryptography/Pbkdf2Implementation.Unix.cs | 2 + .../Pbkdf2Implementation.Windows.cs | 2 + .../src/Resources/Strings.resx | 9 - .../Rfc2898DeriveBytes.OneShot.cs | 22 +- .../tests/Rfc2898OneShotTests.cs | 210 ++++++++++++++++-- 8 files changed, 206 insertions(+), 48 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c index 38a53705c88cf4..0d743ce046bf52 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c @@ -9,7 +9,7 @@ static int32_t PrfAlgorithmFromHashAlgorithm(PAL_HashAlgorithm hashAlgorithm, CC { if (algorithm == NULL) return 0; - + switch (hashAlgorithm) { case PAL_SHA1: @@ -43,7 +43,7 @@ int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, if (errorCode != NULL) *errorCode = noErr; - if (password == NULL || passwordLen < 0 || salt == NULL || saltLen < 0 || + if (passwordLen < 0 || salt == NULL || saltLen < 0 || iterations < 0 || derivedKey == NULL || derivedKeyLen < 0 || errorCode == NULL) { return -1; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c index fd622b78232a6a..3c831fd675b992 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c @@ -161,7 +161,7 @@ int32_t CryptoNative_Pkcs5Pbkdf2Hmac(const char* password, unsigned char* destination, int32_t destinationLength) { - if (password == NULL || passwordLength < 0 || salt == NULL || saltLength < 0 || + if (passwordLength < 0 || salt == NULL || saltLength < 0 || iterations <= 0 || digest == NULL || destination == NULL || destinationLength < 0) { return -1; diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs index e34ae4bc0f1fc6..a944a77401b08b 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs @@ -16,6 +16,9 @@ public static unsafe void Fill( HashAlgorithmName hashAlgorithmName, Span destination) { + Debug.Assert(!salt.IsEmpty); + Debug.Assert(!destination.IsEmpty); + PAL_HashAlgorithm prfAlgorithm = hashAlgorithmName.Name switch { "SHA1" => PAL_HashAlgorithm.Sha1, "SHA256" => PAL_HashAlgorithm.Sha256, diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs index 733a39a7a9047a..7bb6b6b493a2cb 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs @@ -16,6 +16,8 @@ public static unsafe void Fill( HashAlgorithmName hashAlgorithmName, Span destination) { + Debug.Assert(!salt.IsEmpty); + Debug.Assert(!destination.IsEmpty); Debug.Assert(hashAlgorithmName.Name is not null); IntPtr evpHashType = HashProviderDispenser.HashAlgorithmToEvp(hashAlgorithmName.Name); int result = Interop.Crypto.Pkcs5Pbkdf2Hmac(password, salt, iterations, evpHashType, destination); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index 4daa0cda22d619..a5c2cdac733b7e 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -24,6 +24,8 @@ public static unsafe void Fill( HashAlgorithmName hashAlgorithmName, Span destination) { + Debug.Assert(!salt.IsEmpty); + Debug.Assert(!destination.IsEmpty); Debug.Assert(iterations >= 0); Debug.Assert(hashAlgorithmName.Name is not null); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx index 8d35ae31c3d5a3..a0008c802c9915 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx @@ -90,15 +90,6 @@ An encrypted key was found, but no password was provided. Use ImportFromEncryptedPem to import this key. - - The specified salt is too short. The minimum salt size is {0} bytes. - - - The specified number of iterations is too small. The minimum is {0}. - - - The specified number of bytes to extract is too small. The minimum is {0}. - Error occurred during a cryptographic operation. diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 2172500d679aa7..0826b522a97d52 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -8,10 +8,6 @@ namespace System.Security.Cryptography { public partial class Rfc2898DeriveBytes { - private const int OneShotMinIterations = 1000; - private const int OneShotMinSaltLength = 128 / 8; // 128-bits - private const int OneShotMinExtractLength = 112 / 8; // 114-bits - public static byte[] Pbkdf2DeriveBytes( byte[] password, byte[] salt, @@ -34,8 +30,8 @@ public static byte[] Pbkdf2DeriveBytes( int length, HashAlgorithmName hashAlgorithm) { - if (length < OneShotMinExtractLength) - throw new ArgumentOutOfRangeException(nameof(length), SR.Format(SR.Argument_Rfc2898_MinLength, OneShotMinExtractLength)); + if (length < 0) + throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum); byte[] result = new byte[length]; Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithm, result); @@ -74,8 +70,8 @@ public static byte[] Pbkdf2DeriveBytes( int length, HashAlgorithmName hashAlgorithm) { - if (length < OneShotMinExtractLength) - throw new ArgumentOutOfRangeException(nameof(length), SR.Format(SR.Argument_Rfc2898_MinLength, OneShotMinExtractLength)); + if (length < 0) + throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum); byte[] result = new byte[length]; Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithm, result); @@ -103,12 +99,10 @@ private static void Pbkdf2DeriveBytesCore( HashAlgorithmName hashAlgorithm, Span destination) { - if (salt.Length < OneShotMinSaltLength) - throw new ArgumentOutOfRangeException(nameof(salt), SR.Format(SR.Argument_Rfc2898_SaltTooShort, OneShotMinSaltLength)); - if (iterations < OneShotMinIterations) - throw new ArgumentOutOfRangeException(nameof(iterations), SR.Format(SR.Argument_Rfc2898_MinIterations, OneShotMinIterations)); - if (destination.Length < OneShotMinExtractLength) - throw new ArgumentOutOfRangeException(nameof(destination), SR.Format(SR.Argument_Rfc2898_MinLength, OneShotMinExtractLength)); + if (salt.Length < MinimumSaltSize) + throw new ArgumentOutOfRangeException(nameof(salt), SR.Cryptography_PasswordDerivedBytes_FewBytesSalt); + if (iterations <= 0) + throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); if (string.IsNullOrEmpty(hashAlgorithm.Name)) throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm)); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index 460235756d7679..652e4a0705e6ff 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Text; using Xunit; +using Test.Cryptography; namespace System.Security.Cryptography.DeriveBytesTests { @@ -13,10 +14,7 @@ public static class Rfc2898OneShotTests private const string Password = "tired"; private static readonly byte[] s_passwordBytes = Encoding.UTF8.GetBytes(Password); - private static readonly byte[] s_salt = new byte[] - { - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 - }; + private static readonly byte[] s_salt = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }; private static readonly int s_extractLength = 14; [Fact] @@ -24,7 +22,7 @@ public static void Pbkdf2DeriveBytes_PasswordBytes_NullPassword() { AssertExtensions.Throws("password", () => Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - password: (byte[])null, s_salt, iterations: 1000, s_extractLength, HashAlgorithmName.SHA256) + password: (byte[])null, s_salt, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) ); } @@ -33,45 +31,213 @@ public static void Pbkdf2DeriveBytes_PasswordBytes_NullSalt() { AssertExtensions.Throws("salt", () => Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, salt: (byte[])null, iterations: 1000, s_extractLength, HashAlgorithmName.SHA256) + s_passwordBytes, salt: (byte[])null, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) ); } [Fact] public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_TooShort() { - ArgumentOutOfRangeException ex = AssertExtensions.Throws("salt", () => + AssertExtensions.Throws("salt", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA256) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_IterationsNegative() + { + AssertExtensions.Throws("iterations", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, s_salt, iterations: -1, s_extractLength, HashAlgorithmName.SHA256) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_BogusHash() + { + Assert.Throws(() => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("BLAH")) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_NullHashName() + { + AssertExtensions.Throws("hashAlgorithm", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, s_salt, iterations: 1, s_extractLength, default(HashAlgorithmName)) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordBytes_EmptyHashName() + { + AssertExtensions.Throws("hashAlgorithm", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + s_passwordBytes, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("")) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordString_NullPassword() + { + AssertExtensions.Throws("password", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + password: (string)null, s_salt, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordString_NullSalt() + { + AssertExtensions.Throws("salt", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + Password, salt: null, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordString_SaltBytes_TooShort() + { + AssertExtensions.Throws("salt", () => Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, salt: new byte[0], iterations: 1000, s_extractLength, HashAlgorithmName.SHA256) + Password, salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA256) ); + } - Assert.Contains("16", ex.Message); // Exception should be formatted with the minimum salt length. + [Fact] + public static void Pbkdf2DeriveBytes_PasswordString_SaltBytes_IterationsNegative() + { + AssertExtensions.Throws("iterations", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + Password, s_salt, iterations: -1, s_extractLength, HashAlgorithmName.SHA256) + ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_IterationsTooLow() + public static void Pbkdf2DeriveBytes_PasswordString_BogusHash() { - ArgumentOutOfRangeException ex = AssertExtensions.Throws("iterations", () => + Assert.Throws(() => Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, s_salt, iterations: 999, s_extractLength, HashAlgorithmName.SHA256) + Password, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("BLAH")) ); + } - Assert.Contains("1000", ex.Message); // Exception should be formatted with the minimum iterations. + [Fact] + public static void Pbkdf2DeriveBytes_PasswordString_NullHashName() + { + AssertExtensions.Throws("hashAlgorithm", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + Password, s_salt, iterations: 1, s_extractLength, default(HashAlgorithmName)) + ); + } + + [Fact] + public static void Pbkdf2DeriveBytes_PasswordString_EmptyHashName() + { + AssertExtensions.Throws("hashAlgorithm", () => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + Password, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("")) + ); } [Theory] - [InlineData(nameof(HashAlgorithmName.SHA1))] - [InlineData(nameof(HashAlgorithmName.SHA256))] - [InlineData(nameof(HashAlgorithmName.SHA384))] - [InlineData(nameof(HashAlgorithmName.SHA512))] - public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_Success(string hashAlgorithm) + [MemberData(nameof(Pbkdf2DeriveBytes_PasswordBytes_Compare_Data))] + public static void Pbkdf2DeriveBytes_PasswordBytes_Compare( + string hashAlgorithm, + int length, + int iterations, + string passwordHex, + string saltHex) { + byte[] password = Convert.FromHexString(passwordHex); + byte[] salt = Convert.FromHexString(saltHex); HashAlgorithmName hashAlgorithmName = new HashAlgorithmName(hashAlgorithm); - using Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(s_passwordBytes, s_salt, iterations: 1000, hashAlgorithmName); - byte[] key1 = instanceKdf.GetBytes(s_extractLength); - byte[] key2 = Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, s_salt, iterations: 1000, s_extractLength, hashAlgorithmName); + byte[] key1; + + using (Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(password, salt, iterations, hashAlgorithmName)) + { + key1 = instanceKdf.GetBytes(length); + } + + // byte array allocating + byte[] key2 = Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, length, hashAlgorithmName); + Assert.Equal(key1, key2); + + Span destinationBuffer = new byte[length + 2]; + Span destination = destinationBuffer.Slice(1, length); + Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithmName, destination); + + Assert.True(key1.AsSpan().SequenceEqual(destination), "key1 == destination"); + Assert.Equal(0, destinationBuffer[^1]); // Make sure we didn't write past the destination + Assert.Equal(0, destinationBuffer[0]); // Make sure we didn't write before the destination + } + + [Theory] + [MemberData(nameof(Pbkdf2DeriveBytes_PasswordString_Compare_Data))] + public static void Pbkdf2DeriveBytes_PasswordString_Compare( + string hashAlgorithm, + int length, + int iterations, + string password, + string saltHex) + { + byte[] salt = Convert.FromHexString(saltHex); + HashAlgorithmName hashAlgorithmName = new HashAlgorithmName(hashAlgorithm); + byte[] key1; + + using (Rfc2898DeriveBytes instanceKdf = new Rfc2898DeriveBytes(password, salt, iterations, hashAlgorithmName)) + { + key1 = instanceKdf.GetBytes(length); + } + + // byte array allocating + byte[] key2 = Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, length, hashAlgorithmName); Assert.Equal(key1, key2); + + Span destinationBuffer = new byte[length + 2]; + Span destination = destinationBuffer.Slice(1, length); + Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithmName, destination); + + Assert.True(key1.AsSpan().SequenceEqual(destination), "key1 == destination"); + Assert.Equal(0, destinationBuffer[^1]); // Make sure we didn't write past the destination + Assert.Equal(0, destinationBuffer[0]); // Make sure we didn't write before the destination + } + + public static IEnumerable Pbkdf2DeriveBytes_PasswordBytes_Compare_Data() + { + foreach (HashAlgorithmName hashAlgorithm in SupportedHashAlgorithms) + { + // hashAlgorithm, length, iterations, passwordHex, saltHex + yield return new object[] { hashAlgorithm.Name, 1, 1, s_passwordBytes.ByteArrayToHex(), s_salt.ByteArrayToHex() }; + yield return new object[] { hashAlgorithm.Name, 1, 1, "", s_salt.ByteArrayToHex() }; + yield return new object[] { hashAlgorithm.Name, 257, 257, "", s_salt.ByteArrayToHex() }; + yield return new object[] { hashAlgorithm.Name, 257, 257, "D8D8D8D8D8D8D8D8", "D8D8D8D8D8D8D8D8" }; + yield return new object[] { hashAlgorithm.Name, 257, 257, "0000000000000000", "0000000000000000" }; + } } + + public static IEnumerable Pbkdf2DeriveBytes_PasswordString_Compare_Data() + { + foreach (HashAlgorithmName hashAlgorithm in SupportedHashAlgorithms) + { + // hashAlgorithm, length, iterations, passwordHex, saltHex + yield return new object[] { hashAlgorithm.Name, 1, 1, Password, s_salt.ByteArrayToHex() }; + yield return new object[] { hashAlgorithm.Name, 1, 1, "", s_salt.ByteArrayToHex() }; + yield return new object[] { hashAlgorithm.Name, 257, 257, "", s_salt.ByteArrayToHex() }; + yield return new object[] { hashAlgorithm.Name, 257, 257, Password, "D8D8D8D8D8D8D8D8" }; + yield return new object[] { hashAlgorithm.Name, 257, 257, Password, "0000000000000000" }; + } + } + + private static HashAlgorithmName[] SupportedHashAlgorithms => new [] + { + HashAlgorithmName.SHA1, + HashAlgorithmName.SHA256, + HashAlgorithmName.SHA384, + HashAlgorithmName.SHA512 + }; } } From d8e5c2d380d191cbc98f7d0a3c6ca1208814be7c Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 17:06:15 -0500 Subject: [PATCH 08/29] Fix empty password on macOS. --- .../pal_keyderivation.c | 9 +++++++++ .../Internal/Cryptography/Pbkdf2Implementation.OSX.cs | 1 + 2 files changed, 10 insertions(+) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c index 0d743ce046bf52..b036fd902b3593 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c @@ -49,6 +49,15 @@ int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, return -1; } + char empty = 0; + + if (passwordLen == 0) + { + // macOS will not accept a null password, but it will accept a zero-length + // password with a valid pointer. + password = ∅ + } + CCPseudoRandomAlgorithm prf; if (!PrfAlgorithmFromHashAlgorithm(prfAlgorithm, &prf)) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs index a944a77401b08b..c228ed049afc13 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; using System.Security.Cryptography; using PAL_HashAlgorithm = Interop.AppleCrypto.PAL_HashAlgorithm; From 5ef000d6811507e386fa1f4ebe7cd560a9cee4e8 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 10 Feb 2021 21:14:52 -0500 Subject: [PATCH 09/29] Permit empty salts --- .../pal_keyderivation.c | 4 ++-- .../pal_evp.c | 4 ++-- .../Cryptography/Pbkdf2Implementation.OSX.cs | 1 - .../Cryptography/Pbkdf2Implementation.Unix.cs | 1 - .../Pbkdf2Implementation.Windows.cs | 1 - .../Rfc2898DeriveBytes.OneShot.cs | 2 -- .../tests/Rfc2898OneShotTests.cs | 20 +++++++++---------- 7 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c index b036fd902b3593..b920e8721dd4e1 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c @@ -43,8 +43,8 @@ int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, if (errorCode != NULL) *errorCode = noErr; - if (passwordLen < 0 || salt == NULL || saltLen < 0 || - iterations < 0 || derivedKey == NULL || derivedKeyLen < 0 || errorCode == NULL) + if (passwordLen < 0 || saltLen < 0 || iterations < 0 || derivedKey == NULL || + derivedKeyLen < 0 || errorCode == NULL) { return -1; } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c index 3c831fd675b992..e2245670001d83 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c @@ -161,8 +161,8 @@ int32_t CryptoNative_Pkcs5Pbkdf2Hmac(const char* password, unsigned char* destination, int32_t destinationLength) { - if (passwordLength < 0 || salt == NULL || saltLength < 0 || - iterations <= 0 || digest == NULL || destination == NULL || destinationLength < 0) + if (passwordLength < 0 || saltLength < 0 || iterations <= 0 || digest == NULL || + destination == NULL || destinationLength < 0) { return -1; } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs index c228ed049afc13..289afa5bf5b487 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs @@ -17,7 +17,6 @@ public static unsafe void Fill( HashAlgorithmName hashAlgorithmName, Span destination) { - Debug.Assert(!salt.IsEmpty); Debug.Assert(!destination.IsEmpty); PAL_HashAlgorithm prfAlgorithm = hashAlgorithmName.Name switch { diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs index 7bb6b6b493a2cb..a68d4f22f12627 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs @@ -16,7 +16,6 @@ public static unsafe void Fill( HashAlgorithmName hashAlgorithmName, Span destination) { - Debug.Assert(!salt.IsEmpty); Debug.Assert(!destination.IsEmpty); Debug.Assert(hashAlgorithmName.Name is not null); IntPtr evpHashType = HashProviderDispenser.HashAlgorithmToEvp(hashAlgorithmName.Name); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index a5c2cdac733b7e..f7e4f5e88897db 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -24,7 +24,6 @@ public static unsafe void Fill( HashAlgorithmName hashAlgorithmName, Span destination) { - Debug.Assert(!salt.IsEmpty); Debug.Assert(!destination.IsEmpty); Debug.Assert(iterations >= 0); Debug.Assert(hashAlgorithmName.Name is not null); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 0826b522a97d52..0cc32299308021 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -99,8 +99,6 @@ private static void Pbkdf2DeriveBytesCore( HashAlgorithmName hashAlgorithm, Span destination) { - if (salt.Length < MinimumSaltSize) - throw new ArgumentOutOfRangeException(nameof(salt), SR.Cryptography_PasswordDerivedBytes_FewBytesSalt); if (iterations <= 0) throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); if (string.IsNullOrEmpty(hashAlgorithm.Name)) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index 652e4a0705e6ff..9c9aae0ef87bba 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -36,12 +36,12 @@ public static void Pbkdf2DeriveBytes_PasswordBytes_NullSalt() } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_TooShort() + public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_SaltEmpty() { - AssertExtensions.Throws("salt", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA256) - ); + byte[] expectedKey = "1E437A1C79D75BE61E91141DAE20".HexToByteArray(); + byte[] key = Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + new byte[0], salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA1); + Assert.Equal(expectedKey, key); } [Fact] @@ -99,12 +99,12 @@ public static void Pbkdf2DeriveBytes_PasswordString_NullSalt() } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_SaltBytes_TooShort() + public static void Pbkdf2DeriveBytes_PasswordString_SaltBytes_SaltEmpty() { - AssertExtensions.Throws("salt", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - Password, salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA256) - ); + byte[] expectedKey = "1E437A1C79D75BE61E91141DAE20".HexToByteArray(); + byte[] key = Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + password: "", salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA1); + Assert.Equal(expectedKey, key); } [Fact] From d1c34a3faa3b56ce0893a03be65389cca2a389dd Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 11 Feb 2021 10:59:44 -0500 Subject: [PATCH 10/29] Fix OpenSSL 1.0 with empty values --- .../pal_keyderivation.c | 7 ++++--- .../System.Security.Cryptography.Native/pal_evp.c | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c index b920e8721dd4e1..763a6fe273f49a 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c @@ -49,13 +49,14 @@ int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, return -1; } - char empty = 0; + const char* empty = ""; - if (passwordLen == 0) + if (password == NULL) { // macOS will not accept a null password, but it will accept a zero-length // password with a valid pointer. - password = ∅ + password = empty; + passwordLen = 0; } CCPseudoRandomAlgorithm prf; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c index e2245670001d83..61524e42ee322d 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c @@ -167,6 +167,20 @@ int32_t CryptoNative_Pkcs5Pbkdf2Hmac(const char* password, return -1; } + const char* empty = ""; + + if (salt == NULL) + { + assert(saltLength == 0); + salt = (const unsigned char*)empty; + } + + if (password == NULL) + { + assert(passwordLength == 0); + password = empty; + } + return PKCS5_PBKDF2_HMAC( password, passwordLength, salt, saltLength, iterations, digest, destinationLength, destination); } From a873c760e8232ffe0f04cc69c3ac44f1b1a3a26d Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 11 Feb 2021 11:10:43 -0500 Subject: [PATCH 11/29] Rename exported function --- .../Interop.EVP.cs | 8 ++++---- .../entrypoints.c | 2 +- .../pal_evp.c | 16 ++++++++-------- .../pal_evp.h | 16 ++++++++-------- .../Cryptography/Pbkdf2Implementation.Unix.cs | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs index 096387f893a7e6..45e3cce6715d94 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs @@ -54,8 +54,8 @@ internal static int EvpDigestUpdate(SafeEvpMdCtxHandle ctx, ReadOnlySpan d [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetMaxMdSize")] private static extern int GetMaxMdSize(); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_Pkcs5Pbkdf2Hmac")] - private static unsafe extern int Pkcs5Pbkdf2Hmac( + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_Pbkdf2")] + private static unsafe extern int Pbkdf2( byte* pPassword, int passwordLength, byte* pSalt, @@ -65,7 +65,7 @@ private static unsafe extern int Pkcs5Pbkdf2Hmac( byte* pDestination, int destinationLength); - internal static unsafe int Pkcs5Pbkdf2Hmac( + internal static unsafe int Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, int iterations, @@ -76,7 +76,7 @@ internal static unsafe int Pkcs5Pbkdf2Hmac( fixed (byte* pSalt = salt) fixed (byte* pDestination = destination) { - return Pkcs5Pbkdf2Hmac( + return Pbkdf2( pPassword, password.Length, pSalt, diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index 455077551454e7..f067c7d532fb7f 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -214,7 +214,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_PemReadX509FromBio) DllImportEntry(CryptoNative_PemReadX509FromBioAux) DllImportEntry(CryptoNative_PemWriteBioX509Crl) - DllImportEntry(CryptoNative_Pkcs5Pbkdf2Hmac) + DllImportEntry(CryptoNative_Pbkdf2) DllImportEntry(CryptoNative_Pkcs7CreateCertificateCollection) DllImportEntry(CryptoNative_Pkcs7Destroy) DllImportEntry(CryptoNative_PushX509StackField) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c index 61524e42ee322d..01fe9fca1e4567 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c @@ -152,14 +152,14 @@ int32_t CryptoNative_GetMaxMdSize() return EVP_MAX_MD_SIZE; } -int32_t CryptoNative_Pkcs5Pbkdf2Hmac(const char* password, - int32_t passwordLength, - const unsigned char* salt, - int32_t saltLength, - int32_t iterations, - const EVP_MD* digest, - unsigned char* destination, - int32_t destinationLength) +int32_t CryptoNative_Pbkdf2(const char* password, + int32_t passwordLength, + const unsigned char* salt, + int32_t saltLength, + int32_t iterations, + const EVP_MD* digest, + unsigned char* destination, + int32_t destinationLength) { if (passwordLength < 0 || saltLength < 0 || iterations <= 0 || digest == NULL || destination == NULL || destinationLength < 0) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h index 0eb067bff72eb2..cffa2e1524808d 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h @@ -121,11 +121,11 @@ Returns the maxium bytes for a message digest. */ PALEXPORT int32_t CryptoNative_GetMaxMdSize(void); -PALEXPORT int32_t CryptoNative_Pkcs5Pbkdf2Hmac(const char* password, - int32_t passwordLength, - const unsigned char* salt, - int32_t saltLength, - int32_t iterations, - const EVP_MD* digest, - unsigned char* destination, - int32_t destinationLength); +PALEXPORT int32_t CryptoNative_Pbkdf2(const char* password, + int32_t passwordLength, + const unsigned char* salt, + int32_t saltLength, + int32_t iterations, + const EVP_MD* digest, + unsigned char* destination, + int32_t destinationLength); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs index a68d4f22f12627..a4a04b1a6887de 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Unix.cs @@ -19,7 +19,7 @@ public static unsafe void Fill( Debug.Assert(!destination.IsEmpty); Debug.Assert(hashAlgorithmName.Name is not null); IntPtr evpHashType = HashProviderDispenser.HashAlgorithmToEvp(hashAlgorithmName.Name); - int result = Interop.Crypto.Pkcs5Pbkdf2Hmac(password, salt, iterations, evpHashType, destination); + int result = Interop.Crypto.Pbkdf2(password, salt, iterations, evpHashType, destination); const int Success = 1; if (result != Success) From eed78a43fdc8aa5a07a8140534edfaba79cd8441 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 11 Feb 2021 11:14:16 -0500 Subject: [PATCH 12/29] Rename Apple exported function --- .../Interop.Pbkdf2.cs | 8 ++++---- .../entrypoints.c | 2 +- .../pal_keyderivation.c | 18 +++++++++--------- .../pal_keyderivation.h | 18 +++++++++--------- .../Cryptography/Pbkdf2Implementation.OSX.cs | 2 +- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs index 33b6ed7e760fce..4eb5bfd57edfae 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Pbkdf2.cs @@ -10,7 +10,7 @@ internal static partial class Interop { internal static partial class AppleCrypto { - internal static unsafe void KeyDerivationPBKDF( + internal static unsafe void Pbkdf2( PAL_HashAlgorithm prfAlgorithm, ReadOnlySpan password, ReadOnlySpan salt, @@ -21,7 +21,7 @@ internal static unsafe void KeyDerivationPBKDF( fixed (byte* pSalt = salt) fixed (byte* pDestination = destination) { - int ret = AppleCryptoNative_KeyDerivationPBKDF( + int ret = AppleCryptoNative_Pbkdf2( prfAlgorithm, pPassword, password.Length, @@ -41,14 +41,14 @@ internal static unsafe void KeyDerivationPBKDF( if (ret != 1) { - Debug.Fail($"KeyDerivationPBKDF failed with invalid input {ret}"); + Debug.Fail($"Pbkdf2 failed with invalid input {ret}"); throw new CryptographicException(); } } } [DllImport(Libraries.AppleCryptoNative)] - private static extern unsafe int AppleCryptoNative_KeyDerivationPBKDF( + private static extern unsafe int AppleCryptoNative_Pbkdf2( PAL_HashAlgorithm prfAlgorithm, byte* password, int passwordLen, diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c index 82f6f67cb6b72e..43f76ca4028236 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/entrypoints.c @@ -104,7 +104,7 @@ static const Entry s_cryptoAppleNative[] = DllImportEntry(AppleCryptoNative_X509ChainGetStatusAtIndex) DllImportEntry(AppleCryptoNative_GetOSStatusForChainStatus) DllImportEntry(AppleCryptoNative_X509ChainSetTrustAnchorCertificates) - DllImportEntry(AppleCryptoNative_KeyDerivationPBKDF) + DllImportEntry(AppleCryptoNative_Pbkdf2) }; EXTERN_C const void* CryptoAppleResolveDllImport(const char* name); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c index 763a6fe273f49a..bc6b47066643bf 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c @@ -30,15 +30,15 @@ static int32_t PrfAlgorithmFromHashAlgorithm(PAL_HashAlgorithm hashAlgorithm, CC } } -int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, - const char* password, - int32_t passwordLen, - const uint8_t* salt, - int32_t saltLen, - int32_t iterations, - uint8_t* derivedKey, - uint32_t derivedKeyLen, - int32_t* errorCode) +int32_t AppleCryptoNative_Pbkdf2(PAL_HashAlgorithm prfAlgorithm, + const char* password, + int32_t passwordLen, + const uint8_t* salt, + int32_t saltLen, + int32_t iterations, + uint8_t* derivedKey, + uint32_t derivedKeyLen, + int32_t* errorCode) { if (errorCode != NULL) *errorCode = noErr; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h index c707b05788b438..998ce1d1bf5d22 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h @@ -7,13 +7,13 @@ #include #if !defined(TARGET_IOS) && !defined(TARGET_TVOS) -PALEXPORT int32_t AppleCryptoNative_KeyDerivationPBKDF(PAL_HashAlgorithm prfAlgorithm, - const char* password, - int32_t passwordLen, - const uint8_t* salt, - int32_t saltLen, - int32_t iterations, - uint8_t* derivedKey, - uint32_t derivedKeyLen, - int32_t* errorCode); +PALEXPORT int32_t AppleCryptoNative_Pbkdf2(PAL_HashAlgorithm prfAlgorithm, + const char* password, + int32_t passwordLen, + const uint8_t* salt, + int32_t saltLen, + int32_t iterations, + uint8_t* derivedKey, + uint32_t derivedKeyLen, + int32_t* errorCode); #endif diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs index 289afa5bf5b487..1a2b5118cf9293 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs @@ -27,7 +27,7 @@ public static unsafe void Fill( _ => throw new CryptographicException() // Should have been validated before getting here. }; - Interop.AppleCrypto.KeyDerivationPBKDF(prfAlgorithm, password, salt, iterations, destination); + Interop.AppleCrypto.Pbkdf2(prfAlgorithm, password, salt, iterations, destination); } } } From 51b77c13c3c8a5522370419d73c89a74f5b51254 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 11 Feb 2021 13:55:33 -0500 Subject: [PATCH 13/29] Cleanup native PALs. Reorder to keep alphabetic consistency. Add comment documentation. Tighten validation between password and salt inputs and their length. NULL input requires a zero length. --- .../pal_keyderivation.c | 11 +++++++++- .../pal_keyderivation.h | 21 +++++++++++++++++++ .../entrypoints.c | 2 +- .../pal_evp.c | 12 +++++++++-- .../pal_evp.h | 16 +++++++++++++- 5 files changed, 57 insertions(+), 5 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c index bc6b47066643bf..6358fe85085cfe 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.c @@ -49,14 +49,23 @@ int32_t AppleCryptoNative_Pbkdf2(PAL_HashAlgorithm prfAlgorithm, return -1; } + if (salt == NULL && saltLen != 0) + { + return -1; + } + const char* empty = ""; if (password == NULL) { + if (passwordLen != 0) + { + return -1; + } + // macOS will not accept a null password, but it will accept a zero-length // password with a valid pointer. password = empty; - passwordLen = 0; } CCPseudoRandomAlgorithm prf; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h index 998ce1d1bf5d22..ee9cfae8dc3d1e 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_keyderivation.h @@ -7,6 +7,27 @@ #include #if !defined(TARGET_IOS) && !defined(TARGET_TVOS) +/* +Filled the derivedKey buffer with PBKDF2 derived data. + +Implemented by: +1) Validating input +2) Calling CCKeyDerivationPBKDF + +password and salt may be NULL if their respective length parameter +is zero. When password is NULL, it will be replaced with a pointer to an empty +location. + +Returns -1 on invalid input, or -2 if the prfAlgorithm is an unknown +or unsupported hash algorithm. On valid input, the return value +is 1 if successful, and 0 if unsuccessful. + +Returns the result of SecKeychainCreate. + +Output: +errorCode: Contains the CCStatus of the operation. This will contain the +error code when the call is unsuccessful with valid input. +*/ PALEXPORT int32_t AppleCryptoNative_Pbkdf2(PAL_HashAlgorithm prfAlgorithm, const char* password, int32_t passwordLen, diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index f067c7d532fb7f..bd379df88242e1 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -209,12 +209,12 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_ObjTxt2Obj) DllImportEntry(CryptoNative_OcspRequestDestroy) DllImportEntry(CryptoNative_OcspResponseDestroy) + DllImportEntry(CryptoNative_Pbkdf2) DllImportEntry(CryptoNative_PemReadBioPkcs7) DllImportEntry(CryptoNative_PemReadBioX509Crl) DllImportEntry(CryptoNative_PemReadX509FromBio) DllImportEntry(CryptoNative_PemReadX509FromBioAux) DllImportEntry(CryptoNative_PemWriteBioX509Crl) - DllImportEntry(CryptoNative_Pbkdf2) DllImportEntry(CryptoNative_Pkcs7CreateCertificateCollection) DllImportEntry(CryptoNative_Pkcs7Destroy) DllImportEntry(CryptoNative_PushX509StackField) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c index 01fe9fca1e4567..a119f8178f0017 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.c @@ -171,13 +171,21 @@ int32_t CryptoNative_Pbkdf2(const char* password, if (salt == NULL) { - assert(saltLength == 0); + if (saltLength != 0) + { + return -1; + } + salt = (const unsigned char*)empty; } if (password == NULL) { - assert(passwordLength == 0); + if (passwordLength != 0) + { + return -1; + } + password = empty; } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h index cffa2e1524808d..11c8a167191815 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp.h @@ -117,10 +117,24 @@ PALEXPORT const EVP_MD* CryptoNative_EvpSha512(void); Function: GetMaxMdSize -Returns the maxium bytes for a message digest. +Returns the maximum bytes for a message digest. */ PALEXPORT int32_t CryptoNative_GetMaxMdSize(void); +/* +Filled the destination buffer with PBKDF2 derived data. + +Implemented by: +1) Validating input +2) Calling PKCS5_PBKDF2_HMAC + +password and salt may be NULL if their respective length parameters +are zero. When null, it will be replaced with a pointer to an empty +location. + +Returns -1 on invalid input. On valid input, the return value +is the return value of PKCS5_PBKDF2_HMAC. +*/ PALEXPORT int32_t CryptoNative_Pbkdf2(const char* password, int32_t passwordLength, const unsigned char* salt, From 02104418a247e21d9ca00878a7148418a3e4fa2d Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 12 Feb 2021 10:56:39 -0500 Subject: [PATCH 14/29] Fix issue with not slicing the rented buffer. While here, use a stack allocation for small password inputs, which seems likely. --- .../Rfc2898DeriveBytes.OneShot.cs | 19 ++++++++++++++++--- .../tests/Rfc2898OneShotTests.cs | 7 ++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 0cc32299308021..409c756d84d43b 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -85,11 +85,24 @@ public static void Pbkdf2DeriveBytes( HashAlgorithmName hashAlgorithm, Span destination) { - byte[] passwordBytes = CryptoPool.Rent(Encoding.UTF8.GetMaxByteCount(password.Length)); - int passwordBytesWritten = Encoding.UTF8.GetBytes(password, passwordBytes); + const int MaxPasswordStackSize = 256; + + byte[]? rentedPasswordBuffer = null; + int maxEncodedSize = Encoding.UTF8.GetMaxByteCount(password.Length); + + Span passwordBuffer = maxEncodedSize > MaxPasswordStackSize ? + (rentedPasswordBuffer = CryptoPool.Rent(maxEncodedSize)) : + stackalloc byte[MaxPasswordStackSize]; + int passwordBytesWritten = Encoding.UTF8.GetBytes(password, passwordBuffer); + Span passwordBytes = passwordBuffer.Slice(0, passwordBytesWritten); Pbkdf2DeriveBytesCore(passwordBytes, salt, iterations, hashAlgorithm, destination); - CryptoPool.Return(passwordBytes, clearSize: passwordBytesWritten); + CryptographicOperations.ZeroMemory(passwordBytes); + + if (rentedPasswordBuffer is not null) + { + CryptoPool.Return(rentedPasswordBuffer, clearSize: 0); // manually cleared above. + } } private static void Pbkdf2DeriveBytesCore( diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index 9c9aae0ef87bba..aab5264638dce4 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -221,14 +221,19 @@ public static IEnumerable Pbkdf2DeriveBytes_PasswordBytes_Compare_Data public static IEnumerable Pbkdf2DeriveBytes_PasswordString_Compare_Data() { + string largePassword = new string('y', 1024); + foreach (HashAlgorithmName hashAlgorithm in SupportedHashAlgorithms) { - // hashAlgorithm, length, iterations, passwordHex, saltHex + // hashAlgorithm, length, iterations, password, saltHex yield return new object[] { hashAlgorithm.Name, 1, 1, Password, s_salt.ByteArrayToHex() }; yield return new object[] { hashAlgorithm.Name, 1, 1, "", s_salt.ByteArrayToHex() }; yield return new object[] { hashAlgorithm.Name, 257, 257, "", s_salt.ByteArrayToHex() }; yield return new object[] { hashAlgorithm.Name, 257, 257, Password, "D8D8D8D8D8D8D8D8" }; yield return new object[] { hashAlgorithm.Name, 257, 257, Password, "0000000000000000" }; + + // case for password exceeding the stack buffer limit. + yield return new object[] { hashAlgorithm.Name, 257, 257, largePassword, "0000000000000000" }; } } From f8d82d328a8f4333b15ee1fa57560ca40aece12a Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 15 Feb 2021 16:46:20 -0500 Subject: [PATCH 15/29] Use BCryptKeyDerivation on Windows 8+. This improves performance on platforms which support it. --- .../BCrypt/Interop.BCryptAlgPseudoHandle.cs | 6 +- .../BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs | 12 - .../Interop.BCryptGenerateSymmetricKey.cs | 32 +++ .../BCrypt/Interop.BCryptKeyDerivation.cs | 23 ++ .../Interop/Windows/BCrypt/Interop.Blobs.cs | 23 +- .../Cryptography/ECCng.ImportExport.cs | 2 +- .../Pbkdf2Implementation.Windows.cs | 261 +++++++++++++++--- ...em.Security.Cryptography.Algorithms.csproj | 8 + 8 files changed, 304 insertions(+), 63 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptGenerateSymmetricKey.cs create mode 100644 src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptKeyDerivation.cs diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs index c56ab953e5d194..a33bc3a245ed82 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs @@ -18,11 +18,7 @@ public enum BCryptAlgPseudoHandle : uint BCRYPT_SHA384_ALG_HANDLE = 0x00000051, BCRYPT_SHA512_ALG_HANDLE = 0x00000061, - BCRYPT_HMAC_MD5_ALG_HANDLE = 0x00000091, - BCRYPT_HMAC_SHA1_ALG_HANDLE = 0x000000A1, - BCRYPT_HMAC_SHA256_ALG_HANDLE = 0x000000B1, - BCRYPT_HMAC_SHA384_ALG_HANDLE = 0x000000C1, - BCRYPT_HMAC_SHA512_ALG_HANDLE = 0x000000D1, + BCRYPT_PBKDF2_ALG_HANDLE = 0x00000331, } } } diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs index c38596757bebb5..56845c8b8cb0a7 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs @@ -22,17 +22,5 @@ internal static extern unsafe NTSTATUS BCryptDeriveKeyPBKDF2( byte* pbDerivedKey, int cbDerivedKey, int dwFlags); - - [DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)] - internal static extern unsafe NTSTATUS BCryptDeriveKeyPBKDF2( - nuint hPrf, - byte* pbPassword, - int cbPassword, - byte* pbSalt, - int cbSalt, - ulong cIterations, - byte* pbDerivedKey, - int cbDerivedKey, - int dwFlags); } } diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptGenerateSymmetricKey.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptGenerateSymmetricKey.cs new file mode 100644 index 00000000000000..0f829c5d33ea95 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptGenerateSymmetricKey.cs @@ -0,0 +1,32 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +internal partial class Interop +{ + internal partial class BCrypt + { + [DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)] + internal static unsafe extern NTSTATUS BCryptGenerateSymmetricKey( + SafeBCryptAlgorithmHandle hAlgorithm, + out SafeBCryptKeyHandle phKey, + IntPtr pbKeyObject, + int cbKeyObject, + byte* pbSecret, + int cbSecret, + uint dwFlags); + + [DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)] + internal static unsafe extern NTSTATUS BCryptGenerateSymmetricKey( + nuint hAlgorithm, + out SafeBCryptKeyHandle phKey, + IntPtr pbKeyObject, + int cbKeyObject, + byte* pbSecret, + int cbSecret, + uint dwFlags); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptKeyDerivation.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptKeyDerivation.cs new file mode 100644 index 00000000000000..dfe2c7d3f32f77 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptKeyDerivation.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; + +using Microsoft.Win32.SafeHandles; + +internal partial class Interop +{ + internal partial class BCrypt + { + [DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)] + internal static unsafe extern NTSTATUS BCryptKeyDerivation( + SafeBCryptKeyHandle hKey, + BCryptBufferDesc* pParameterList, + byte* pbDerivedKey, + int cbDerivedKey, + out uint pcbResult, + int dwFlags); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs index 8caf36e8a17947..6238245c7a21df 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs @@ -227,10 +227,27 @@ internal struct BCRYPT_ECCFULLKEY_BLOB } /// - /// NCrypt buffer descriptors + /// NCrypt or BCrypt buffer descriptors /// - internal enum NCryptBufferDescriptors : int + internal enum NCryptBCryptBufferDescriptors : int { + KDF_HASH_ALGORITHM = 0, + KDF_SECRET_PREPEND = 1, + KDF_SECRET_APPEND = 2, + KDF_HMAC_KEY = 3, + KDF_TLS_PRF_LABEL = 4, + KDF_TLS_PRF_SEED = 5, + KDF_SECRET_HANDLE = 6, + KDF_TLS_PRF_PROTOCOL = 7, + KDF_ALGORITHMID = 8, + KDF_PARTYUINFO = 9, + KDF_PARTYVINFO = 10, + KDF_SUPPPUBINFO = 11, + KDF_SUPPPRIVINFO = 12, + KDF_LABEL = 13, + KDF_CONTEXT = 14, + KDF_SALT = 15, + KDF_ITERATION_COUNT = 16, NCRYPTBUFFER_ECC_CURVE_NAME = 60, } @@ -241,7 +258,7 @@ internal enum NCryptBufferDescriptors : int internal struct BCryptBuffer { internal int cbBuffer; // Length of buffer, in bytes - internal NCryptBufferDescriptors BufferType; // Buffer type + internal NCryptBCryptBufferDescriptors BufferType; // Buffer type internal IntPtr pvBuffer; // Pointer to buffer } diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs index 4064f60f5f4ff4..cf632972eb8fbe 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs @@ -474,7 +474,7 @@ internal static SafeNCryptKeyHandle ImportKeyBlob( descPtr = Marshal.AllocHGlobal(Marshal.SizeOf(desc)); buffPtr = Marshal.AllocHGlobal(Marshal.SizeOf(buff)); buff.cbBuffer = (curveName.Length + 1) * 2; // Add 1 for null terminator - buff.BufferType = Interop.BCrypt.NCryptBufferDescriptors.NCRYPTBUFFER_ECC_CURVE_NAME; + buff.BufferType = Interop.BCrypt.NCryptBCryptBufferDescriptors.NCRYPTBUFFER_ECC_CURVE_NAME; buff.pvBuffer = safeCurveName.DangerousGetHandle(); Marshal.StructureToPtr(buff, buffPtr, false); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index f7e4f5e88897db..30bf466b4856ee 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -6,17 +6,23 @@ using System.Security.Cryptography; using Microsoft.Win32.SafeHandles; using BCryptAlgPseudoHandle = Interop.BCrypt.BCryptAlgPseudoHandle; +using BCryptBuffer = Interop.BCrypt.BCryptBuffer; using BCryptOpenAlgorithmProviderFlags = Interop.BCrypt.BCryptOpenAlgorithmProviderFlags; +using NCryptBCryptBufferDescriptors = Interop.BCrypt.NCryptBCryptBufferDescriptors; using NTSTATUS = Interop.BCrypt.NTSTATUS; namespace Internal.Cryptography { internal partial class Pbkdf2Implementation { - // BCryptDeriveKeyPBKDF2 on Windows 7 will crash the process with an access violation - // when given a pseudo handle, so we can't detect based on the NTSTATUS of the native call. private static readonly bool s_usePseudoHandles = OperatingSystem.IsWindowsVersionAtLeast(10, 0, 0); + // For Windows 7 we will use BCryptDeriveKeyPBKDF2. For Windows 8 we will use BCryptDeriveKey + // since it has better performance. + private static readonly bool s_useKeyDerivation = OperatingSystem.IsWindowsVersionAtLeast(8, 0, 0); + + private static volatile SafeBCryptAlgorithmHandle? s_pbkdf2AlgorithmHandle; + public static unsafe void Fill( ReadOnlySpan password, ReadOnlySpan salt, @@ -28,61 +34,232 @@ public static unsafe void Fill( Debug.Assert(iterations >= 0); Debug.Assert(hashAlgorithmName.Name is not null); - NTSTATUS status; + if (s_useKeyDerivation) + { + FillKeyDerivation(password, salt, iterations, hashAlgorithmName.Name, destination); + } + else + { + FillDeriveKeyPBKDF2(password, salt, iterations, hashAlgorithmName.Name, destination); + } + } - fixed (byte* pPassword = password) - fixed (byte* pSalt = salt) - fixed (byte* pDestination = destination) + private static unsafe void FillKeyDerivation( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + string hashAlgorithmName, + Span destination) + { + SafeBCryptKeyHandle keyHandle; + int hashBlockSizeBytes = GetHashBlockSize(hashAlgorithmName); + + // stackalloc 0 to let compiler know this cannot escape. + ReadOnlySpan symmetricKeyMaterial = stackalloc byte[0]; + int symmetricKeyMaterialLength; + Span clearSpan = stackalloc byte[0]; + + if (password.IsEmpty) + { + symmetricKeyMaterial = stackalloc byte[1]; + symmetricKeyMaterialLength = 0; + clearSpan = default; + } + else if (password.Length <= hashBlockSizeBytes) { - if (s_usePseudoHandles) + symmetricKeyMaterial = password; + symmetricKeyMaterialLength = password.Length; + clearSpan = default; + } + else + { + // RFC 2104: "The key for HMAC can be of any length (keys longer than B bytes are + // first hashed using H). + // We denote by B the byte-length of such + // blocks (B=64 for all the above mentioned examples of hash functions) + // + // Windows' PBKDF2 will do this up to a point. To ensure we accept arbitrary inputs for + // PBKDF2, we do the hashing ourselves. + Span hashBuffer = stackalloc byte[512 / 8]; // 64 bytes is SHA512, the largest digest handled. + int hashBufferSize; + + switch (hashAlgorithmName) + { + case HashAlgorithmNames.SHA1: + hashBufferSize = SHA1.HashData(password, hashBuffer); + break; + case HashAlgorithmNames.SHA256: + hashBufferSize = SHA256.HashData(password, hashBuffer); + break; + case HashAlgorithmNames.SHA384: + hashBufferSize = SHA384.HashData(password, hashBuffer); + break; + case HashAlgorithmNames.SHA512: + hashBufferSize = SHA512.HashData(password, hashBuffer); + break; + default: + throw new CryptographicException(); + } + + clearSpan = hashBuffer.Slice(0, hashBufferSize); + symmetricKeyMaterial = clearSpan; + symmetricKeyMaterialLength = symmetricKeyMaterial.Length; + } + + if (s_usePseudoHandles) + { + fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial) { - BCryptAlgPseudoHandle pseudoHandle = PseudoHandleFromIdentifier(hashAlgorithmName.Name); - status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( - (nuint)pseudoHandle, - pPassword, - password.Length, - pSalt, - salt.Length, - (ulong)iterations, - pDestination, - destination.Length, + NTSTATUS generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey( + (nuint)BCryptAlgPseudoHandle.BCRYPT_PBKDF2_ALG_HANDLE, + out keyHandle, + pbKeyObject: IntPtr.Zero, + cbKeyObject: 0, + pSymmetricKeyMaterial, + symmetricKeyMaterialLength, dwFlags: 0); + + CryptographicOperations.ZeroMemory(clearSpan); + + if (generateKeyStatus != NTSTATUS.STATUS_SUCCESS) + { + throw Interop.BCrypt.CreateCryptographicException(generateKeyStatus); + } } - else + } + else + { + if (s_pbkdf2AlgorithmHandle is null) + { + NTSTATUS openStatus = Interop.BCrypt.BCryptOpenAlgorithmProvider( + out SafeBCryptAlgorithmHandle pbkdf2AlgorithmHandle, + "PBKDF2", + null, + BCryptOpenAlgorithmProviderFlags.None); + + if (openStatus != NTSTATUS.STATUS_SUCCESS) + { + throw Interop.BCrypt.CreateCryptographicException(openStatus); + } + + // This might race on the null check above, and that's okay. Worst + // case the algorithm is opened more than once, and they will get + // cleaned up during collection. + s_pbkdf2AlgorithmHandle = pbkdf2AlgorithmHandle; + } + + fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial) { - const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; - // Do not dispose handle since it is shared and cached. - SafeBCryptAlgorithmHandle handle = - Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName.Name, OpenAlgorithmFlags, out _); - - status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( - handle, - pPassword, - password.Length, - pSalt, - salt.Length, - (ulong)iterations, - pDestination, - destination.Length, + NTSTATUS generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey( + s_pbkdf2AlgorithmHandle, + out keyHandle, + pbKeyObject: IntPtr.Zero, + cbKeyObject: 0, + pSymmetricKeyMaterial, + symmetricKeyMaterialLength, dwFlags: 0); + + CryptographicOperations.ZeroMemory(clearSpan); + + if (generateKeyStatus != NTSTATUS.STATUS_SUCCESS) + { + throw Interop.BCrypt.CreateCryptographicException(generateKeyStatus); + } } + } - if (status != NTSTATUS.STATUS_SUCCESS) + Debug.Assert(!keyHandle.IsInvalid); + + ulong kdfIterations = (ulong)iterations; // Previously asserted to be positive. + + using (keyHandle) + fixed (char* pHashAlgorithmName = hashAlgorithmName) + fixed (byte* pSalt = salt) + fixed (byte* pDestination = destination) + { + BCryptBuffer* buffers = stackalloc BCryptBuffer[3]; + buffers[0].BufferType = NCryptBCryptBufferDescriptors.KDF_ITERATION_COUNT; + buffers[0].pvBuffer = (IntPtr)(&kdfIterations); + buffers[0].cbBuffer = sizeof(ulong); + + buffers[1].BufferType = NCryptBCryptBufferDescriptors.KDF_SALT; + buffers[1].pvBuffer = (IntPtr)pSalt; + buffers[1].cbBuffer = salt.Length; + + buffers[2].BufferType = NCryptBCryptBufferDescriptors.KDF_HASH_ALGORITHM; + buffers[2].pvBuffer = (IntPtr)pHashAlgorithmName; + + // C# spec: "A char* value produced by fixing a string instance always points to a null-terminated string" + buffers[2].cbBuffer = checked((hashAlgorithmName.Length + 1) * sizeof(char)); // Add null terminator. + + Interop.BCrypt.BCryptBufferDesc bufferDesc; + bufferDesc.ulVersion = Interop.BCrypt.BCRYPTBUFFER_VERSION; + bufferDesc.cBuffers = 3; + bufferDesc.pBuffers = (IntPtr)buffers; + + NTSTATUS deriveStatus = Interop.BCrypt.BCryptKeyDerivation( + keyHandle, + &bufferDesc, + pDestination, + destination.Length, + out uint resultLength, + dwFlags: 0); + + if (deriveStatus != NTSTATUS.STATUS_SUCCESS) { - throw Interop.BCrypt.CreateCryptographicException(status); + throw Interop.BCrypt.CreateCryptographicException(deriveStatus); + } + + if (destination.Length != resultLength) + { + throw new CryptographicException(); } } } - private static BCryptAlgPseudoHandle PseudoHandleFromIdentifier(string hashAlgorithmId) + private static unsafe void FillDeriveKeyPBKDF2( + ReadOnlySpan password, + ReadOnlySpan salt, + int iterations, + string hashAlgorithmName, + Span destination) { - return hashAlgorithmId switch { - HashAlgorithmNames.SHA1 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA1_ALG_HANDLE, - HashAlgorithmNames.SHA256 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA256_ALG_HANDLE, - HashAlgorithmNames.SHA384 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA384_ALG_HANDLE, - HashAlgorithmNames.SHA512 => BCryptAlgPseudoHandle.BCRYPT_HMAC_SHA512_ALG_HANDLE, - _ => throw new CryptographicException() - }; + const BCryptOpenAlgorithmProviderFlags OpenAlgorithmFlags = BCryptOpenAlgorithmProviderFlags.BCRYPT_ALG_HANDLE_HMAC_FLAG; + + // This code path will only be taken on Windows 7, so we can assume pseudo handles are not supported. + // Do not dispose handle since it is shared and cached. + SafeBCryptAlgorithmHandle handle = + Interop.BCrypt.BCryptAlgorithmCache.GetCachedBCryptAlgorithmHandle(hashAlgorithmName, OpenAlgorithmFlags, out _); + + fixed (byte* pPassword = password) + fixed (byte* pSalt = salt) + fixed (byte* pDestination = destination) + { + NTSTATUS status = Interop.BCrypt.BCryptDeriveKeyPBKDF2( + handle, + pPassword, + password.Length, + pSalt, + salt.Length, + (ulong)iterations, + pDestination, + destination.Length, + dwFlags: 0); + + if (status != NTSTATUS.STATUS_SUCCESS) + { + throw Interop.BCrypt.CreateCryptographicException(status); + } + } } + + private static int GetHashBlockSize(string hashAlgorithmName) => hashAlgorithmName switch { + // Block sizes per NIST FIPS pub 180-4. + HashAlgorithmNames.SHA1 => 512, + HashAlgorithmNames.SHA256 => 512, + HashAlgorithmNames.SHA384 => 1024, + HashAlgorithmNames.SHA512 => 1024, + _ => throw new CryptographicException(), // Should have been validated before getting here. + }; } } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj b/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj index eb1f604843e9fd..4e6ab6f54dddce 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System.Security.Cryptography.Algorithms.csproj @@ -334,12 +334,18 @@ Link="Common\Interop\Windows\BCrypt\Interop.BCryptAlgPseudoHandle.cs" /> + + + + Date: Tue, 16 Feb 2021 11:11:11 -0500 Subject: [PATCH 16/29] Cleanup and some refactoring for Windows implementation --- .../Common/src/Interop/Windows/BCrypt/Cng.cs | 1 + .../Pbkdf2Implementation.Windows.cs | 93 ++++++++++--------- 2 files changed, 50 insertions(+), 44 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs index 3163dceeda0d29..7642b9faeafa5a 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs @@ -35,6 +35,7 @@ internal static class AlgorithmName public const string Sha256 = "SHA256"; // BCRYPT_SHA256_ALGORITHM public const string Sha384 = "SHA384"; // BCRYPT_SHA384_ALGORITHM public const string Sha512 = "SHA512"; // BCRYPT_SHA512_ALGORITHM + public const string Pbkdf2 = "PBKDF2"; // BCRYPT_PBKDF2_ALGORITHM } internal static class KeyDerivationFunction diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index 30bf466b4856ee..cd013c53f7bc62 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; +using System.Threading; using System.Security.Cryptography; using Microsoft.Win32.SafeHandles; using BCryptAlgPseudoHandle = Interop.BCrypt.BCryptAlgPseudoHandle; @@ -17,11 +18,12 @@ internal partial class Pbkdf2Implementation { private static readonly bool s_usePseudoHandles = OperatingSystem.IsWindowsVersionAtLeast(10, 0, 0); - // For Windows 7 we will use BCryptDeriveKeyPBKDF2. For Windows 8 we will use BCryptDeriveKey + // For Windows 7 we will use BCryptDeriveKeyPBKDF2. For Windows 8+ we will use BCryptKeyDerivation // since it has better performance. private static readonly bool s_useKeyDerivation = OperatingSystem.IsWindowsVersionAtLeast(8, 0, 0); - private static volatile SafeBCryptAlgorithmHandle? s_pbkdf2AlgorithmHandle; + // A cached instance of PBKDF2 for Windows 8, where pseudo handles are not supported. + private static SafeBCryptAlgorithmHandle? s_pbkdf2AlgorithmHandle; public static unsafe void Fill( ReadOnlySpan password, @@ -55,18 +57,20 @@ private static unsafe void FillKeyDerivation( int hashBlockSizeBytes = GetHashBlockSize(hashAlgorithmName); // stackalloc 0 to let compiler know this cannot escape. + Span clearSpan = stackalloc byte[0]; ReadOnlySpan symmetricKeyMaterial = stackalloc byte[0]; int symmetricKeyMaterialLength; - Span clearSpan = stackalloc byte[0]; if (password.IsEmpty) { + // CNG won't accept a null pointer for the password. symmetricKeyMaterial = stackalloc byte[1]; symmetricKeyMaterialLength = 0; clearSpan = default; } else if (password.Length <= hashBlockSizeBytes) { + // Password is small enough to use as-is. symmetricKeyMaterial = password; symmetricKeyMaterialLength = password.Length; clearSpan = default; @@ -103,14 +107,18 @@ private static unsafe void FillKeyDerivation( clearSpan = hashBuffer.Slice(0, hashBufferSize); symmetricKeyMaterial = clearSpan; - symmetricKeyMaterialLength = symmetricKeyMaterial.Length; + symmetricKeyMaterialLength = hashBufferSize; } + Debug.Assert(symmetricKeyMaterial.Length > 0); + + NTSTATUS generateKeyStatus; + if (s_usePseudoHandles) { fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial) { - NTSTATUS generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey( + generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey( (nuint)BCryptAlgPseudoHandle.BCRYPT_PBKDF2_ALG_HANDLE, out keyHandle, pbKeyObject: IntPtr.Zero, @@ -118,13 +126,6 @@ private static unsafe void FillKeyDerivation( pSymmetricKeyMaterial, symmetricKeyMaterialLength, dwFlags: 0); - - CryptographicOperations.ZeroMemory(clearSpan); - - if (generateKeyStatus != NTSTATUS.STATUS_SUCCESS) - { - throw Interop.BCrypt.CreateCryptographicException(generateKeyStatus); - } } } else @@ -133,24 +134,24 @@ private static unsafe void FillKeyDerivation( { NTSTATUS openStatus = Interop.BCrypt.BCryptOpenAlgorithmProvider( out SafeBCryptAlgorithmHandle pbkdf2AlgorithmHandle, - "PBKDF2", + Internal.NativeCrypto.BCryptNative.AlgorithmName.Pbkdf2, null, BCryptOpenAlgorithmProviderFlags.None); if (openStatus != NTSTATUS.STATUS_SUCCESS) { + CryptographicOperations.ZeroMemory(clearSpan); throw Interop.BCrypt.CreateCryptographicException(openStatus); } - // This might race on the null check above, and that's okay. Worst - // case the algorithm is opened more than once, and they will get - // cleaned up during collection. - s_pbkdf2AlgorithmHandle = pbkdf2AlgorithmHandle; + // This might race, and that's okay. Worst case the algorithm is opened + // more than once, and the ones that lost will get cleaned up during collection. + Interlocked.CompareExchange(ref s_pbkdf2AlgorithmHandle, pbkdf2AlgorithmHandle, null); } fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial) { - NTSTATUS generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey( + generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey( s_pbkdf2AlgorithmHandle, out keyHandle, pbKeyObject: IntPtr.Zero, @@ -158,14 +159,14 @@ private static unsafe void FillKeyDerivation( pSymmetricKeyMaterial, symmetricKeyMaterialLength, dwFlags: 0); + } + } - CryptographicOperations.ZeroMemory(clearSpan); + CryptographicOperations.ZeroMemory(clearSpan); - if (generateKeyStatus != NTSTATUS.STATUS_SUCCESS) - { - throw Interop.BCrypt.CreateCryptographicException(generateKeyStatus); - } - } + if (generateKeyStatus != NTSTATUS.STATUS_SUCCESS) + { + throw Interop.BCrypt.CreateCryptographicException(generateKeyStatus); } Debug.Assert(!keyHandle.IsInvalid); @@ -177,7 +178,7 @@ private static unsafe void FillKeyDerivation( fixed (byte* pSalt = salt) fixed (byte* pDestination = destination) { - BCryptBuffer* buffers = stackalloc BCryptBuffer[3]; + Span buffers = stackalloc BCryptBuffer[3]; buffers[0].BufferType = NCryptBCryptBufferDescriptors.KDF_ITERATION_COUNT; buffers[0].pvBuffer = (IntPtr)(&kdfIterations); buffers[0].cbBuffer = sizeof(ulong); @@ -192,27 +193,31 @@ private static unsafe void FillKeyDerivation( // C# spec: "A char* value produced by fixing a string instance always points to a null-terminated string" buffers[2].cbBuffer = checked((hashAlgorithmName.Length + 1) * sizeof(char)); // Add null terminator. - Interop.BCrypt.BCryptBufferDesc bufferDesc; - bufferDesc.ulVersion = Interop.BCrypt.BCRYPTBUFFER_VERSION; - bufferDesc.cBuffers = 3; - bufferDesc.pBuffers = (IntPtr)buffers; - - NTSTATUS deriveStatus = Interop.BCrypt.BCryptKeyDerivation( - keyHandle, - &bufferDesc, - pDestination, - destination.Length, - out uint resultLength, - dwFlags: 0); - - if (deriveStatus != NTSTATUS.STATUS_SUCCESS) + fixed (BCryptBuffer* pBuffers = buffers) { - throw Interop.BCrypt.CreateCryptographicException(deriveStatus); - } + Interop.BCrypt.BCryptBufferDesc bufferDesc; + bufferDesc.ulVersion = Interop.BCrypt.BCRYPTBUFFER_VERSION; + bufferDesc.cBuffers = buffers.Length; + bufferDesc.pBuffers = (IntPtr)pBuffers; + + NTSTATUS deriveStatus = Interop.BCrypt.BCryptKeyDerivation( + keyHandle, + &bufferDesc, + pDestination, + destination.Length, + out uint resultLength, + dwFlags: 0); - if (destination.Length != resultLength) - { - throw new CryptographicException(); + if (deriveStatus != NTSTATUS.STATUS_SUCCESS) + { + throw Interop.BCrypt.CreateCryptographicException(deriveStatus); + } + + if (destination.Length != resultLength) + { + Debug.Fail("PBKDF2 resultLength != destination.Length"); + throw new CryptographicException(); + } } } } From c322666687a158e78dfd31383f3d86f1a0a14479 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 16 Feb 2021 13:06:37 -0500 Subject: [PATCH 17/29] Use throwing UTF8. If a string input is invalid UTF8, use an encoding that has a throwing encoder fallback. --- .../Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs | 7 +++++-- .../tests/Rfc2898OneShotTests.cs | 8 ++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 409c756d84d43b..6aea0a3c2c3aee 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -8,6 +8,9 @@ namespace System.Security.Cryptography { public partial class Rfc2898DeriveBytes { + // Throwing UTF8 on invalid input. + private static readonly Encoding s_throwingUtf8Encoding = new UTF8Encoding(false, true); + public static byte[] Pbkdf2DeriveBytes( byte[] password, byte[] salt, @@ -88,12 +91,12 @@ public static void Pbkdf2DeriveBytes( const int MaxPasswordStackSize = 256; byte[]? rentedPasswordBuffer = null; - int maxEncodedSize = Encoding.UTF8.GetMaxByteCount(password.Length); + int maxEncodedSize = s_throwingUtf8Encoding.GetMaxByteCount(password.Length); Span passwordBuffer = maxEncodedSize > MaxPasswordStackSize ? (rentedPasswordBuffer = CryptoPool.Rent(maxEncodedSize)) : stackalloc byte[MaxPasswordStackSize]; - int passwordBytesWritten = Encoding.UTF8.GetBytes(password, passwordBuffer); + int passwordBytesWritten = s_throwingUtf8Encoding.GetBytes(password, passwordBuffer); Span passwordBytes = passwordBuffer.Slice(0, passwordBytesWritten); Pbkdf2DeriveBytesCore(passwordBytes, salt, iterations, hashAlgorithm, destination); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index aab5264638dce4..615a9cbc031683 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -143,6 +143,14 @@ public static void Pbkdf2DeriveBytes_PasswordString_EmptyHashName() ); } + [Fact] + public static void Pbkdf2DeriveBytes_PasswordString_InvalidUtf8() + { + Assert.Throws(() => + Rfc2898DeriveBytes.Pbkdf2DeriveBytes( + "\uD800", s_salt, iterations: 1, s_extractLength, HashAlgorithmName.SHA256)); + } + [Theory] [MemberData(nameof(Pbkdf2DeriveBytes_PasswordBytes_Compare_Data))] public static void Pbkdf2DeriveBytes_PasswordBytes_Compare( From 8c6d37019ba18d9ee46363ee14590a04470fcb18 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 16 Feb 2021 14:16:42 -0500 Subject: [PATCH 18/29] Change to follow approved API. --- ...System.Security.Cryptography.Algorithms.cs | 12 +- .../Rfc2898DeriveBytes.OneShot.cs | 66 ++++----- .../tests/Rfc2898OneShotTests.cs | 128 ++++++++++-------- 3 files changed, 112 insertions(+), 94 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs b/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs index f173f440dd7939..e5a78f387eedf3 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs @@ -587,12 +587,12 @@ public Rfc2898DeriveBytes(string password, int saltSize, int iterations, System. public byte[] CryptDeriveKey(string algname, string alghashname, int keySize, byte[] rgbIV) { throw null; } protected override void Dispose(bool disposing) { } public override byte[] GetBytes(int cb) { throw null; } - public static byte[] Pbkdf2DeriveBytes(byte[] password, byte[] salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } - public static byte[] Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } - public static void Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, System.Span destination) { } - public static byte[] Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } - public static void Pbkdf2DeriveBytes(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, System.Span destination) { } - public static byte[] Pbkdf2DeriveBytes(string password, byte[] salt, int iterations, int length, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } + public static byte[] Pbkdf2(byte[] password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; } + public static byte[] Pbkdf2(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; } + public static void Pbkdf2(System.ReadOnlySpan password, System.ReadOnlySpan salt, System.Span destination, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { } + public static byte[] Pbkdf2(System.ReadOnlySpan password, System.ReadOnlySpan salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; } + public static void Pbkdf2(System.ReadOnlySpan password, System.ReadOnlySpan salt, System.Span destination, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { } + public static byte[] Pbkdf2(string password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; } public override void Reset() { } } [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 6aea0a3c2c3aee..84bcc0c44c17e3 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -11,82 +11,82 @@ public partial class Rfc2898DeriveBytes // Throwing UTF8 on invalid input. private static readonly Encoding s_throwingUtf8Encoding = new UTF8Encoding(false, true); - public static byte[] Pbkdf2DeriveBytes( + public static byte[] Pbkdf2( byte[] password, byte[] salt, int iterations, - int length, - HashAlgorithmName hashAlgorithm) + HashAlgorithmName hashAlgorithm, + int outputLength) { if (password is null) throw new ArgumentNullException(nameof(password)); if (salt is null) throw new ArgumentNullException(nameof(salt)); - return Pbkdf2DeriveBytes(new ReadOnlySpan(password), new ReadOnlySpan(salt), iterations, length, hashAlgorithm); + return Pbkdf2(new ReadOnlySpan(password), new ReadOnlySpan(salt), iterations, hashAlgorithm, outputLength); } - public static byte[] Pbkdf2DeriveBytes( + public static byte[] Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, int iterations, - int length, - HashAlgorithmName hashAlgorithm) + HashAlgorithmName hashAlgorithm, + int outputLength) { - if (length < 0) - throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum); + if (outputLength < 0) + throw new ArgumentOutOfRangeException(nameof(outputLength), SR.ArgumentOutOfRange_NeedNonNegNum); - byte[] result = new byte[length]; - Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithm, result); + byte[] result = new byte[outputLength]; + Pbkdf2(password, salt, result, iterations, hashAlgorithm); return result; } - public static void Pbkdf2DeriveBytes( + public static void Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, + Span destination, int iterations, - HashAlgorithmName hashAlgorithm, - Span destination) + HashAlgorithmName hashAlgorithm) { - Pbkdf2DeriveBytesCore(password, salt, iterations, hashAlgorithm, destination); + Pbkdf2Core(password, salt, destination, iterations, hashAlgorithm); } - public static byte[] Pbkdf2DeriveBytes( + public static byte[] Pbkdf2( string password, byte[] salt, int iterations, - int length, - HashAlgorithmName hashAlgorithm) + HashAlgorithmName hashAlgorithm, + int outputLength) { if (password is null) throw new ArgumentNullException(nameof(password)); if (salt is null) throw new ArgumentNullException(nameof(salt)); - return Pbkdf2DeriveBytes(password.AsSpan(), new ReadOnlySpan(salt), iterations, length, hashAlgorithm); + return Pbkdf2(password.AsSpan(), new ReadOnlySpan(salt), iterations, hashAlgorithm, outputLength); } - public static byte[] Pbkdf2DeriveBytes( + public static byte[] Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, int iterations, - int length, - HashAlgorithmName hashAlgorithm) + HashAlgorithmName hashAlgorithm, + int outputLength) { - if (length < 0) - throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum); + if (outputLength < 0) + throw new ArgumentOutOfRangeException(nameof(outputLength), SR.ArgumentOutOfRange_NeedNonNegNum); - byte[] result = new byte[length]; - Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithm, result); + byte[] result = new byte[outputLength]; + Pbkdf2(password, salt, result, iterations, hashAlgorithm); return result; } - public static void Pbkdf2DeriveBytes( + public static void Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, + Span destination, int iterations, - HashAlgorithmName hashAlgorithm, - Span destination) + HashAlgorithmName hashAlgorithm) { const int MaxPasswordStackSize = 256; @@ -99,7 +99,7 @@ public static void Pbkdf2DeriveBytes( int passwordBytesWritten = s_throwingUtf8Encoding.GetBytes(password, passwordBuffer); Span passwordBytes = passwordBuffer.Slice(0, passwordBytesWritten); - Pbkdf2DeriveBytesCore(passwordBytes, salt, iterations, hashAlgorithm, destination); + Pbkdf2Core(passwordBytes, salt, destination, iterations, hashAlgorithm); CryptographicOperations.ZeroMemory(passwordBytes); if (rentedPasswordBuffer is not null) @@ -108,12 +108,12 @@ public static void Pbkdf2DeriveBytes( } } - private static void Pbkdf2DeriveBytesCore( + private static void Pbkdf2Core( ReadOnlySpan password, ReadOnlySpan salt, + Span destination, int iterations, - HashAlgorithmName hashAlgorithm, - Span destination) + HashAlgorithmName hashAlgorithm) { if (iterations <= 0) throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index 615a9cbc031683..b00df13104ef36 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -18,142 +18,160 @@ public static class Rfc2898OneShotTests private static readonly int s_extractLength = 14; [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_NullPassword() + public static void Pbkdf2_PasswordBytes_NullPassword() { AssertExtensions.Throws("password", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - password: (byte[])null, s_salt, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) + Rfc2898DeriveBytes.Pbkdf2( + password: (byte[])null, s_salt, iterations: 1, HashAlgorithmName.SHA256, s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_NullSalt() + public static void Pbkdf2_PasswordBytes_NullSalt() { AssertExtensions.Throws("salt", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, salt: (byte[])null, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) + Rfc2898DeriveBytes.Pbkdf2( + s_passwordBytes, salt: (byte[])null, iterations: 1, HashAlgorithmName.SHA256, s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_SaltEmpty() + public static void Pbkdf2_PasswordBytes_SaltBytes_SaltEmpty() { byte[] expectedKey = "1E437A1C79D75BE61E91141DAE20".HexToByteArray(); - byte[] key = Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - new byte[0], salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA1); + byte[] key = Rfc2898DeriveBytes.Pbkdf2( + new byte[0], salt: new byte[0], iterations: 1, HashAlgorithmName.SHA1, s_extractLength); Assert.Equal(expectedKey, key); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_SaltBytes_IterationsNegative() + public static void Pbkdf2_PasswordBytes_SaltBytes_IterationsNegative() { AssertExtensions.Throws("iterations", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, s_salt, iterations: -1, s_extractLength, HashAlgorithmName.SHA256) + Rfc2898DeriveBytes.Pbkdf2( + s_passwordBytes, s_salt, iterations: -1, HashAlgorithmName.SHA256, s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_BogusHash() + public static void Pbkdf2_PasswordBytes_SaltBytes_OutputLengthNegative() + { + AssertExtensions.Throws("outputLength", () => + Rfc2898DeriveBytes.Pbkdf2( + s_passwordBytes, s_salt, iterations: 1, HashAlgorithmName.SHA256, -1) + ); + } + + [Fact] + public static void Pbkdf2_PasswordBytes_BogusHash() { Assert.Throws(() => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("BLAH")) + Rfc2898DeriveBytes.Pbkdf2( + s_passwordBytes, s_salt, iterations: 1, new HashAlgorithmName("BLAH"), s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_NullHashName() + public static void Pbkdf2_PasswordBytes_NullHashName() { AssertExtensions.Throws("hashAlgorithm", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, s_salt, iterations: 1, s_extractLength, default(HashAlgorithmName)) + Rfc2898DeriveBytes.Pbkdf2( + s_passwordBytes, s_salt, iterations: 1, default(HashAlgorithmName), s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordBytes_EmptyHashName() + public static void Pbkdf2_PasswordBytes_EmptyHashName() { AssertExtensions.Throws("hashAlgorithm", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - s_passwordBytes, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("")) + Rfc2898DeriveBytes.Pbkdf2( + s_passwordBytes, s_salt, iterations: 1, new HashAlgorithmName(""), s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_NullPassword() + public static void Pbkdf2_PasswordString_NullPassword() { AssertExtensions.Throws("password", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - password: (string)null, s_salt, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) + Rfc2898DeriveBytes.Pbkdf2( + password: (string)null, s_salt, iterations: 1, HashAlgorithmName.SHA256, s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_NullSalt() + public static void Pbkdf2_PasswordString_NullSalt() { AssertExtensions.Throws("salt", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - Password, salt: null, iterations: 1, s_extractLength, HashAlgorithmName.SHA256) + Rfc2898DeriveBytes.Pbkdf2( + Password, salt: null, iterations: 1, HashAlgorithmName.SHA256, s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_SaltBytes_SaltEmpty() + public static void Pbkdf2_PasswordString_SaltBytes_SaltEmpty() { byte[] expectedKey = "1E437A1C79D75BE61E91141DAE20".HexToByteArray(); - byte[] key = Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - password: "", salt: new byte[0], iterations: 1, s_extractLength, HashAlgorithmName.SHA1); + byte[] key = Rfc2898DeriveBytes.Pbkdf2( + password: "", salt: new byte[0], iterations: 1, HashAlgorithmName.SHA1, s_extractLength); Assert.Equal(expectedKey, key); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_SaltBytes_IterationsNegative() + public static void Pbkdf2_PasswordString_SaltBytes_IterationsNegative() { AssertExtensions.Throws("iterations", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - Password, s_salt, iterations: -1, s_extractLength, HashAlgorithmName.SHA256) + Rfc2898DeriveBytes.Pbkdf2( + Password, s_salt, iterations: -1, HashAlgorithmName.SHA256, s_extractLength) + ); + } + + [Fact] + public static void Pbkdf2_PasswordString_SaltBytes_OutputLengthNegative() + { + AssertExtensions.Throws("outputLength", () => + Rfc2898DeriveBytes.Pbkdf2( + Password, s_salt, iterations: 1, HashAlgorithmName.SHA256, -1) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_BogusHash() + public static void Pbkdf2_PasswordString_BogusHash() { Assert.Throws(() => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - Password, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("BLAH")) + Rfc2898DeriveBytes.Pbkdf2( + Password, s_salt, iterations: 1, new HashAlgorithmName("BLAH"), s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_NullHashName() + public static void Pbkdf2_PasswordString_NullHashName() { AssertExtensions.Throws("hashAlgorithm", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - Password, s_salt, iterations: 1, s_extractLength, default(HashAlgorithmName)) + Rfc2898DeriveBytes.Pbkdf2( + Password, s_salt, iterations: 1, default(HashAlgorithmName), s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_EmptyHashName() + public static void Pbkdf2_PasswordString_EmptyHashName() { AssertExtensions.Throws("hashAlgorithm", () => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - Password, s_salt, iterations: 1, s_extractLength, new HashAlgorithmName("")) + Rfc2898DeriveBytes.Pbkdf2( + Password, s_salt, iterations: 1, new HashAlgorithmName(""), s_extractLength) ); } [Fact] - public static void Pbkdf2DeriveBytes_PasswordString_InvalidUtf8() + public static void Pbkdf2_PasswordString_InvalidUtf8() { Assert.Throws(() => - Rfc2898DeriveBytes.Pbkdf2DeriveBytes( - "\uD800", s_salt, iterations: 1, s_extractLength, HashAlgorithmName.SHA256)); + Rfc2898DeriveBytes.Pbkdf2( + "\uD800", s_salt, iterations: 1, HashAlgorithmName.SHA256, s_extractLength)); } [Theory] - [MemberData(nameof(Pbkdf2DeriveBytes_PasswordBytes_Compare_Data))] - public static void Pbkdf2DeriveBytes_PasswordBytes_Compare( + [MemberData(nameof(Pbkdf2_PasswordBytes_Compare_Data))] + public static void Pbkdf2_PasswordBytes_Compare( string hashAlgorithm, int length, int iterations, @@ -171,12 +189,12 @@ public static void Pbkdf2DeriveBytes_PasswordBytes_Compare( } // byte array allocating - byte[] key2 = Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, length, hashAlgorithmName); + byte[] key2 = Rfc2898DeriveBytes.Pbkdf2(password, salt, iterations, hashAlgorithmName, length); Assert.Equal(key1, key2); Span destinationBuffer = new byte[length + 2]; Span destination = destinationBuffer.Slice(1, length); - Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithmName, destination); + Rfc2898DeriveBytes.Pbkdf2(password, salt, destination, iterations, hashAlgorithmName); Assert.True(key1.AsSpan().SequenceEqual(destination), "key1 == destination"); Assert.Equal(0, destinationBuffer[^1]); // Make sure we didn't write past the destination @@ -184,8 +202,8 @@ public static void Pbkdf2DeriveBytes_PasswordBytes_Compare( } [Theory] - [MemberData(nameof(Pbkdf2DeriveBytes_PasswordString_Compare_Data))] - public static void Pbkdf2DeriveBytes_PasswordString_Compare( + [MemberData(nameof(Pbkdf2_PasswordString_Compare_Data))] + public static void Pbkdf2_PasswordString_Compare( string hashAlgorithm, int length, int iterations, @@ -202,19 +220,19 @@ public static void Pbkdf2DeriveBytes_PasswordString_Compare( } // byte array allocating - byte[] key2 = Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, length, hashAlgorithmName); + byte[] key2 = Rfc2898DeriveBytes.Pbkdf2(password, salt, iterations, hashAlgorithmName, length); Assert.Equal(key1, key2); Span destinationBuffer = new byte[length + 2]; Span destination = destinationBuffer.Slice(1, length); - Rfc2898DeriveBytes.Pbkdf2DeriveBytes(password, salt, iterations, hashAlgorithmName, destination); + Rfc2898DeriveBytes.Pbkdf2(password, salt, destination, iterations, hashAlgorithmName); Assert.True(key1.AsSpan().SequenceEqual(destination), "key1 == destination"); Assert.Equal(0, destinationBuffer[^1]); // Make sure we didn't write past the destination Assert.Equal(0, destinationBuffer[0]); // Make sure we didn't write before the destination } - public static IEnumerable Pbkdf2DeriveBytes_PasswordBytes_Compare_Data() + public static IEnumerable Pbkdf2_PasswordBytes_Compare_Data() { foreach (HashAlgorithmName hashAlgorithm in SupportedHashAlgorithms) { @@ -227,7 +245,7 @@ public static IEnumerable Pbkdf2DeriveBytes_PasswordBytes_Compare_Data } } - public static IEnumerable Pbkdf2DeriveBytes_PasswordString_Compare_Data() + public static IEnumerable Pbkdf2_PasswordString_Compare_Data() { string largePassword = new string('y', 1024); From ddfd96099afcb4b5c7bcb3a34e6c52be21f515ef Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 16 Feb 2021 14:57:43 -0500 Subject: [PATCH 19/29] Tests for overlapping input / output buffers. --- .../tests/Rfc2898OneShotTests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index b00df13104ef36..791a5d7a67dc6d 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -169,6 +169,37 @@ public static void Pbkdf2_PasswordString_InvalidUtf8() "\uD800", s_salt, iterations: 1, HashAlgorithmName.SHA256, s_extractLength)); } + [Fact] + public static void Pbkdf2_Password_Salt_Overlapping_Completely() + { + Span buffer = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }; + byte[] expected = { 0xBE, 0xA4, 0xEE, 0x0E, 0xC3, 0x98, 0xBF, 0x32 }; + Rfc2898DeriveBytes.Pbkdf2(buffer, buffer, buffer, iterations: 1, HashAlgorithmName.SHA256); + Assert.Equal(expected, buffer.ToArray()); + } + + [Fact] + public static void Pbkdf2_Password_Salt_Overlapping_Forward() + { + Span buffer = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 0xFF }; + Span output = buffer[1..]; + Span inputs = buffer[..^1]; + byte[] expected = { 0xBE, 0xA4, 0xEE, 0x0E, 0xC3, 0x98, 0xBF, 0x32 }; + Rfc2898DeriveBytes.Pbkdf2(inputs, inputs, output, iterations: 1, HashAlgorithmName.SHA256); + Assert.Equal(expected, output.ToArray()); + } + + [Fact] + public static void Pbkdf2_Password_Salt_Overlapping_Backward() + { + Span buffer = new byte[] { 0xFF, 1, 2, 3, 4, 5, 6, 7, 8 }; + Span output = buffer[..^1]; + Span inputs = buffer[1..]; + byte[] expected = { 0xBE, 0xA4, 0xEE, 0x0E, 0xC3, 0x98, 0xBF, 0x32 }; + Rfc2898DeriveBytes.Pbkdf2(inputs, inputs, output, iterations: 1, HashAlgorithmName.SHA256); + Assert.Equal(expected, output.ToArray()); + } + [Theory] [MemberData(nameof(Pbkdf2_PasswordBytes_Compare_Data))] public static void Pbkdf2_PasswordBytes_Compare( From 5a2c91aafd8de3910efbbdc88a2660fac779b819 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 16 Feb 2021 15:15:29 -0500 Subject: [PATCH 20/29] Add tests around HMAC block size boundaries. --- .../tests/Rfc2898OneShotTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index 791a5d7a67dc6d..eeb089e5b24b46 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -274,6 +274,22 @@ public static IEnumerable Pbkdf2_PasswordBytes_Compare_Data() yield return new object[] { hashAlgorithm.Name, 257, 257, "D8D8D8D8D8D8D8D8", "D8D8D8D8D8D8D8D8" }; yield return new object[] { hashAlgorithm.Name, 257, 257, "0000000000000000", "0000000000000000" }; } + + // Test around HMAC SHA1 and SHA256 block boundaries + for (int blockBoundary = 63; blockBoundary <= 65; blockBoundary++) + { + byte[] password = new byte[blockBoundary]; + yield return new object[] { HashAlgorithmName.SHA1.Name, 257, 257, password.ByteArrayToHex(), "0000000000000000" }; + yield return new object[] { HashAlgorithmName.SHA256.Name, 257, 257, password.ByteArrayToHex(), "0000000000000000" }; + } + + // Test around HMAC SHA384 and SHA512 block boundaries + for (int blockBoundary = 127; blockBoundary <= 129; blockBoundary++) + { + byte[] password = new byte[blockBoundary]; + yield return new object[] { HashAlgorithmName.SHA384.Name, 257, 257, password.ByteArrayToHex(), "0000000000000000" }; + yield return new object[] { HashAlgorithmName.SHA512.Name, 257, 257, password.ByteArrayToHex(), "0000000000000000" }; + } } public static IEnumerable Pbkdf2_PasswordString_Compare_Data() From b82528e69259fb14b9b35be83e4b7d8dc0613923 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 16 Feb 2021 15:50:32 -0500 Subject: [PATCH 21/29] Fix block sizes. --- .../Internal/Cryptography/Pbkdf2Implementation.Windows.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index cd013c53f7bc62..d2916e6584a9f1 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -260,10 +260,10 @@ private static unsafe void FillDeriveKeyPBKDF2( private static int GetHashBlockSize(string hashAlgorithmName) => hashAlgorithmName switch { // Block sizes per NIST FIPS pub 180-4. - HashAlgorithmNames.SHA1 => 512, - HashAlgorithmNames.SHA256 => 512, - HashAlgorithmNames.SHA384 => 1024, - HashAlgorithmNames.SHA512 => 1024, + HashAlgorithmNames.SHA1 => 512 / 8, + HashAlgorithmNames.SHA256 => 512 / 8, + HashAlgorithmNames.SHA384 => 1024 / 8, + HashAlgorithmNames.SHA512 => 1024 / 8, _ => throw new CryptographicException(), // Should have been validated before getting here. }; } From dc72dc3e7a105005b496b4b5d9f3f89326b22e92 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 16 Feb 2021 18:30:20 -0500 Subject: [PATCH 22/29] XML documentation. --- .../Rfc2898DeriveBytes.OneShot.cs | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 84bcc0c44c17e3..3dd4dbcdc66edc 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -11,6 +11,31 @@ public partial class Rfc2898DeriveBytes // Throwing UTF8 on invalid input. private static readonly Encoding s_throwingUtf8Encoding = new UTF8Encoding(false, true); + /// + /// Creates a PBKDF2 derived key from password bytes. + /// + /// The password used to derive the key. + /// The key salt used to derive the key. + /// The number of iterations for the operation. + /// The hash algorithm to use to derive the key. + /// The size of key to derive. + /// + /// or is . + /// + /// + /// is not zero or a positive value. + /// -or- + /// is not a positive value. + /// + /// + /// has a + /// that is empty or . + /// + /// + /// is an unsupported hash algorithm. Supported algorithms + /// are , , + /// , and . + /// public static byte[] Pbkdf2( byte[] password, byte[] salt, @@ -26,6 +51,28 @@ public static byte[] Pbkdf2( return Pbkdf2(new ReadOnlySpan(password), new ReadOnlySpan(salt), iterations, hashAlgorithm, outputLength); } + /// + /// Creates a PBKDF2 derived key from password bytes. + /// + /// The password used to derive the key. + /// The key salt used to derive the key. + /// The number of iterations for the operation. + /// The hash algorithm to use to derive the key. + /// The size of key to derive. + /// + /// is not zero or a positive value. + /// -or- + /// is not a positive value. + /// + /// + /// has a + /// that is empty or . + /// + /// + /// is an unsupported hash algorithm. Supported algorithms + /// are , , + /// , and . + /// public static byte[] Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, @@ -41,6 +88,26 @@ public static byte[] Pbkdf2( return result; } + /// + /// Fills a buffer with a PBKDF2 derived key. + /// + /// The password used to derive the key. + /// The key salt used to derive the key. + /// The number of iterations for the operation. + /// The hash algorithm to use to derive the key. + /// The buffer to fill with a derived key. + /// + /// is not a positive value. + /// + /// + /// has a + /// that is empty or . + /// + /// + /// is an unsupported hash algorithm. Supported algorithms + /// are , , + /// , and . + /// public static void Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, @@ -51,6 +118,39 @@ public static void Pbkdf2( Pbkdf2Core(password, salt, destination, iterations, hashAlgorithm); } + /// + /// Creates a PBKDF2 derived key from a password. + /// + /// The password used to derive the key. + /// The key salt used to derive the key. + /// The number of iterations for the operation. + /// The hash algorithm to use to derive the key. + /// The size of key to derive. + /// + /// or is . + /// + /// + /// is not zero or a positive value. + /// -or- + /// is not a positive value. + /// + /// + /// has a + /// that is empty or . + /// + /// + /// is an unsupported hash algorithm. Supported algorithms + /// are , , + /// , and . + /// + /// + /// contains text that cannot be converted to UTF8. + /// + /// + /// The will be converted to bytes using the UTF8 encoding. For + /// other encodings, convert the password string to bytes using the appropriate + /// and use . + /// public static byte[] Pbkdf2( string password, byte[] salt, @@ -66,6 +166,36 @@ public static byte[] Pbkdf2( return Pbkdf2(password.AsSpan(), new ReadOnlySpan(salt), iterations, hashAlgorithm, outputLength); } + /// + /// Creates a PBKDF2 derived key from a password. + /// + /// The password used to derive the key. + /// The key salt used to derive the key. + /// The number of iterations for the operation. + /// The hash algorithm to use to derive the key. + /// The size of key to derive. + /// + /// is not zero or a positive value. + /// -or- + /// is not a positive value. + /// + /// + /// has a + /// that is empty or . + /// + /// + /// is an unsupported hash algorithm. Supported algorithms + /// are , , + /// , and . + /// + /// + /// contains text that cannot be converted to UTF8. + /// + /// + /// The will be converted to bytes using the UTF8 encoding. For + /// other encodings, convert the password string to bytes using the appropriate + /// and use . + /// public static byte[] Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, @@ -81,6 +211,34 @@ public static byte[] Pbkdf2( return result; } + /// + /// Fills a buffer with a PBKDF2 derived key. + /// + /// The password used to derive the key. + /// The key salt used to derive the key. + /// The number of iterations for the operation. + /// The hash algorithm to use to derive the key. + /// The buffer to fill with a derived key. + /// + /// is not a positive value. + /// + /// + /// has a + /// that is empty or . + /// + /// + /// is an unsupported hash algorithm. Supported algorithms + /// are , , + /// , and . + /// + /// + /// contains text that cannot be converted to UTF8. + /// + /// + /// The will be converted to bytes using the UTF8 encoding. For + /// other encodings, convert the password string to bytes using the appropriate + /// and use . + /// public static void Pbkdf2( ReadOnlySpan password, ReadOnlySpan salt, From 3ec9a55764195c49e8e520eed01abd3e62a3631e Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 16 Feb 2021 21:15:58 -0500 Subject: [PATCH 23/29] Code review feedback. --- .../BCrypt/Interop.BCryptAlgPseudoHandle.cs | 1 - .../BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs | 2 +- .../Rfc2898DeriveBytes.OneShot.cs | 10 ++++++++-- .../tests/Rfc2898OneShotTests.cs | 20 +++++++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs index a33bc3a245ed82..8ebae440952617 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptAlgPseudoHandle.cs @@ -17,7 +17,6 @@ public enum BCryptAlgPseudoHandle : uint BCRYPT_SHA256_ALG_HANDLE = 0x00000041, BCRYPT_SHA384_ALG_HANDLE = 0x00000051, BCRYPT_SHA512_ALG_HANDLE = 0x00000061, - BCRYPT_PBKDF2_ALG_HANDLE = 0x00000331, } } diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs index 56845c8b8cb0a7..d514f2b8e5471e 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptDeriveKeyPBKDF2.cs @@ -21,6 +21,6 @@ internal static extern unsafe NTSTATUS BCryptDeriveKeyPBKDF2( ulong cIterations, byte* pbDerivedKey, int cbDerivedKey, - int dwFlags); + uint dwFlags); } } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 3dd4dbcdc66edc..2c78b5cd54c29a 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -257,8 +257,14 @@ public static void Pbkdf2( int passwordBytesWritten = s_throwingUtf8Encoding.GetBytes(password, passwordBuffer); Span passwordBytes = passwordBuffer.Slice(0, passwordBytesWritten); - Pbkdf2Core(passwordBytes, salt, destination, iterations, hashAlgorithm); - CryptographicOperations.ZeroMemory(passwordBytes); + try + { + Pbkdf2Core(passwordBytes, salt, destination, iterations, hashAlgorithm); + } + finally + { + CryptographicOperations.ZeroMemory(passwordBytes); + } if (rentedPasswordBuffer is not null) { diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index eeb089e5b24b46..9845a84fbc55c8 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -263,6 +263,16 @@ public static void Pbkdf2_PasswordString_Compare( Assert.Equal(0, destinationBuffer[0]); // Make sure we didn't write before the destination } + [Theory] + [MemberData(nameof(Pbkdf2_Rfc6070_Vectors))] + public static void Pbkdf2_Rfc6070(string password, string salt, int iterations, string expectedHex) + { + byte[] expected = expectedHex.HexToByteArray(); + byte[] saltBytes = Encoding.UTF8.GetBytes(salt); + byte[] actual = Rfc2898DeriveBytes.Pbkdf2(password, saltBytes, iterations, HashAlgorithmName.SHA1, expected.Length); + Assert.Equal(expected, actual); + } + public static IEnumerable Pbkdf2_PasswordBytes_Compare_Data() { foreach (HashAlgorithmName hashAlgorithm in SupportedHashAlgorithms) @@ -310,6 +320,16 @@ public static IEnumerable Pbkdf2_PasswordString_Compare_Data() } } + public static IEnumerable Pbkdf2_Rfc6070_Vectors() + { + // password (P), salt (S), iterations (c), expected (DK) + yield return new object[] { "password", "salt", 1, "0c60c80f961f0e71f3a9b524af6012062fe037a6" }; + yield return new object[] { "password", "salt", 2, "ea6c014dc72d6f8ccd1ed92ace1d41f0d8de8957" }; + yield return new object[] { "password", "salt", 16777216, "eefe3d61cd4da4e4e9945b3d6ba2158c2634e984" }; + yield return new object[] { "passwordPASSWORDpassword", "saltSALTsaltSALTsaltSALTsaltSALTsalt", 4096, "3d2eec4fe41c849b80c8d83662c0e44a8b291a964cf2f07038" }; + yield return new object[] { "pass\0word", "sa\0lt", 4096, "56fa6aa75548099dcc37d7f03425e0c3" }; + } + private static HashAlgorithmName[] SupportedHashAlgorithms => new [] { HashAlgorithmName.SHA1, From 19faa41458b7a4ccc1e04ec477303bbb60bd0f38 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 17 Feb 2021 09:51:05 -0500 Subject: [PATCH 24/29] Move high iterations test to OuterLoop. --- .../tests/Rfc2898OneShotTests.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index 9845a84fbc55c8..7e470192aac3fb 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -273,6 +273,18 @@ public static void Pbkdf2_Rfc6070(string password, string salt, int iterations, Assert.Equal(expected, actual); } + [Fact] + [OuterLoop("Uses a high number of iterations that can take over 20 seconds on some machines")] + public static void Pbkdf2_Rfc6070_HighIterations() + { + string password = "password"; + int iterations = 16777216; + byte[] expected = "eefe3d61cd4da4e4e9945b3d6ba2158c2634e984".HexToByteArray(); + byte[] salt = Encoding.UTF8.GetBytes("salt"); + byte[] actual = Rfc2898DeriveBytes.Pbkdf2(password, salt, iterations, HashAlgorithmName.SHA1, expected.Length); + Assert.Equal(expected, actual); + } + public static IEnumerable Pbkdf2_PasswordBytes_Compare_Data() { foreach (HashAlgorithmName hashAlgorithm in SupportedHashAlgorithms) @@ -325,7 +337,6 @@ public static IEnumerable Pbkdf2_Rfc6070_Vectors() // password (P), salt (S), iterations (c), expected (DK) yield return new object[] { "password", "salt", 1, "0c60c80f961f0e71f3a9b524af6012062fe037a6" }; yield return new object[] { "password", "salt", 2, "ea6c014dc72d6f8ccd1ed92ace1d41f0d8de8957" }; - yield return new object[] { "password", "salt", 16777216, "eefe3d61cd4da4e4e9945b3d6ba2158c2634e984" }; yield return new object[] { "passwordPASSWORDpassword", "saltSALTsaltSALTsaltSALTsaltSALTsalt", 4096, "3d2eec4fe41c849b80c8d83662c0e44a8b291a964cf2f07038" }; yield return new object[] { "pass\0word", "sa\0lt", 4096, "56fa6aa75548099dcc37d7f03425e0c3" }; } From b98a6793b028c657eee21862fd04309f170ff5f3 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 17 Feb 2021 13:38:26 -0500 Subject: [PATCH 25/29] Dispose of handles in error conditions --- .../src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index d2916e6584a9f1..a53af0d17b93d2 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -140,6 +140,7 @@ private static unsafe void FillKeyDerivation( if (openStatus != NTSTATUS.STATUS_SUCCESS) { + pbkdf2AlgorithmHandle.Dispose(); CryptographicOperations.ZeroMemory(clearSpan); throw Interop.BCrypt.CreateCryptographicException(openStatus); } @@ -166,6 +167,7 @@ private static unsafe void FillKeyDerivation( if (generateKeyStatus != NTSTATUS.STATUS_SUCCESS) { + keyHandle.Dispose(); throw Interop.BCrypt.CreateCryptographicException(generateKeyStatus); } From bc02881d8a90de0300be70adec909f9058d15fc5 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 17 Feb 2021 13:40:49 -0500 Subject: [PATCH 26/29] Rename buffer descriptors with clearer prefix --- .../Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs | 4 ++-- .../System/Security/Cryptography/ECCng.ImportExport.cs | 2 +- .../Internal/Cryptography/Pbkdf2Implementation.Windows.cs | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs index 6238245c7a21df..374a80eb7c67f8 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Interop.Blobs.cs @@ -229,7 +229,7 @@ internal struct BCRYPT_ECCFULLKEY_BLOB /// /// NCrypt or BCrypt buffer descriptors /// - internal enum NCryptBCryptBufferDescriptors : int + internal enum CngBufferDescriptors : int { KDF_HASH_ALGORITHM = 0, KDF_SECRET_PREPEND = 1, @@ -258,7 +258,7 @@ internal enum NCryptBCryptBufferDescriptors : int internal struct BCryptBuffer { internal int cbBuffer; // Length of buffer, in bytes - internal NCryptBCryptBufferDescriptors BufferType; // Buffer type + internal CngBufferDescriptors BufferType; // Buffer type internal IntPtr pvBuffer; // Pointer to buffer } diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs index cf632972eb8fbe..5e394540dad205 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs @@ -474,7 +474,7 @@ internal static SafeNCryptKeyHandle ImportKeyBlob( descPtr = Marshal.AllocHGlobal(Marshal.SizeOf(desc)); buffPtr = Marshal.AllocHGlobal(Marshal.SizeOf(buff)); buff.cbBuffer = (curveName.Length + 1) * 2; // Add 1 for null terminator - buff.BufferType = Interop.BCrypt.NCryptBCryptBufferDescriptors.NCRYPTBUFFER_ECC_CURVE_NAME; + buff.BufferType = Interop.BCrypt.CngBufferDescriptors.NCRYPTBUFFER_ECC_CURVE_NAME; buff.pvBuffer = safeCurveName.DangerousGetHandle(); Marshal.StructureToPtr(buff, buffPtr, false); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index a53af0d17b93d2..c8293ed12839c9 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -9,7 +9,7 @@ using BCryptAlgPseudoHandle = Interop.BCrypt.BCryptAlgPseudoHandle; using BCryptBuffer = Interop.BCrypt.BCryptBuffer; using BCryptOpenAlgorithmProviderFlags = Interop.BCrypt.BCryptOpenAlgorithmProviderFlags; -using NCryptBCryptBufferDescriptors = Interop.BCrypt.NCryptBCryptBufferDescriptors; +using CngBufferDescriptors = Interop.BCrypt.CngBufferDescriptors; using NTSTATUS = Interop.BCrypt.NTSTATUS; namespace Internal.Cryptography @@ -181,15 +181,15 @@ private static unsafe void FillKeyDerivation( fixed (byte* pDestination = destination) { Span buffers = stackalloc BCryptBuffer[3]; - buffers[0].BufferType = NCryptBCryptBufferDescriptors.KDF_ITERATION_COUNT; + buffers[0].BufferType = CngBufferDescriptors.KDF_ITERATION_COUNT; buffers[0].pvBuffer = (IntPtr)(&kdfIterations); buffers[0].cbBuffer = sizeof(ulong); - buffers[1].BufferType = NCryptBCryptBufferDescriptors.KDF_SALT; + buffers[1].BufferType = CngBufferDescriptors.KDF_SALT; buffers[1].pvBuffer = (IntPtr)pSalt; buffers[1].cbBuffer = salt.Length; - buffers[2].BufferType = NCryptBCryptBufferDescriptors.KDF_HASH_ALGORITHM; + buffers[2].BufferType = CngBufferDescriptors.KDF_HASH_ALGORITHM; buffers[2].pvBuffer = (IntPtr)pHashAlgorithmName; // C# spec: "A char* value produced by fixing a string instance always points to a null-terminated string" From 8f00cf66dfdc84afd8ecf6c206a12cba7be0df9c Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 17 Feb 2021 14:39:09 -0500 Subject: [PATCH 27/29] Refactor input validation. Checks that inputs are valid before doing any allocations. --- .../Rfc2898DeriveBytes.OneShot.cs | 65 +++++++++++++++---- 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs index 2c78b5cd54c29a..205bb91860c1fc 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/Rfc2898DeriveBytes.OneShot.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using System.Text; using Internal.Cryptography; @@ -80,11 +81,15 @@ public static byte[] Pbkdf2( HashAlgorithmName hashAlgorithm, int outputLength) { + if (iterations <= 0) + throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); if (outputLength < 0) throw new ArgumentOutOfRangeException(nameof(outputLength), SR.ArgumentOutOfRange_NeedNonNegNum); + ValidateHashAlgorithm(hashAlgorithm); + byte[] result = new byte[outputLength]; - Pbkdf2(password, salt, result, iterations, hashAlgorithm); + Pbkdf2Core(password, salt, result, iterations, hashAlgorithm); return result; } @@ -115,6 +120,11 @@ public static void Pbkdf2( int iterations, HashAlgorithmName hashAlgorithm) { + if (iterations <= 0) + throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); + + ValidateHashAlgorithm(hashAlgorithm); + Pbkdf2Core(password, salt, destination, iterations, hashAlgorithm); } @@ -205,9 +215,13 @@ public static byte[] Pbkdf2( { if (outputLength < 0) throw new ArgumentOutOfRangeException(nameof(outputLength), SR.ArgumentOutOfRange_NeedNonNegNum); + if (iterations <= 0) + throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); + + ValidateHashAlgorithm(hashAlgorithm); byte[] result = new byte[outputLength]; - Pbkdf2(password, salt, result, iterations, hashAlgorithm); + Pbkdf2Core(password, salt, result, iterations, hashAlgorithm); return result; } @@ -246,6 +260,29 @@ public static void Pbkdf2( int iterations, HashAlgorithmName hashAlgorithm) { + if (iterations <= 0) + throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); + + ValidateHashAlgorithm(hashAlgorithm); + + Pbkdf2Core(password, salt, destination, iterations, hashAlgorithm); + } + + private static void Pbkdf2Core( + ReadOnlySpan password, + ReadOnlySpan salt, + Span destination, + int iterations, + HashAlgorithmName hashAlgorithm) + { + Debug.Assert(hashAlgorithm.Name is not null); + Debug.Assert(iterations > 0); + + if (destination.IsEmpty) + { + return; + } + const int MaxPasswordStackSize = 256; byte[]? rentedPasswordBuffer = null; @@ -259,7 +296,7 @@ public static void Pbkdf2( try { - Pbkdf2Core(passwordBytes, salt, destination, iterations, hashAlgorithm); + Pbkdf2Implementation.Fill(passwordBytes, salt, iterations, hashAlgorithm, destination); } finally { @@ -279,8 +316,19 @@ private static void Pbkdf2Core( int iterations, HashAlgorithmName hashAlgorithm) { - if (iterations <= 0) - throw new ArgumentOutOfRangeException(nameof(iterations), SR.ArgumentOutOfRange_NeedPosNum); + Debug.Assert(hashAlgorithm.Name is not null); + Debug.Assert(iterations > 0); + + if (destination.IsEmpty) + { + return; + } + + Pbkdf2Implementation.Fill(password, salt, iterations, hashAlgorithm, destination); + } + + private static void ValidateHashAlgorithm(HashAlgorithmName hashAlgorithm) + { if (string.IsNullOrEmpty(hashAlgorithm.Name)) throw new ArgumentException(SR.Cryptography_HashAlgorithmNameNullOrEmpty, nameof(hashAlgorithm)); @@ -294,13 +342,6 @@ private static void Pbkdf2Core( { throw new CryptographicException(SR.Format(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithmName)); } - - if (destination.IsEmpty) - { - return; - } - - Pbkdf2Implementation.Fill(password, salt, iterations, hashAlgorithm, destination); } } } From b2199b11a7b0d2be422d005feb5206cf99e4856b Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 17 Feb 2021 14:52:46 -0500 Subject: [PATCH 28/29] Test for large passwords and salts. --- .../tests/Rfc2898OneShotTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs index 7e470192aac3fb..12f2fd03d5fff2 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/tests/Rfc2898OneShotTests.cs @@ -287,6 +287,8 @@ public static void Pbkdf2_Rfc6070_HighIterations() public static IEnumerable Pbkdf2_PasswordBytes_Compare_Data() { + string largeInputHex = new string('A', 8192); // 8192 hex characters = 4096 bytes. + foreach (HashAlgorithmName hashAlgorithm in SupportedHashAlgorithms) { // hashAlgorithm, length, iterations, passwordHex, saltHex @@ -295,6 +297,7 @@ public static IEnumerable Pbkdf2_PasswordBytes_Compare_Data() yield return new object[] { hashAlgorithm.Name, 257, 257, "", s_salt.ByteArrayToHex() }; yield return new object[] { hashAlgorithm.Name, 257, 257, "D8D8D8D8D8D8D8D8", "D8D8D8D8D8D8D8D8" }; yield return new object[] { hashAlgorithm.Name, 257, 257, "0000000000000000", "0000000000000000" }; + yield return new object[] { hashAlgorithm.Name, 257, 257, largeInputHex, largeInputHex }; } // Test around HMAC SHA1 and SHA256 block boundaries From b881b3a3a34642db97f4cf8e8b41a5effa6b4feb Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 19 Feb 2021 13:03:21 -0500 Subject: [PATCH 29/29] Use asserts to make what should be pre-validated paths clearer. --- .../Cryptography/Pbkdf2Implementation.OSX.cs | 25 ++++++++++++++----- .../Pbkdf2Implementation.Windows.cs | 23 +++++++++++------ 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs index 1a2b5118cf9293..f588543899e286 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.OSX.cs @@ -19,12 +19,25 @@ public static unsafe void Fill( { Debug.Assert(!destination.IsEmpty); - PAL_HashAlgorithm prfAlgorithm = hashAlgorithmName.Name switch { - "SHA1" => PAL_HashAlgorithm.Sha1, - "SHA256" => PAL_HashAlgorithm.Sha256, - "SHA384" => PAL_HashAlgorithm.Sha384, - "SHA512" => PAL_HashAlgorithm.Sha512, - _ => throw new CryptographicException() // Should have been validated before getting here. + PAL_HashAlgorithm prfAlgorithm; + + switch (hashAlgorithmName.Name) + { + case HashAlgorithmNames.SHA1: + prfAlgorithm = PAL_HashAlgorithm.Sha1; + break; + case HashAlgorithmNames.SHA256: + prfAlgorithm = PAL_HashAlgorithm.Sha256; + break; + case HashAlgorithmNames.SHA384: + prfAlgorithm = PAL_HashAlgorithm.Sha384; + break; + case HashAlgorithmNames.SHA512: + prfAlgorithm = PAL_HashAlgorithm.Sha512; + break; + default: + Debug.Fail($"Unexpected hash algorithm '{hashAlgorithmName.Name}'"); + throw new CryptographicException(); }; Interop.AppleCrypto.Pbkdf2(prfAlgorithm, password, salt, iterations, destination); diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs index c8293ed12839c9..93c0bd55ebb894 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Pbkdf2Implementation.Windows.cs @@ -102,6 +102,7 @@ private static unsafe void FillKeyDerivation( hashBufferSize = SHA512.HashData(password, hashBuffer); break; default: + Debug.Fail($"Unexpected hash algorithm '{hashAlgorithmName}'"); throw new CryptographicException(); } @@ -260,13 +261,21 @@ private static unsafe void FillDeriveKeyPBKDF2( } } - private static int GetHashBlockSize(string hashAlgorithmName) => hashAlgorithmName switch { + private static int GetHashBlockSize(string hashAlgorithmName) + { // Block sizes per NIST FIPS pub 180-4. - HashAlgorithmNames.SHA1 => 512 / 8, - HashAlgorithmNames.SHA256 => 512 / 8, - HashAlgorithmNames.SHA384 => 1024 / 8, - HashAlgorithmNames.SHA512 => 1024 / 8, - _ => throw new CryptographicException(), // Should have been validated before getting here. - }; + switch (hashAlgorithmName) + { + case HashAlgorithmNames.SHA1: + case HashAlgorithmNames.SHA256: + return 512 / 8; + case HashAlgorithmNames.SHA384: + case HashAlgorithmNames.SHA512: + return 1024 / 8; + default: + Debug.Fail($"Unexpected hash algorithm '{hashAlgorithmName}'"); + throw new CryptographicException(); + } + } } }