[edk2-devel] [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib
Ard Biesheuvel
ard.biesheuvel at arm.com
Thu Aug 20 08:05:16 UTC 2020
On 8/19/20 9:37 PM, matthewfcarlson at gmail.com wrote:
> From: Matthew Carlson <macarl at microsoft.com>
>
> Added a new RngLib that provides random numbers from the TimerLib
> using the performance counter. This is meant to be used for OpenSSL
> to replicate past behavior. This should not be used in production as
> a real source of entropy.
>
> Ref: https://github.com/tianocore/edk2/pull/845
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Cc: Liming Gao <liming.gao at intel.com>
> Cc: Zhiguang Liu <zhiguang.liu at intel.com>
> Signed-off-by: Matthew Carlson <matthewfcarlson at gmail.com>
Hi Matthew,
I am still not convinced that this approach is sound.
What happens is that we take the least significant byte of the counter,
wait for it to assume the next value in the sequence, capture it again,
etc etc
So for a 4 byte random number, we end up with
{counter, counter + N, counter + 2 * N, counter + 3 * N }
for some platform specific value of N.
How is that any better than simply returning 32 bits of the performance
counter?
Some nits below.
> ---
> MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c | 191 ++++++++++++++++++++
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf | 36 ++++
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni | 15 ++
> MdePkg/MdePkg.dsc | 3 +-
> 4 files changed, 244 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
> new file mode 100644
> index 000000000000..c72aa335823d
> --- /dev/null
> +++ b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
> @@ -0,0 +1,191 @@
> +/** @file
> + BaseRng Library that uses the TimerLib to provide reasonably random numbers.
> + Do not use this on a production system.
> +
> + Copyright (c) Microsoft Corporation.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Base.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/TimerLib.h>
> +
> +/**
> + * Using the TimerLib GetPerformanceCounterProperties() we delay
> + * for enough time for the PerformanceCounter to increment.
> + * Depending on your system
> + *
> + * If the return value from GetPerformanceCounterProperties (TimerLib)
> + * is zero, this function will not delay and attempt to assert.
> + */
> +STATIC
> +UINT32
> +CalculateMinimumDecentDelayInMicroseconds (
> + VOID
> + )
> +{
> + UINT64 StartValue;
> + UINT64 EndValue;
> + UINT64 CounterHz;
> + UINT64 MinumumDelayInMicroSeconds;
> +
> + // Get the counter properties
> + CounterHz = GetPerformanceCounterProperties (&StartValue, &EndValue);
> + // Make sure we won't divide by zero
> + if (CounterHz == 0) {
> + ASSERT(CounterHz != 0); // Assert so the developer knows something is wrong
Space before (
> + return;
> + }
> + // Calculate the minimum delay based on 1.5 microseconds divided by the hertz.
> + // We calculate the length of a cycle (1/CounterHz) and multiply it by 1.5 microseconds
> + // This ensures that the performance counter has increased by at least one
> + return (UINT32)(MAX(DivU64x64Remainder(1500000 / CounterHz, NULL), 1));
Space before (
> +}
> +
> +
> +/**
> + Generates a 16-bit random number.
> +
> + if Rand is NULL, then ASSERT().
> +
> + @param[out] Rand Buffer pointer to store the 16-bit random value.
> +
> + @retval TRUE Random number generated successfully.
> + @retval FALSE Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber16 (
> + OUT UINT16 *Rand
> + )
> +{
> + UINT32 Index;
> + UINT8 *RandPtr;
Please align the * with the other variable names
> + UINT32 DelayInMicroSeconds;
> +
> + ASSERT (Rand != NULL);
> +
> + if (Rand == NULL) {
> + return FALSE;
> + }
> + DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
> + RandPtr = (UINT8*)Rand;
> + // Get 2 bytes of random ish data
> + for (Index = 0; Index < 2; Index ++) {
> + *RandPtr = (UINT8)(GetPerformanceCounter () & 0xFF);
> + // Delay to give the performance counter a chance to change
> + MicroSecondDelay (DelayInMicroSeconds);
> + RandPtr++;
> + }
> + return TRUE;
> +}
> +
> +/**
> + Generates a 32-bit random number.
> +
> + if Rand is NULL, then ASSERT().
> +
> + @param[out] Rand Buffer pointer to store the 32-bit random value.
> +
> + @retval TRUE Random number generated successfully.
> + @retval FALSE Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber32 (
> + OUT UINT32 *Rand
> + )
> +{
> + UINT32 Index;
> + UINT8 *RandPtr;
Same here
> + UINT32 DelayInMicroSeconds;
> +
> + ASSERT (Rand != NULL);
> +
> + if (NULL == Rand) {
> + return FALSE;
> + }
> +
> + RandPtr = (UINT8 *) Rand;
No space after () cast operator, please
> + DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
> + // Get 4 bytes of random ish data
> + for (Index = 0; Index < 4; Index ++) {
> + *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
> + // Delay to give the performance counter a chance to change
> + MicroSecondDelay (DelayInMicroSeconds);
> + RandPtr++;
> + }
> + return TRUE;
> +}
> +
> +/**
> + Generates a 64-bit random number.
> +
> + if Rand is NULL, then ASSERT().
> +
> + @param[out] Rand Buffer pointer to store the 64-bit random value.
> +
> + @retval TRUE Random number generated successfully.
> + @retval FALSE Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber64 (
> + OUT UINT64 *Rand
> + )
> +{
> + UINT32 Index;
> + UINT8 *RandPtr;
> + UINT32 DelayInMicroSeconds;
> +
> + ASSERT (Rand != NULL);
> +
> + if (NULL == Rand) {
> + return FALSE;
> + }
> +
> + RandPtr = (UINT8 *) Rand;
> + DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
> + // Get 8 bytes of random ish data
> + for (Index = 0; Index < 8; Index ++) {
> + *RandPtr = (UINT8)(GetPerformanceCounter () & 0xFF);
> + // Delay to give the performance counter a chance to change
> + MicroSecondDelay (DelayInMicroSeconds);
> + RandPtr++;
> + }
> +
> + return TRUE;
> +}
> +
> +/**
> + Generates a 128-bit random number.
> +
> + if Rand is NULL, then ASSERT().
> +
> + @param[out] Rand Buffer pointer to store the 128-bit random value.
> +
> + @retval TRUE Random number generated successfully.
> + @retval FALSE Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber128 (
> + OUT UINT64 *Rand
> + )
> +{
> + ASSERT (Rand != NULL);
> + // This should take around 80ms
> +
> + // Read first 64 bits
> + if (!GetRandomNumber64 (Rand)) {
> + return FALSE;
> + }
> +
> + // Read second 64 bits
> + return GetRandomNumber64 (++Rand);
> +}
> diff --git a/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> new file mode 100644
> index 000000000000..c499e5327351
> --- /dev/null
> +++ b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> @@ -0,0 +1,36 @@
> +## @file
> +# Instance of RNG (Random Number Generator) Library.
> +#
> +# BaseRng Library that uses the TimerLib to provide reasonably random numbers.
> +# Do NOT use this on a production system as this uses the system performance
> +# counter rather than a true source of random in addition to having a weak
> +# random algorithm. This is provided primarily as a source of entropy for
> +# OpenSSL for platforms that do not have a good built in RngLib as this
> +# emulates what was done before (though it isn't perfect).
> +#
> +# Copyright (c) Microsoft Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 1.27
> + BASE_NAME = BaseRngLibTimerLib
> + MODULE_UNI_FILE = BaseRngLibTimerLib.uni
> + FILE_GUID = 74950C45-10FC-4AB5-B114-49C87C17409B
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = RngLib
> + CONSTRUCTOR = BaseRngLibConstructor
> +
> +[Sources]
> + RngLibTimer.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + TimerLib
> diff --git a/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni
> new file mode 100644
> index 000000000000..fde24b9f0107
> --- /dev/null
> +++ b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni
> @@ -0,0 +1,15 @@
> +// @file
> +// Instance of RNG (Random Number Generator) Library.
> +//
> +// RngLib that uses TimerLib's performance counter to provide random numbers.
> +//
> +// Copyright (c) Microsoft Corporation.
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +
> +
> +#string STR_MODULE_ABSTRACT #language en-US "Instance of RNG Library"
> +
> +#string STR_MODULE_DESCRIPTION #language en-US "BaseRng Library that uses the TimerLib to provide low-entropy random numbers"
> +
> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
> index 472fa3777412..d7ba3a730909 100644
> --- a/MdePkg/MdePkg.dsc
> +++ b/MdePkg/MdePkg.dsc
> @@ -62,6 +62,8 @@
> MdePkg/Library/BasePostCodeLibPort80/BasePostCodeLibPort80.inf
> MdePkg/Library/BasePrintLib/BasePrintLib.inf
> MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
> + MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> + MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf
> MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
> MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> @@ -69,7 +71,6 @@
> MdePkg/Library/BaseUefiDecompressLib/BaseUefiTianoCustomDecompressLib.inf
> MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> - MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf
>
> MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#64486): https://edk2.groups.io/g/devel/message/64486
Mute This Topic: https://groups.io/mt/76294211/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list