public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib
@ 2020-08-19 19:37 Matthew Carlson
  2020-08-19 19:37 ` [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-08-19 19:37 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Anthony Perard, Jiewen Yao, Jian J Wang,
	Julien Grall, Jordan Justen, Laszlo Ersek, Liming Gao,
	Leif Lindholm, Michael D Kinney, Xiaoyu Lu, Zhiguang Liu,
	Sean Brogan, Matthew Carlson

From: Matthew Carlson <macarl@microsoft.com>

Hello all,

This patch contains a fix for Bugzilla 1871.
There's been a good bit of community discussion around the topic,
so below follows a general overview of the discussion and what this patch does.

This is the seventh iteration of this patch series, focused on code style and a
few functions being renamed to comply with style.

Back in Devel message#40590 (https://edk2.groups.io/g/devel/message/40590)
around the patch series that updates OpenSSL to 1.1.1b, a comment was made
that suggested that platforms be in charge of the entropy/randomness that
is provided to OpenSSL as currently the entropry source seems to be a
hand-rolled random number generator that uses the PerformanceCounter from
TimerLib. This causes OpenSSL to depend on TimerLib, which is often platform
specific. In addition to being a potentially weaker source of randomness,
this also poses a challenge to compile BaseCryptLibOnProtocol with a platform-
agnostic version of TimerLib that works universally.

The solution here is to allow platform to specify their source of entropy in
addition to providing two new RngLibs: one that uses the TimerLib as well as
one that uses RngProtocol to provide randomness. Then the decision to use
RDRAND or other entropy sources is up to the platform. Mixing various entropy
sources is the onus of the platform. It has been suggested on Devel#40590 and
BZ#1871 that there should be mixing of the PerformanceCounter and RDRAND using
something similar to the yarrow alogirthm that FreeBSD uses for example. This
patch series doesn't offer an RngLib that offers that sort of mixing as the
ultimate source of random is defined by the platform.

This patch series offers three benefits:
1. Dependency reduction: Removes the need for a platform specific timer
library.  We publish a single binary used on numerous platforms for
crypto and the introduced timer lib dependency caused issues because we
could not fulfill our platform needs with one library instance.

2. Code maintenance: Removing this additional code and leveraging an existing
library within Edk2 means less code to maintain.

3. Platform defined quality: A platform can choose which instance to use and
the implications of that instance.

This patch series seeks to address five seperate issues.
  1) Use RngLib interface to generate random entropy in rand_pool
  2) Remove dependency on TimerLib in OpensslLib
  3) Add a new version of RngLib implemented by TimerLib
  4) Add a new version of RngLib implemented by EFI_RNG_PROTOCOL
  5) Add RngLib to platforms in EDK2 such as ArmVirtPkg and OvmfPkg

Since this changes the dependencies of OpenSSL, this has the potential of being
a breaking change for platforms in edk2-platforms. The easiest solution is just
to use the RngLib that uses the TimerLib as this closely mimics the behavior of
OpenSSL prior to this patch series. There is also a null version of RngLib for
CI environments that need this change
(https://edk2.groups.io/g/devel/message/50432). Though it should be pointed out
that in CI environments, the null version of BaseCryptLib or OpenSSL should be
used.

In addition, it has been suggested that
1) Add AsmRdSeed to BaseLib.
2) Update BaseRngLib to use AsmRdSeed() for the random number,
if RdSeed is supported (CPUID BIT18)

However, this is largely out of scope for this particular patch series and
will likely need to be in a follow-up series later.

It is my understanding that the OpenSSL code uses the values provided as a
randomness pool rather than a seed or random numbers itself, so the
requirements for randomness are not quite as stringent as other applications.

For the ArmVirtPkg and OvmfPkg platforms, the patch series here just adds in
the TimerLib based RngLib as that is similar to the functionality of before.
It is added as a common library so any custom RngLib defined in the DSC
should take precedence over the TimerLibRngLib.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Julien Grall <julien@xen.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>

Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>

Matthew Carlson (5):
  MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
  OvmfPkg: Add RngLib based on TimerLib for Crypto
  ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg
  CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

 CryptoPkg/Library/OpensslLib/rand_pool.c                 | 265 +++++---------------
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c           |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c       |  43 ----
 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c          | 191 ++++++++++++++
 MdePkg/Library/DxeRngLib/DxeRngLib.c                     | 206 +++++++++++++++
 ArmVirtPkg/ArmVirt.dsc.inc                               |   1 +
 CryptoPkg/CryptoPkg.dsc                                  |   1 +
 CryptoPkg/Library/OpensslLib/OpensslLib.inf              |  15 +-
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf        |  15 +-
 CryptoPkg/Library/OpensslLib/rand_pool_noise.h           |  29 ---
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  36 +++
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  15 ++
 MdePkg/Library/DxeRngLib/DxeRngLib.inf                   |  38 +++
 MdePkg/Library/DxeRngLib/DxeRngLib.uni                   |  15 ++
 MdePkg/MdePkg.dsc                                        |   5 +-
 OvmfPkg/Bhyve/BhyvePkgX64.dsc                            |   1 +
 OvmfPkg/OvmfPkgIa32.dsc                                  |   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                               |   1 +
 OvmfPkg/OvmfPkgX64.dsc                                   |   1 +
 OvmfPkg/OvmfXen.dsc                                      |   1 +
 20 files changed, 574 insertions(+), 335 deletions(-)
 delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.c
 delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c
 create mode 100644 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
 create mode 100644 MdePkg/Library/DxeRngLib/DxeRngLib.c
 delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.h
 create mode 100644 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 create mode 100644 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni
 create mode 100644 MdePkg/Library/DxeRngLib/DxeRngLib.inf
 create mode 100644 MdePkg/Library/DxeRngLib/DxeRngLib.uni

-- 
2.28.0.windows.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  2020-08-19 19:37 [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
@ 2020-08-19 19:37 ` Matthew Carlson
  2020-08-20  8:05   ` Ard Biesheuvel
  2020-08-20 15:21   ` Michael D Kinney
  2020-08-19 19:37 ` [PATCH v8 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe Matthew Carlson
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-08-19 19:37 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Michael D Kinney, Liming Gao, Zhiguang Liu,
	Matthew Carlson

From: Matthew Carlson <macarl@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@arm.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
---
 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
+    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));
+}
+
+
+/**
+  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;
+  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;
+  UINT32  DelayInMicroSeconds;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+    return FALSE;
+  }
+
+  RandPtr = (UINT8 *) Rand;
+  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
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
  2020-08-19 19:37 [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
  2020-08-19 19:37 ` [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
@ 2020-08-19 19:37 ` Matthew Carlson
  2020-08-20  8:09   ` Ard Biesheuvel
  2020-08-19 19:37 ` [PATCH v8 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto Matthew Carlson
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Matthew Carlson @ 2020-08-19 19:37 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Michael D Kinney, Liming Gao, Zhiguang Liu,
	Matthew Carlson

From: Matthew Carlson <macarl@microsoft.com>

This adds a RngLib that uses the RngProtocol to provide randomness.
This means that the RngLib is meant to be used with DXE_DRIVERS.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
---
 MdePkg/Library/DxeRngLib/DxeRngLib.c   | 206 ++++++++++++++++++++
 MdePkg/Library/DxeRngLib/DxeRngLib.inf |  38 ++++
 MdePkg/Library/DxeRngLib/DxeRngLib.uni |  15 ++
 MdePkg/MdePkg.dsc                      |   4 +-
 4 files changed, 262 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c b/MdePkg/Library/DxeRngLib/DxeRngLib.c
new file mode 100644
index 000000000000..0bd6585357b5
--- /dev/null
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c
@@ -0,0 +1,206 @@
+/** @file
+ Provides an implementation of the library class RngLib that uses the Rng protocol.
+
+ Copyright (c) Microsoft Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include <Uefi.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DebugLib.h>
+#include <Library/RngLib.h>
+#include <Protocol/Rng.h>
+
+/**
+Routine Description:
+
+    Generates a random number via the NIST
+    800-9A algorithm.  Refer to
+    http://csrc.nist.gov/groups/STM/cavp/documents/drbg/DRBGVS.pdf
+    for more information.
+
+    Arguments:
+
+    Buffer      -- Buffer to receive the random number.
+    BufferSize  -- Number of bytes in Buffer.
+
+Return Value:
+
+    EFI_SUCCESS or underlying failure code.
+
+**/
+STATIC
+EFI_STATUS
+GenerateRandomNumberViaNist800Algorithm (
+  OUT UINT8 *Buffer,
+  IN  UINTN  BufferSize
+  )
+{
+  EFI_STATUS        Status;
+  EFI_RNG_PROTOCOL *RngProtocol;
+
+  RngProtocol = NULL;
+
+  if (Buffer == NULL) {
+      DEBUG((DEBUG_ERROR, "[%a] Buffer == NULL.\n", __FUNCTION__));
+      return EFI_INVALID_PARAMETER;
+  }
+
+  Status = gBS->LocateProtocol (&gEfiRngProtocolGuid, NULL, (VOID **)&RngProtocol);
+  if (EFI_ERROR (Status) || RngProtocol == NULL) {
+      DEBUG((DEBUG_ERROR, "%a: Could not locate RNG prototocol, Status = %r\n", __FUNCTION__, Status));
+      return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, &gEfiRngAlgorithmSp80090Ctr256Guid, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm CTR-256 - Status = %r\n", __FUNCTION__, Status));
+  if(!EFI_ERROR(Status)) {
+    return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, &gEfiRngAlgorithmSp80090Hmac256Guid, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm HMAC-256 - Status = %r\n", __FUNCTION__, Status));
+  if(!EFI_ERROR(Status)) {
+    return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, &gEfiRngAlgorithmSp80090Hash256Guid, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", __FUNCTION__, Status));
+  if (!EFI_ERROR(Status)) {
+    return Status;
+  }
+  // If all the other methods have failed, use the default method from the RngProtocol
+  Status = RngProtocol->GetRNG (RngProtocol, NULL, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", __FUNCTION__, Status));
+  if (!EFI_ERROR(Status)) {
+    return Status;
+  }
+  // If we get to this point, we have failed
+  DEBUG((DEBUG_ERROR, "%a: GetRNG() failed, staus = %r\n", __FUNCTION__, Status));
+
+  return Status;
+}// GenerateRandomNumberViaNist800Algorithm()
+
+
+/**
+  Generates a 16-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @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
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL)
+  {
+    return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 2);
+  if (EFI_ERROR (Status))
+  {
+    return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 32-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @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
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL) {
+    return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 4);
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 64-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @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
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL)
+  {
+    return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 8);
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 128-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @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
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL) {
+    return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 16);
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+  return TRUE;
+}
diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.inf b/MdePkg/Library/DxeRngLib/DxeRngLib.inf
new file mode 100644
index 000000000000..f4838aa05b7e
--- /dev/null
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.inf
@@ -0,0 +1,38 @@
+# @file
+# Provides implementation of the library class RngLib that uses the RngProtocol
+#
+# @copyright
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION     = 0x00010017
+  BASE_NAME       = DxeRngLib
+  MODULE_UNI_FILE = DxeRngLib.uni
+  FILE_GUID       = FF9F84C5-A33E-44E3-9BB5-0D654B2D4149
+  MODULE_TYPE     = DXE_DRIVER
+  VERSION_STRING  = 1.0
+  LIBRARY_CLASS   = RngLib|DXE_DRIVER UEFI_APPLICATION UEFI_DRIVER
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[Sources]
+  DxeRngLib.c
+
+[LibraryClasses]
+  DebugLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gEfiRngProtocolGuid                 ## CONSUMES
+
+[Depex]
+  gEfiRngProtocolGuid
+
+[Guids]
+  gEfiRngAlgorithmSp80090Ctr256Guid
+  gEfiRngAlgorithmSp80090Hash256Guid
+  gEfiRngAlgorithmSp80090Hmac256Guid
diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.uni b/MdePkg/Library/DxeRngLib/DxeRngLib.uni
new file mode 100644
index 000000000000..c904e54b6fb0
--- /dev/null
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.uni
@@ -0,0 +1,15 @@
+// @file
+// Instance of RNG (Random Number Generator) Library.
+//
+// RngLib that uses the Rng Protocol 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 Rng Protocol to provide random numbers"
+
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index d7ba3a730909..2c3b7966b086 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -62,8 +62,10 @@
   MdePkg/Library/BasePostCodeLibPort80/BasePostCodeLibPort80.inf
   MdePkg/Library/BasePrintLib/BasePrintLib.inf
   MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
-  MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
+  MdePkg/Library/DxeRngLib/DxeRngLib.inf
   MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf
+  MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
+
   MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto
  2020-08-19 19:37 [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
  2020-08-19 19:37 ` [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
  2020-08-19 19:37 ` [PATCH v8 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe Matthew Carlson
@ 2020-08-19 19:37 ` Matthew Carlson
  2020-08-19 19:37 ` [PATCH v8 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg Matthew Carlson
  2020-08-19 19:37 ` [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
  4 siblings, 0 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-08-19 19:37 UTC (permalink / raw)
  To: devel
  Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Anthony Perard,
	Julien Grall, Matthew Carlson

From: Matthew Carlson <macarl@microsoft.com>

Updates the DSC's for Ovmf based platforms to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
---
 OvmfPkg/Bhyve/BhyvePkgX64.dsc | 1 +
 OvmfPkg/OvmfPkgIa32.dsc       | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc    | 1 +
 OvmfPkg/OvmfPkgX64.dsc        | 1 +
 OvmfPkg/OvmfXen.dsc           | 1 +
 5 files changed, 5 insertions(+)

diff --git a/OvmfPkg/Bhyve/BhyvePkgX64.dsc b/OvmfPkg/Bhyve/BhyvePkgX64.dsc
index 99e214619be0..0bf1acbc8dc8 100644
--- a/OvmfPkg/Bhyve/BhyvePkgX64.dsc
+++ b/OvmfPkg/Bhyve/BhyvePkgX64.dsc
@@ -185,6 +185,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Bhyve/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 133a9a93c071..fa18adeb5c5a 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -189,6 +189,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 338c38db29b5..7456a154168d 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -193,6 +193,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index b80710fbdca4..5bda143fd14d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -193,6 +193,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 37b63a874067..e562abd7175d 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -179,6 +179,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
   AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg
  2020-08-19 19:37 [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
                   ` (2 preceding siblings ...)
  2020-08-19 19:37 ` [PATCH v8 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto Matthew Carlson
@ 2020-08-19 19:37 ` Matthew Carlson
  2020-08-19 19:37 ` [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
  4 siblings, 0 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-08-19 19:37 UTC (permalink / raw)
  To: devel; +Cc: Laszlo Ersek, Ard Biesheuvel, Leif Lindholm, Matthew Carlson

From: Matthew Carlson <macarl@microsoft.com>

Updates the DSC for the ArmVirtPkg platform to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
---
 ArmVirtPkg/ArmVirt.dsc.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index cf44fc73890b..cb3845d2bd37 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -160,6 +160,7 @@
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
   #
   # Secure Boot dependencies
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
  2020-08-19 19:37 [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
                   ` (3 preceding siblings ...)
  2020-08-19 19:37 ` [PATCH v8 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg Matthew Carlson
@ 2020-08-19 19:37 ` Matthew Carlson
  2020-08-19 23:29   ` [edk2-devel] " Yao, Jiewen
  4 siblings, 1 reply; 11+ messages in thread
From: Matthew Carlson @ 2020-08-19 19:37 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Jiewen Yao, Jian J Wang, Xiaoyu Lu, Jiewen Yao,
	Matthew Carlson

From: Matthew Carlson <macarl@microsoft.com>

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 <ard.biesheuvel@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>

Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
---
 CryptoPkg/Library/OpensslLib/rand_pool.c           | 265 +++++---------------
 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, 63 insertions(+), 334 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c b/CryptoPkg/Library/OpensslLib/rand_pool.c
index 9e0179b03490..490b9e2f4692 100644
--- a/CryptoPkg/Library/OpensslLib/rand_pool.c
+++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
@@ -2,8 +2,8 @@
   OpenSSL_1_1_1b doesn't implement rand_pool_* functions for UEFI.
   The file implement these functions.
 
-Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
@@ -11,53 +11,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <openssl/aes.h>
 
 #include <Uefi.h>
-#include <Library/TimerLib.h>
-
-#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 <Library/RngLib.h>
 
 /**
   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
@@ -65,7 +30,7 @@ BOOLEAN
 EFIAPI
 RandGetBytes (
   IN UINTN         Length,
-  OUT UINT8        *RandBuffer
+  OUT UINT8       *RandBuffer
   )
 {
   BOOLEAN     Ret;
@@ -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 (RandBuffer != NULL); // 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;
     }
@@ -91,7 +56,8 @@ RandGetBytes (
       *((UINT64*) RandBuffer) = TempRand;
       RandBuffer += sizeof (UINT64);
       Length -= sizeof (TempRand);
-    } else {
+    }
+    else {
       CopyMem (RandBuffer, &TempRand, Length);
       Length = 0;
     }
@@ -100,125 +66,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
  *
@@ -227,27 +74,31 @@ RandGenerateEntropy (
  *
  * This is OpenSSL required interface.
  */
-size_t rand_pool_acquire_entropy(RAND_POOL *pool)
+size_t
+rand_pool_acquire_entropy (
+  RAND_POOL *pool
+  )
 {
-  BOOLEAN  Ret;
-  size_t bytes_needed;
-  unsigned char * buffer;
+  BOOLEAN        Ret;
+  size_t         bytes_needed;
+  unsigned char *buffer;
 
-  bytes_needed = rand_pool_bytes_needed(pool, 1 /*entropy_factor*/);
+  bytes_needed = rand_pool_bytes_needed (pool, 1 /*entropy_factor*/);
   if (bytes_needed > 0) {
-    buffer = rand_pool_add_begin(pool, bytes_needed);
+    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 {
-        rand_pool_add_end(pool, bytes_needed, 8 * bytes_needed);
+        rand_pool_add_end (pool, 0, 0);
+      }
+      else {
+        rand_pool_add_end (pool, bytes_needed, 8 * bytes_needed);
       }
     }
   }
 
-  return rand_pool_entropy_available(pool);
+  return rand_pool_entropy_available (pool);
 }
 
 /*
@@ -255,17 +106,15 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool)
  *
  * This is OpenSSL required interface.
  */
-int rand_pool_add_nonce_data(RAND_POOL *pool)
+int
+rand_pool_add_nonce_data (
+  RAND_POOL *pool
+  )
 {
-  struct {
-    UINT64  Rand;
-    UINT64  TimerValue;
-  } data = { 0 };
+  UINT8 data[16];
+  RandGetBytes (sizeof(data), data);
 
-  RandGetBytes(8, (UINT8 *)&(data.Rand));
-  data.TimerValue = GetPerformanceCounter();
-
-  return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0);
+  return rand_pool_add (pool, (unsigned char*)&data, sizeof(data), 0);
 }
 
 /*
@@ -273,17 +122,15 @@ int rand_pool_add_nonce_data(RAND_POOL *pool)
  *
  * This is OpenSSL required interface.
  */
-int rand_pool_add_additional_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);
+  return rand_pool_add (pool, (unsigned char*)&data, sizeof(data), 0);
 }
 
 /*
@@ -291,7 +138,10 @@ int rand_pool_add_additional_data(RAND_POOL *pool)
  *
  * This is OpenSSL required interface.
  */
-int rand_pool_init(void)
+int
+rand_pool_init (
+  VOID
+  )
 {
   return 1;
 }
@@ -301,7 +151,10 @@ int rand_pool_init(void)
  *
  * This is OpenSSL required interface.
  */
-void rand_pool_cleanup(void)
+VOID
+rand_pool_cleanup(
+  VOID
+  )
 {
 }
 
@@ -310,7 +163,9 @@ void rand_pool_cleanup(void)
  *
  * This is OpenSSL required interface.
  */
-void rand_pool_keep_random_devices_open(int keep)
+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.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include <Library/BaseLib.h>
-
-/**
-  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.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
-#include <Library/TimerLib.h>
-
-/**
-  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 cc27b8c57cb3..b00bb74ce67e 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.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef __RAND_POOL_NOISE_H__
-#define __RAND_POOL_NOISE_H__
-
-#include <Uefi/UefiBaseType.h>
-
-/**
-   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__
-- 
2.28.0.windows.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
  2020-08-19 19:37 ` [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
@ 2020-08-19 23:29   ` Yao, Jiewen
  0 siblings, 0 replies; 11+ messages in thread
From: Yao, Jiewen @ 2020-08-19 23:29 UTC (permalink / raw)
  To: devel@edk2.groups.io, matthewfcarlson@gmail.com
  Cc: Ard Biesheuvel, Wang, Jian J, Lu, XiaoyuX

Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Matthew
> Carlson
> Sent: Thursday, August 20, 2020 3:37 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Matthew
> Carlson <matthewfcarlson@gmail.com>
> Subject: [edk2-devel] [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to
> generate entropy in rand_pool
> 
> From: Matthew Carlson <macarl@microsoft.com>
> 
> 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 <ard.biesheuvel@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> 
> Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> ---
>  CryptoPkg/Library/OpensslLib/rand_pool.c           | 265 +++++---------------
>  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, 63 insertions(+), 334 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c
> b/CryptoPkg/Library/OpensslLib/rand_pool.c
> index 9e0179b03490..490b9e2f4692 100644
> --- a/CryptoPkg/Library/OpensslLib/rand_pool.c
> +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
> @@ -2,8 +2,8 @@
>    OpenSSL_1_1_1b doesn't implement rand_pool_* functions for UEFI.
> 
>    The file implement these functions.
> 
> 
> 
> -Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> 
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> 
> 
> @@ -11,53 +11,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <openssl/aes.h>
> 
> 
> 
>  #include <Uefi.h>
> 
> -#include <Library/TimerLib.h>
> 
> -
> 
> -#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 <Library/RngLib.h>
> 
> 
> 
>  /**
> 
>    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
> 
> @@ -65,7 +30,7 @@ BOOLEAN
>  EFIAPI
> 
>  RandGetBytes (
> 
>    IN UINTN         Length,
> 
> -  OUT UINT8        *RandBuffer
> 
> +  OUT UINT8       *RandBuffer
> 
>    )
> 
>  {
> 
>    BOOLEAN     Ret;
> 
> @@ -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 (RandBuffer != NULL); // 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;
> 
>      }
> 
> @@ -91,7 +56,8 @@ RandGetBytes (
>        *((UINT64*) RandBuffer) = TempRand;
> 
>        RandBuffer += sizeof (UINT64);
> 
>        Length -= sizeof (TempRand);
> 
> -    } else {
> 
> +    }
> 
> +    else {
> 
>        CopyMem (RandBuffer, &TempRand, Length);
> 
>        Length = 0;
> 
>      }
> 
> @@ -100,125 +66,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
> 
>   *
> 
> @@ -227,27 +74,31 @@ RandGenerateEntropy (
>   *
> 
>   * This is OpenSSL required interface.
> 
>   */
> 
> -size_t rand_pool_acquire_entropy(RAND_POOL *pool)
> 
> +size_t
> 
> +rand_pool_acquire_entropy (
> 
> +  RAND_POOL *pool
> 
> +  )
> 
>  {
> 
> -  BOOLEAN  Ret;
> 
> -  size_t bytes_needed;
> 
> -  unsigned char * buffer;
> 
> +  BOOLEAN        Ret;
> 
> +  size_t         bytes_needed;
> 
> +  unsigned char *buffer;
> 
> 
> 
> -  bytes_needed = rand_pool_bytes_needed(pool, 1 /*entropy_factor*/);
> 
> +  bytes_needed = rand_pool_bytes_needed (pool, 1 /*entropy_factor*/);
> 
>    if (bytes_needed > 0) {
> 
> -    buffer = rand_pool_add_begin(pool, bytes_needed);
> 
> +    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 {
> 
> -        rand_pool_add_end(pool, bytes_needed, 8 * bytes_needed);
> 
> +        rand_pool_add_end (pool, 0, 0);
> 
> +      }
> 
> +      else {
> 
> +        rand_pool_add_end (pool, bytes_needed, 8 * bytes_needed);
> 
>        }
> 
>      }
> 
>    }
> 
> 
> 
> -  return rand_pool_entropy_available(pool);
> 
> +  return rand_pool_entropy_available (pool);
> 
>  }
> 
> 
> 
>  /*
> 
> @@ -255,17 +106,15 @@ size_t rand_pool_acquire_entropy(RAND_POOL
> *pool)
>   *
> 
>   * This is OpenSSL required interface.
> 
>   */
> 
> -int rand_pool_add_nonce_data(RAND_POOL *pool)
> 
> +int
> 
> +rand_pool_add_nonce_data (
> 
> +  RAND_POOL *pool
> 
> +  )
> 
>  {
> 
> -  struct {
> 
> -    UINT64  Rand;
> 
> -    UINT64  TimerValue;
> 
> -  } data = { 0 };
> 
> +  UINT8 data[16];
> 
> +  RandGetBytes (sizeof(data), data);
> 
> 
> 
> -  RandGetBytes(8, (UINT8 *)&(data.Rand));
> 
> -  data.TimerValue = GetPerformanceCounter();
> 
> -
> 
> -  return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0);
> 
> +  return rand_pool_add (pool, (unsigned char*)&data, sizeof(data), 0);
> 
>  }
> 
> 
> 
>  /*
> 
> @@ -273,17 +122,15 @@ int rand_pool_add_nonce_data(RAND_POOL *pool)
>   *
> 
>   * This is OpenSSL required interface.
> 
>   */
> 
> -int rand_pool_add_additional_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);
> 
> +  return rand_pool_add (pool, (unsigned char*)&data, sizeof(data), 0);
> 
>  }
> 
> 
> 
>  /*
> 
> @@ -291,7 +138,10 @@ int rand_pool_add_additional_data(RAND_POOL *pool)
>   *
> 
>   * This is OpenSSL required interface.
> 
>   */
> 
> -int rand_pool_init(void)
> 
> +int
> 
> +rand_pool_init (
> 
> +  VOID
> 
> +  )
> 
>  {
> 
>    return 1;
> 
>  }
> 
> @@ -301,7 +151,10 @@ int rand_pool_init(void)
>   *
> 
>   * This is OpenSSL required interface.
> 
>   */
> 
> -void rand_pool_cleanup(void)
> 
> +VOID
> 
> +rand_pool_cleanup(
> 
> +  VOID
> 
> +  )
> 
>  {
> 
>  }
> 
> 
> 
> @@ -310,7 +163,9 @@ void rand_pool_cleanup(void)
>   *
> 
>   * This is OpenSSL required interface.
> 
>   */
> 
> -void rand_pool_keep_random_devices_open(int keep)
> 
> +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.<BR>
> 
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -
> 
> -**/
> 
> -
> 
> -#include <Library/BaseLib.h>
> 
> -
> 
> -/**
> 
> -  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.<BR>
> 
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -
> 
> -**/
> 
> -
> 
> -#include <Library/BaseLib.h>
> 
> -#include <Library/DebugLib.h>
> 
> -#include <Library/TimerLib.h>
> 
> -
> 
> -/**
> 
> -  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 cc27b8c57cb3..b00bb74ce67e 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.<BR>
> 
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -
> 
> -**/
> 
> -
> 
> -#ifndef __RAND_POOL_NOISE_H__
> 
> -#define __RAND_POOL_NOISE_H__
> 
> -
> 
> -#include <Uefi/UefiBaseType.h>
> 
> -
> 
> -/**
> 
> -   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__
> 
> --
> 2.28.0.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#64470): https://edk2.groups.io/g/devel/message/64470
> Mute This Topic: https://groups.io/mt/76294219/1772286
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [jiewen.yao@intel.com]
> -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  2020-08-19 19:37 ` [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
@ 2020-08-20  8:05   ` Ard Biesheuvel
  2020-08-20 15:21   ` Michael D Kinney
  1 sibling, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-08-20  8:05 UTC (permalink / raw)
  To: matthewfcarlson, devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu

On 8/19/20 9:37 PM, matthewfcarlson@gmail.com wrote:
> From: Matthew Carlson <macarl@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@arm.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Matthew Carlson <matthewfcarlson@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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
  2020-08-19 19:37 ` [PATCH v8 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe Matthew Carlson
@ 2020-08-20  8:09   ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-08-20  8:09 UTC (permalink / raw)
  To: matthewfcarlson, devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu

On 8/19/20 9:37 PM, matthewfcarlson@gmail.com wrote:
> From: Matthew Carlson <macarl@microsoft.com>
> 
> This adds a RngLib that uses the RngProtocol to provide randomness.
> This means that the RngLib is meant to be used with DXE_DRIVERS.
> 
> Ref: https://github.com/tianocore/edk2/pull/845
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> ---
>   MdePkg/Library/DxeRngLib/DxeRngLib.c   | 206 ++++++++++++++++++++
>   MdePkg/Library/DxeRngLib/DxeRngLib.inf |  38 ++++
>   MdePkg/Library/DxeRngLib/DxeRngLib.uni |  15 ++
>   MdePkg/MdePkg.dsc                      |   4 +-
>   4 files changed, 262 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c b/MdePkg/Library/DxeRngLib/DxeRngLib.c
> new file mode 100644
> index 000000000000..0bd6585357b5
> --- /dev/null
> +++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c
> @@ -0,0 +1,206 @@
> +/** @file
> + Provides an implementation of the library class RngLib that uses the Rng protocol.
> +
> + Copyright (c) Microsoft Corporation. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#include <Uefi.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/RngLib.h>
> +#include <Protocol/Rng.h>
> +
> +/**
> +Routine Description:
> +
> +    Generates a random number via the NIST
> +    800-9A algorithm.  Refer to
> +    http://csrc.nist.gov/groups/STM/cavp/documents/drbg/DRBGVS.pdf
> +    for more information.
> +
> +    Arguments:
> +
> +    Buffer      -- Buffer to receive the random number.
> +    BufferSize  -- Number of bytes in Buffer.
> +
> +Return Value:
> +
> +    EFI_SUCCESS or underlying failure code.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +GenerateRandomNumberViaNist800Algorithm (
> +  OUT UINT8 *Buffer,
> +  IN  UINTN  BufferSize
> +  )
> +{
> +  EFI_STATUS        Status;
> +  EFI_RNG_PROTOCOL *RngProtocol;

Align * with variable name please

> +
> +  RngProtocol = NULL;
> +
> +  if (Buffer == NULL) {
> +      DEBUG((DEBUG_ERROR, "[%a] Buffer == NULL.\n", __FUNCTION__));

Space before (

Also, drop the [] around %a for consistency with other code.

> +      return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = gBS->LocateProtocol (&gEfiRngProtocolGuid, NULL, (VOID **)&RngProtocol);
> +  if (EFI_ERROR (Status) || RngProtocol == NULL) {
> +      DEBUG((DEBUG_ERROR, "%a: Could not locate RNG prototocol, Status = %r\n", __FUNCTION__, Status));

Space before (

> +      return Status;
> +  }
> +
> +  Status = RngProtocol->GetRNG (RngProtocol, &gEfiRngAlgorithmSp80090Ctr256Guid, BufferSize, Buffer);
> +  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm CTR-256 - Status = %r\n", __FUNCTION__, Status));

Same here

> +  if(!EFI_ERROR(Status)) {

Space after if

I think I mentioned this the last time around: please make sure that you 
adhere to the coding style, and fix *all* occurrences, not just the ones 
that were pointed out to you.

> +    return Status;
> +  }
> +
> +  Status = RngProtocol->GetRNG (RngProtocol, &gEfiRngAlgorithmSp80090Hmac256Guid, BufferSize, Buffer);
> +  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm HMAC-256 - Status = %r\n", __FUNCTION__, Status));
> +  if(!EFI_ERROR(Status)) {
> +    return Status;
> +  }
> +
> +  Status = RngProtocol->GetRNG (RngProtocol, &gEfiRngAlgorithmSp80090Hash256Guid, BufferSize, Buffer);
> +  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", __FUNCTION__, Status));
> +  if (!EFI_ERROR(Status)) {
> +    return Status;
> +  }
> +  // If all the other methods have failed, use the default method from the RngProtocol
> +  Status = RngProtocol->GetRNG (RngProtocol, NULL, BufferSize, Buffer);
> +  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", __FUNCTION__, Status));
> +  if (!EFI_ERROR(Status)) {
> +    return Status;
> +  }
> +  // If we get to this point, we have failed
> +  DEBUG((DEBUG_ERROR, "%a: GetRNG() failed, staus = %r\n", __FUNCTION__, Status));
> +
> +  return Status;
> +}// GenerateRandomNumberViaNist800Algorithm()
> +
> +
> +/**
> +  Generates a 16-bit random number.
> +
> +  if Rand is NULL, return FALSE.
> +
> +  @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
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  if (Rand == NULL)
> +  {

Opening brace should be on the previous line.

> +    return FALSE;
> +  }
> +
> +  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 2);
> +  if (EFI_ERROR (Status))
> +  {

Same here

> +    return FALSE;
> +  }
> +  return TRUE;
> +}
> +
> +/**
> +  Generates a 32-bit random number.
> +
> +  if Rand is NULL, return FALSE.
> +
> +  @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
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  if (Rand == NULL) {
> +    return FALSE;
> +  }
> +
> +  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 4);
> +  if (EFI_ERROR (Status)) {
> +    return FALSE;
> +  }
> +  return TRUE;
> +}
> +
> +/**
> +  Generates a 64-bit random number.
> +
> +  if Rand is NULL, return FALSE.
> +
> +  @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
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  if (Rand == NULL)
> +  {
> +    return FALSE;
> +  }
> +
> +  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 8);
> +  if (EFI_ERROR (Status)) {
> +    return FALSE;
> +  }
> +  return TRUE;
> +}
> +
> +/**
> +  Generates a 128-bit random number.
> +
> +  if Rand is NULL, return FALSE.
> +
> +  @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
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  if (Rand == NULL) {
> +    return FALSE;
> +  }
> +
> +  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 16);
> +  if (EFI_ERROR (Status)) {
> +    return FALSE;
> +  }
> +  return TRUE;
> +}
> diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.inf b/MdePkg/Library/DxeRngLib/DxeRngLib.inf
> new file mode 100644
> index 000000000000..f4838aa05b7e
> --- /dev/null
> +++ b/MdePkg/Library/DxeRngLib/DxeRngLib.inf
> @@ -0,0 +1,38 @@
> +# @file
> +# Provides implementation of the library class RngLib that uses the RngProtocol
> +#
> +# @copyright
> +# Copyright (c) Microsoft Corporation. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION     = 0x00010017

Please use 1.27 (or 0x00001001B) for new modules.

> +  BASE_NAME       = DxeRngLib
> +  MODULE_UNI_FILE = DxeRngLib.uni
> +  FILE_GUID       = FF9F84C5-A33E-44E3-9BB5-0D654B2D4149
> +  MODULE_TYPE     = DXE_DRIVER
> +  VERSION_STRING  = 1.0
> +  LIBRARY_CLASS   = RngLib|DXE_DRIVER UEFI_APPLICATION UEFI_DRIVER
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[Sources]
> +  DxeRngLib.c
> +
> +[LibraryClasses]
> +  DebugLib
> +  UefiBootServicesTableLib
> +
> +[Protocols]
> +  gEfiRngProtocolGuid                 ## CONSUMES
> +
> +[Depex]
> +  gEfiRngProtocolGuid
> +
> +[Guids]
> +  gEfiRngAlgorithmSp80090Ctr256Guid
> +  gEfiRngAlgorithmSp80090Hash256Guid
> +  gEfiRngAlgorithmSp80090Hmac256Guid
> diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.uni b/MdePkg/Library/DxeRngLib/DxeRngLib.uni
> new file mode 100644
> index 000000000000..c904e54b6fb0
> --- /dev/null
> +++ b/MdePkg/Library/DxeRngLib/DxeRngLib.uni
> @@ -0,0 +1,15 @@
> +// @file
> +// Instance of RNG (Random Number Generator) Library.
> +//
> +// RngLib that uses the Rng Protocol 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 Rng Protocol to provide random numbers"
> +
> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
> index d7ba3a730909..2c3b7966b086 100644
> --- a/MdePkg/MdePkg.dsc
> +++ b/MdePkg/MdePkg.dsc
> @@ -62,8 +62,10 @@
>     MdePkg/Library/BasePostCodeLibPort80/BasePostCodeLibPort80.inf
>     MdePkg/Library/BasePrintLib/BasePrintLib.inf
>     MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
> -  MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> +  MdePkg/Library/DxeRngLib/DxeRngLib.inf
>     MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf
> +  MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> +
>     MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
>     MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>     MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  2020-08-19 19:37 ` [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
  2020-08-20  8:05   ` Ard Biesheuvel
@ 2020-08-20 15:21   ` Michael D Kinney
  2020-08-20 16:18     ` Matthew Carlson
  1 sibling, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2020-08-20 15:21 UTC (permalink / raw)
  To: matthewfcarlson@gmail.com, devel@edk2.groups.io,
	Kinney, Michael D
  Cc: Ard Biesheuvel, Gao, Liming, Liu, Zhiguang

Hi Matt,

Some comments inline below.

I also see come comments from Ard on this series about code style.
I did not provide feedback on the code style issues here (except for
a function header comment block style).

There is a tool called ECC (EFI Code Checker) that is now enabled in
EDK II CI.  Please run this checker locally and resolve all issues in
your patch series.

Thanks,

Mike


> -----Original Message-----
> From: matthewfcarlson@gmail.com <matthewfcarlson@gmail.com>
> Sent: Wednesday, August 19, 2020 12:37 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Liu, Zhiguang <zhiguang.liu@intel.com>; Matthew Carlson <matthewfcarlson@gmail.com>
> Subject: [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib
> 
> From: Matthew Carlson <macarl@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@arm.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> ---
>  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


Please update the comments to describe that this function returns 
delay in microseconds.  It does not actually do the delay.

> 
> + *
> 
> + * If the return value from GetPerformanceCounterProperties (TimerLib)
> 
> + * is zero, this function will not delay and attempt to assert.
> 
> + */

