From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.8615.1597321321418034389 for ; Thu, 13 Aug 2020 05:22:01 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1B2BC1063; Thu, 13 Aug 2020 05:22:01 -0700 (PDT) Received: from [192.168.178.54] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CD9C83F6CF; Thu, 13 Aug 2020 05:21:59 -0700 (PDT) Subject: Re: [PATCH v6 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool To: matthewfcarlson@gmail.com, devel@edk2.groups.io Cc: Jiewen Yao , Jian J Wang , Xiaoyu Lu References: <20200812224338.287-1-matthewfcarlson@gmail.com> <20200812224338.287-6-matthewfcarlson@gmail.com> From: "Ard Biesheuvel" Message-ID: <2e4358bb-15cc-0ab9-30f3-a156f11b9206@arm.com> Date: Thu, 13 Aug 2020 14:21:58 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200812224338.287-6-matthewfcarlson@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 8/13/20 12:43 AM, matthewfcarlson@gmail.com wrote: > From: Matthew Carlson > > Ref: https://github.com/tianocore/edk2/pull/845 > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871 > > Changes OpenSSL to no longer depend on TimerLib and instead use RngLib. > This allows platforms to decide for themsevles what sort of entropy source > they provide to OpenSSL and TlsLib. > > Cc: Ard Biesheuvel > Cc: Jiewen Yao > Cc: Jian J Wang > Cc: Xiaoyu Lu > Signed-off-by: Matthew Carlson With the coding style/whitespace violations fixed: Acked-by: Ard Biesheuvel > --- > CryptoPkg/Library/OpensslLib/rand_pool.c | 203 ++------------------ > CryptoPkg/Library/OpensslLib/rand_pool_noise.c | 29 --- > CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c | 43 ----- > CryptoPkg/CryptoPkg.dsc | 1 + > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 15 +- > CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 15 +- > CryptoPkg/Library/OpensslLib/rand_pool_noise.h | 29 --- > 7 files changed, 22 insertions(+), 313 deletions(-) > > diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c b/CryptoPkg/Library/OpensslLib/rand_pool.c > index 9e0179b03490..3da92699fef6 100644 > --- a/CryptoPkg/Library/OpensslLib/rand_pool.c > +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c > @@ -11,53 +11,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include > > #include > -#include > - > -#include "rand_pool_noise.h" > - > -/** > - Get some randomness from low-order bits of GetPerformanceCounter results. > - And combine them to the 64-bit value > - > - @param[out] Rand Buffer pointer to store the 64-bit random value. > - > - @retval TRUE Random number generated successfully. > - @retval FALSE Failed to generate. > -**/ > -STATIC > -BOOLEAN > -EFIAPI > -GetRandNoise64FromPerformanceCounter( > - OUT UINT64 *Rand > - ) > -{ > - UINT32 Index; > - UINT32 *RandPtr; > - > - if (NULL == Rand) { > - return FALSE; > - } > - > - RandPtr = (UINT32 *) Rand; > - > - for (Index = 0; Index < 2; Index ++) { > - *RandPtr = (UINT32) (GetPerformanceCounter () & 0xFF); > - MicroSecondDelay (10); > - RandPtr++; > - } > - > - return TRUE; > -} > +#include > > /** > Calls RandomNumber64 to fill > a buffer of arbitrary size with random bytes. > + This is a shim layer to RngLib. > > @param[in] Length Size of the buffer, in bytes, to fill with. > @param[out] RandBuffer Pointer to the buffer to store the random result. > > - @retval EFI_SUCCESS Random bytes generation succeeded. > - @retval EFI_NOT_READY Failed to request random bytes. > + @retval TRUE Random bytes generation succeeded. > + @retval FALSE Failed to request random bytes. > > **/ > STATIC > @@ -73,17 +38,17 @@ RandGetBytes ( > > Ret = FALSE; > > + if (RandBuffer == NULL) { > + DEBUG((DEBUG_ERROR, "[OPENSSL_RAND_POOL] NULL RandBuffer. No random numbers are generated and your system is not secure\n")); > + ASSERT(FALSE); // Since we can't generate random numbers, we should assert. Otherwise we will just blow up later. > + return Ret; > + } > + > + > while (Length > 0) { > - // > - // Get random noise from platform. > - // If it failed, fallback to PerformanceCounter > - // If you really care about security, you must override > - // GetRandomNoise64FromPlatform. > - // > - Ret = GetRandomNoise64 (&TempRand); > - if (Ret == FALSE) { > - Ret = GetRandNoise64FromPerformanceCounter (&TempRand); > - } > + // Use RngLib to get random number > + Ret = GetRandomNumber64(&TempRand); > + > if (!Ret) { > return Ret; > } > @@ -100,125 +65,6 @@ RandGetBytes ( > return Ret; > } > > -/** > - Creates a 128bit random value that is fully forward and backward prediction resistant, > - suitable for seeding a NIST SP800-90 Compliant. > - This function takes multiple random numbers from PerformanceCounter to ensure reseeding > - and performs AES-CBC-MAC over the data to compute the seed value. > - > - @param[out] SeedBuffer Pointer to a 128bit buffer to store the random seed. > - > - @retval TRUE Random seed generation succeeded. > - @retval FALSE Failed to request random bytes. > - > -**/ > -STATIC > -BOOLEAN > -EFIAPI > -RandGetSeed128 ( > - OUT UINT8 *SeedBuffer > - ) > -{ > - BOOLEAN Ret; > - UINT8 RandByte[16]; > - UINT8 Key[16]; > - UINT8 Ffv[16]; > - UINT8 Xored[16]; > - UINT32 Index; > - UINT32 Index2; > - AES_KEY AESKey; > - > - // > - // Chose an arbitrary key and zero the feed_forward_value (FFV) > - // > - for (Index = 0; Index < 16; Index++) { > - Key[Index] = (UINT8) Index; > - Ffv[Index] = 0; > - } > - > - AES_set_encrypt_key (Key, 16 * 8, &AESKey); > - > - // > - // Perform CBC_MAC over 32 * 128 bit values, with 10us gaps between 128 bit value > - // The 10us gaps will ensure multiple reseeds within the system time with a large > - // design margin. > - // > - for (Index = 0; Index < 32; Index++) { > - MicroSecondDelay (10); > - Ret = RandGetBytes (16, RandByte); > - if (!Ret) { > - return Ret; > - } > - > - // > - // Perform XOR operations on two 128-bit value. > - // > - for (Index2 = 0; Index2 < 16; Index2++) { > - Xored[Index2] = RandByte[Index2] ^ Ffv[Index2]; > - } > - > - AES_encrypt (Xored, Ffv, &AESKey); > - } > - > - for (Index = 0; Index < 16; Index++) { > - SeedBuffer[Index] = Ffv[Index]; > - } > - > - return Ret; > -} > - > -/** > - Generate high-quality entropy source. > - > - @param[in] Length Size of the buffer, in bytes, to fill with. > - @param[out] Entropy Pointer to the buffer to store the entropy data. > - > - @retval EFI_SUCCESS Entropy generation succeeded. > - @retval EFI_NOT_READY Failed to request random data. > - > -**/ > -STATIC > -BOOLEAN > -EFIAPI > -RandGenerateEntropy ( > - IN UINTN Length, > - OUT UINT8 *Entropy > - ) > -{ > - BOOLEAN Ret; > - UINTN BlockCount; > - UINT8 Seed[16]; > - UINT8 *Ptr; > - > - BlockCount = Length / 16; > - Ptr = (UINT8 *) Entropy; > - > - // > - // Generate high-quality seed for DRBG Entropy > - // > - while (BlockCount > 0) { > - Ret = RandGetSeed128 (Seed); > - if (!Ret) { > - return Ret; > - } > - CopyMem (Ptr, Seed, 16); > - > - BlockCount--; > - Ptr = Ptr + 16; > - } > - > - // > - // Populate the remained data as request. > - // > - Ret = RandGetSeed128 (Seed); > - if (!Ret) { > - return Ret; > - } > - CopyMem (Ptr, Seed, (Length % 16)); > - > - return Ret; > -} > - > /* > * Add random bytes to the pool to acquire requested amount of entropy > * > @@ -238,7 +84,7 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool) > buffer = rand_pool_add_begin(pool, bytes_needed); > > if (buffer != NULL) { > - Ret = RandGenerateEntropy(bytes_needed, buffer); > + Ret = RandGetBytes(bytes_needed, buffer); > if (FALSE == Ret) { > rand_pool_add_end(pool, 0, 0); > } else { > @@ -257,13 +103,8 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool) > */ > int rand_pool_add_nonce_data(RAND_POOL *pool) > { > - struct { > - UINT64 Rand; > - UINT64 TimerValue; > - } data = { 0 }; > - > - RandGetBytes(8, (UINT8 *)&(data.Rand)); > - data.TimerValue = GetPerformanceCounter(); > + UINT8 data[16]; > + RandGetBytes(sizeof(data), data); > > return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0); > } > @@ -275,13 +116,8 @@ int rand_pool_add_nonce_data(RAND_POOL *pool) > */ > int rand_pool_add_additional_data(RAND_POOL *pool) > { > - struct { > - UINT64 Rand; > - UINT64 TimerValue; > - } data = { 0 }; > - > - RandGetBytes(8, (UINT8 *)&(data.Rand)); > - data.TimerValue = GetPerformanceCounter(); > + UINT8 data[16]; > + RandGetBytes(sizeof(data), data); > > return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0); > } > @@ -313,4 +149,3 @@ void rand_pool_cleanup(void) > void rand_pool_keep_random_devices_open(int keep) > { > } > - > diff --git a/CryptoPkg/Library/OpensslLib/rand_pool_noise.c b/CryptoPkg/Library/OpensslLib/rand_pool_noise.c > deleted file mode 100644 > index 212834e27acc..000000000000 > --- a/CryptoPkg/Library/OpensslLib/rand_pool_noise.c > +++ /dev/null > @@ -1,29 +0,0 @@ > -/** @file > - Provide rand noise source. > - > -Copyright (c) 2019, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent > - > -**/ > - > -#include > - > -/** > - Get 64-bit noise source > - > - @param[out] Rand Buffer pointer to store 64-bit noise source > - > - @retval FALSE Failed to generate > -**/ > -BOOLEAN > -EFIAPI > -GetRandomNoise64 ( > - OUT UINT64 *Rand > - ) > -{ > - // > - // Return FALSE will fallback to use PerformanceCounter to > - // generate noise. > - // > - return FALSE; > -} > diff --git a/CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c b/CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c > deleted file mode 100644 > index 4158106231fd..000000000000 > --- a/CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c > +++ /dev/null > @@ -1,43 +0,0 @@ > -/** @file > - Provide rand noise source. > - > -Copyright (c) 2019, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent > - > -**/ > - > -#include > -#include > -#include > - > -/** > - Get 64-bit noise source > - > - @param[out] Rand Buffer pointer to store 64-bit noise source > - > - @retval TRUE Get randomness successfully. > - @retval FALSE Failed to generate > -**/ > -BOOLEAN > -EFIAPI > -GetRandomNoise64 ( > - OUT UINT64 *Rand > - ) > -{ > - UINT32 Index; > - UINT32 *RandPtr; > - > - if (NULL == Rand) { > - return FALSE; > - } > - > - RandPtr = (UINT32 *)Rand; > - > - for (Index = 0; Index < 2; Index ++) { > - *RandPtr = (UINT32) ((AsmReadTsc ()) & 0xFF); > - RandPtr++; > - MicroSecondDelay (10); > - } > - > - return TRUE; > -} > diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc > index 1af78468a19c..0490eeb7e22f 100644 > --- a/CryptoPkg/CryptoPkg.dsc > +++ b/CryptoPkg/CryptoPkg.dsc > @@ -60,6 +60,7 @@ > BaseCryptLib|CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf > TlsLib|CryptoPkg/Library/TlsLibNull/TlsLibNull.inf > HashApiLib|CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.inf > + RngLib|MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf > > [LibraryClasses.ARM, LibraryClasses.AARCH64] > # > diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf > index dbbe5386a10c..4baad565564c 100644 > --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf > +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf > @@ -571,22 +571,9 @@ > $(OPENSSL_PATH)/ssl/statem/statem_local.h > # Autogenerated files list ends here > buildinf.h > - rand_pool_noise.h > ossl_store.c > rand_pool.c > > -[Sources.Ia32] > - rand_pool_noise_tsc.c > - > -[Sources.X64] > - rand_pool_noise_tsc.c > - > -[Sources.ARM] > - rand_pool_noise.c > - > -[Sources.AARCH64] > - rand_pool_noise.c > - > [Packages] > MdePkg/MdePkg.dec > CryptoPkg/CryptoPkg.dec > @@ -594,7 +581,7 @@ > [LibraryClasses] > BaseLib > DebugLib > - TimerLib > + RngLib > PrintLib > > [LibraryClasses.ARM] > diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf > index 616ccd9f62d1..3557711bd85a 100644 > --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf > +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf > @@ -520,22 +520,9 @@ > $(OPENSSL_PATH)/crypto/x509v3/v3_admis.h > # Autogenerated files list ends here > buildinf.h > - rand_pool_noise.h > ossl_store.c > rand_pool.c > > -[Sources.Ia32] > - rand_pool_noise_tsc.c > - > -[Sources.X64] > - rand_pool_noise_tsc.c > - > -[Sources.ARM] > - rand_pool_noise.c > - > -[Sources.AARCH64] > - rand_pool_noise.c > - > [Packages] > MdePkg/MdePkg.dec > CryptoPkg/CryptoPkg.dec > @@ -543,7 +530,7 @@ > [LibraryClasses] > BaseLib > DebugLib > - TimerLib > + RngLib > PrintLib > > [LibraryClasses.ARM] > diff --git a/CryptoPkg/Library/OpensslLib/rand_pool_noise.h b/CryptoPkg/Library/OpensslLib/rand_pool_noise.h > deleted file mode 100644 > index 75acc686a9f1..000000000000 > --- a/CryptoPkg/Library/OpensslLib/rand_pool_noise.h > +++ /dev/null > @@ -1,29 +0,0 @@ > -/** @file > - Provide rand noise source. > - > -Copyright (c) 2019, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent > - > -**/ > - > -#ifndef __RAND_POOL_NOISE_H__ > -#define __RAND_POOL_NOISE_H__ > - > -#include > - > -/** > - Get 64-bit noise source. > - > - @param[out] Rand Buffer pointer to store 64-bit noise source > - > - @retval TRUE Get randomness successfully. > - @retval FALSE Failed to generate > -**/ > -BOOLEAN > -EFIAPI > -GetRandomNoise64 ( > - OUT UINT64 *Rand > - ); > - > - > -#endif // __RAND_POOL_NOISE_H__ >