Comment block style does not match EDK II style.  Remove extra '*'.


> 
> +STATIC
> 
> +UINT32
> 
> +CalculateMinimumDecentDelayInMicroseconds (
> 
> +  VOID
> 
> +  )
> 
> +{
> 
> +  UINT64 StartValue;

Remove

> 
> +  UINT64 EndValue;

Remove

> 
> +  UINT64 CounterHz;
> 
> +  UINT64 MinumumDelayInMicroSeconds;
> 
> +
> 
> +  // Get the counter properties
> 
> +  CounterHz = GetPerformanceCounterProperties (&StartValue, &EndValue);


Change to:

    CounterHz = GetPerformanceCounterProperties (NULL, NULL);

> 
> +  // Make sure we won't divide by zero
> 
> +  if (CounterHz == 0) {
> 
> +    ASSERT(CounterHz != 0); // Assert so the developer knows something is wrong
> 
> +    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));
> 
> +}
> 
> +
> 
> +
> 
> +/**
> 
> +  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;
> 
> +  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;
> 
> +  UINT32  DelayInMicroSeconds;
> 
> +
> 
> +  ASSERT (Rand != NULL);
> 
> +
> 
> +  if (NULL == Rand) {
> 
> +    return FALSE;
> 
> +  }
> 
> +
> 
> +  RandPtr = (UINT8 *) Rand;
> 
> +  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


Update comment to remove mention of specific delay and match
similar comments in functions above.

> 
> +
> 
> +  // 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
> 
> --
> 2.28.0.windows.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  2020-08-20 15:21   ` Michael D Kinney
@ 2020-08-20 16:18     ` Matthew Carlson
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-08-20 16:18 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Gao, Liming, Liu, Zhiguang

[-- Attachment #1: Type: text/plain, Size: 14751 bytes --]

Thank you Mike and Ard,

I'll definitely run ECC and resolve the code nits as well as the other
suggestions.

So the previous RNG code in OpenSSL just had a 10-microsecond delay in
between every 2 bytes. We could go back to that and match the behavior of
before but as Mike pointed out, this approach suffers on systems with slow
timer libs. I know this particular Rng library isn't meant to be a good
source of randomness and it is meant to offer a solution to those who don't
want to make the switch over to a good RngLib specifically for OpenSSL.
That said, I don't see any reason why we can't make this a better source of
random. I don't think it would be quite N, 2N, 3N, ... because of the
variability in the delay mechanism but I do agree that it would be quite
close to that. I'll throw out some options:

1. Implement a seeding mechanism that we mix in values with what we've
previously generated. So that it's no longer N, 2N, 3N but rather some sort
of hashing of the two numbers or other PRNG type system.
2. Use only 8 bits from the performance counter rather than 16. The idea
here is that it would roll over more frequently and you'd be more subject
to randomness. The downside is that this would take twice as long, which
means to generate 64 bits of random data, it would take at least 12
performance timer ticks, which on systems where their performance counters
run in KHz rather than MHz or GHz, means you could be looking at a delay of
milliseconds rather than microseconds. I'd argue in that case that the
platform should use a real RngLib rather than use this one if their timer
is so slow, but that's beside the point.
3. Have some way of specifying the delay that is less deterministic? There
are a few ways I can think of doing this but none are very good.I'm open to
suggestions.

-Matthew Carlson


On Thu, Aug 20, 2020 at 8:21 AM Kinney, Michael D <
michael.d.kinney@intel.com> wrote:

> Hi Matt,
>
> Some comments inline below.
>
> I also see come comments from Ard on this series about code style.
> I did not provide feedback on the code style issues here (except for
> a function header comment block style).
>
> There is a tool called ECC (EFI Code Checker) that is now enabled in
> EDK II CI.  Please run this checker locally and resolve all issues in
> your patch series.
>
> Thanks,
>
> Mike
>
>
> > -----Original Message-----
> > From: matthewfcarlson@gmail.com <matthewfcarlson@gmail.com>
> > Sent: Wednesday, August 19, 2020 12:37 PM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Kinney, Michael D <
> michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> > Liu, Zhiguang <zhiguang.liu@intel.com>; Matthew Carlson <
> matthewfcarlson@gmail.com>
> > Subject: [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses
> TimerLib
> >
> > From: Matthew Carlson <macarl@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@arm.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> > ---
> >  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
>
>
> Please update the comments to describe that this function returns
> delay in microseconds.  It does not actually do the delay.
>
> >
> > + *
> >
> > + * If the return value from GetPerformanceCounterProperties (TimerLib)
> >
> > + * is zero, this function will not delay and attempt to assert.
> >
> > + */
>
> Comment block style does not match EDK II style.  Remove extra '*'.
>
>
> >
> > +STATIC
> >
> > +UINT32
> >
> > +CalculateMinimumDecentDelayInMicroseconds (
> >
> > +  VOID
> >
> > +  )
> >
> > +{
> >
> > +  UINT64 StartValue;
>
> Remove
>
> >
> > +  UINT64 EndValue;
>
> Remove
>
> >
> > +  UINT64 CounterHz;
> >
> > +  UINT64 MinumumDelayInMicroSeconds;
> >
> > +
> >
> > +  // Get the counter properties
> >
> > +  CounterHz = GetPerformanceCounterProperties (&StartValue, &EndValue);
>
>
> Change to:
>
>     CounterHz = GetPerformanceCounterProperties (NULL, NULL);
>
> >
> > +  // Make sure we won't divide by zero
> >
> > +  if (CounterHz == 0) {
> >
> > +    ASSERT(CounterHz != 0); // Assert so the developer knows something
> is wrong
> >
> > +    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));
> >
> > +}
> >
> > +
> >
> > +
> >
> > +/**
> >
> > +  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;
> >
> > +  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;
> >
> > +  UINT32  DelayInMicroSeconds;
> >
> > +
> >
> > +  ASSERT (Rand != NULL);
> >
> > +
> >
> > +  if (NULL == Rand) {
> >
> > +    return FALSE;
> >
> > +  }
> >
> > +
> >
> > +  RandPtr = (UINT8 *) Rand;
> >
> > +  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
>
>
> Update comment to remove mention of specific delay and match
> similar comments in functions above.
>
> >
> > +
> >
> > +  // 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
> >
> > --
> > 2.28.0.windows.1
>
>

[-- Attachment #2: Type: text/html, Size: 20723 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-08-20 16:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-19 19:37 [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
2020-08-19 19:37 ` [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
2020-08-20  8:05   ` Ard Biesheuvel
2020-08-20 15:21   ` Michael D Kinney
2020-08-20 16:18     ` Matthew Carlson
2020-08-19 19:37 ` [PATCH v8 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe Matthew Carlson
2020-08-20  8:09   ` Ard Biesheuvel
2020-08-19 19:37 ` [PATCH v8 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto Matthew Carlson
2020-08-19 19:37 ` [PATCH v8 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg Matthew Carlson
2020-08-19 19:37 ` [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
2020-08-19 23:29   ` [edk2-devel] " Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox