[PATCH v4 0/5] Use RngLib instead of TimerLib for OpensslLib

[PATCH v4 0/5] Use RngLib instead of TimerLib for OpensslLib

[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

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                 | 203 ++------------------
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c           |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c       |  43 -----
 MdePkg/Library/BaseRngLibDxe/RngDxeLib.c                 | 200 +++++++++++++++++++
 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c          | 187 ++++++++++++++++++
 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/BaseRngLibDxe/BaseRngLibDxe.inf           |  38 ++++
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  40 ++++
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  17 ++
 MdePkg/MdePkg.dsc                                        |   5 +-
 OvmfPkg/OvmfPkgIa32.dsc                                  |   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                               |   1 +
 OvmfPkg/OvmfPkgX64.dsc                                   |   1 +
 OvmfPkg/OvmfXen.dsc                                      |   1 +
 18 files changed, 513 insertions(+), 314 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/BaseRngLibDxe/RngDxeLib.c
 create mode 100644 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
 delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.h
 create mode 100644 MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.inf
 create mode 100644 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 create mode 100644 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni

-- 
2.27.0.windows.1

[PATCH v4 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib

[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.

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          | 187 ++++++++++++++++++++
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  40 +++++
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  17 ++
 MdePkg/MdePkg.dsc                                        |   3 +-
 4 files changed, 246 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
new file mode 100644
index 000000000000..915382fb9278
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
@@ -0,0 +1,187 @@
+/** @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.
+ */
+VOID
+EFIAPI
+DecentDelay(
+  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(FALSE); // 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
+  MinumumDelayInMicroSeconds = 1500000 / CounterHz;
+
+  MicroSecondDelay(MinumumDelayInMicroSeconds);
+}
+
+
+/**
+  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;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+    return FALSE;
+  }
+
+  RandPtr = (UINT8 *) Rand;
+  // Get 2 bytes of random ish data
+  // This should take around 10us
+  for (Index = 0; Index < 2; Index ++) {
+    *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+    DecentDelay (); // delay to give chance for performance counter to catch up
+    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;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+    return FALSE;
+  }
+
+  RandPtr = (UINT8 *) Rand;
+  // Get 4 bytes of random ish data
+  // This should take around 20ms
+  for (Index = 0; Index < 4; Index ++) {
+    *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+    DecentDelay (); // delay to give chance for performance counter to catch up
+    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;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+    return FALSE;
+  }
+
+  RandPtr = (UINT8 *) Rand;
+  // Get 8 bytes of random ish data
+  // This should take around 40ms
+  for (Index = 0; Index < 8; Index ++) {
+    *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+    DecentDelay (); // delay to give chance for performance counter to catch up
+    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..34dea0152497
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
@@ -0,0 +1,40 @@
+## @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                    = 0x00010005
+  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
+
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[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..766a8e0ddf97
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni
@@ -0,0 +1,17 @@
+// /** @file
+// Instance of RNG (Random Number Generator) Library.
+//
+// BaseRng Library 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.27.0.windows.1

[PATCH v4 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe

[Matthew Carlson]

From: Matthew Carlson <macarl@microsoft.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/BaseRngLibDxe/RngDxeLib.c       | 200 ++++++++++++++++++++
 MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.inf |  38 ++++
 MdePkg/MdePkg.dsc                              |   4 +-
 3 files changed, 241 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseRngLibDxe/RngDxeLib.c b/MdePkg/Library/BaseRngLibDxe/RngDxeLib.c
new file mode 100644
index 000000000000..8ee29329de13
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibDxe/RngDxeLib.c
@@ -0,0 +1,200 @@
+/** @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.
+
+**/
+EFI_STATUS
+EFIAPI
+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 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/BaseRngLibDxe/BaseRngLibDxe.inf b/MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.inf
new file mode 100644
index 000000000000..819a106b1376
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.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
+#
+# MU_CHANGE: New file
+##
+
+[Defines]
+  INF_VERSION     = 0x00010017
+  BASE_NAME       = BaseRngLibDxe
+  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]
+  RngDxeLib.c
+
+[LibraryClasses]
+  DebugLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gEfiRngProtocolGuid                 ## CONSUMES
+
+[Depex]
+  gEfiRngProtocolGuid
+
+[Guids]
+  gEfiRngAlgorithmSp80090Ctr256Guid            ## CONSUMES
+  gEfiRngAlgorithmSp80090Hash256Guid           ## CONSUMES
+  gEfiRngAlgorithmSp80090Hmac256Guid           ## CONSUMES
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index d7ba3a730909..837a0047400e 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/BaseRngLibDxe/BaseRngLibDxe.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.27.0.windows.1

[PATCH v4 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Matthew Carlson]

From: Matthew Carlson <macarl@microsoft.com>

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>
Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgX64.dsc     | 1 +
 OvmfPkg/OvmfXen.dsc        | 1 +
 4 files changed, 4 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 9178ffeb71cb..118fd1aff246 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -116,6 +116,7 @@
 [LibraryClasses]
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
   ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a665f78f0dc7..6b9da5b996ff 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -120,6 +120,7 @@
 [LibraryClasses]
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
   ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 17f345acf4ee..3a354eb3a2bd 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -120,6 +120,7 @@
 [LibraryClasses]
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
   ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 782803cb2787..f97e2b7e07d0 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -110,6 +110,7 @@
 [LibraryClasses]
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   TimerLib|MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
   ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
-- 
2.27.0.windows.1

[PATCH v4 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg

[Matthew Carlson]

From: Matthew Carlson <macarl@microsoft.com>

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.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..ddfcd0cf9eee 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -42,6 +42,7 @@
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
 
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
   SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
-- 
2.27.0.windows.1

[PATCH v4 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

[Matthew Carlson]

From: Matthew Carlson <macarl@microsoft.com>

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: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
---
 CryptoPkg/Library/OpensslLib/rand_pool.c           | 203 ++------------------
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c     |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |  43 -----
 CryptoPkg/CryptoPkg.dsc                            |   1 +
 CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  15 +-
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
 CryptoPkg/Library/OpensslLib/rand_pool_noise.h     |  29 ---
 7 files changed, 22 insertions(+), 313 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c b/CryptoPkg/Library/OpensslLib/rand_pool.c
index 9e0179b03490..3da92699fef6 100644
--- a/CryptoPkg/Library/OpensslLib/rand_pool.c
+++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
@@ -11,53 +11,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <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
@@ -73,17 +38,17 @@ RandGetBytes (
 
   Ret = FALSE;
 
+  if (RandBuffer == NULL) {
+    DEBUG((DEBUG_ERROR, "[OPENSSL_RAND_POOL] NULL RandBuffer. No random numbers are generated and your system is not secure\n"));
+    ASSERT(FALSE); // Since we can't generate random numbers, we should assert. Otherwise we will just blow up later.
+    return Ret;
+  }
+
+
   while (Length > 0) {
-    //
-    // Get random noise from platform.
-    // If it failed, fallback to PerformanceCounter
-    // If you really care about security, you must override
-    // GetRandomNoise64FromPlatform.
-    //
-    Ret = GetRandomNoise64 (&TempRand);
-    if (Ret == FALSE) {
-      Ret = GetRandNoise64FromPerformanceCounter (&TempRand);
-    }
+    // Use RngLib to get random number
+    Ret = GetRandomNumber64(&TempRand);
+
     if (!Ret) {
       return Ret;
     }
@@ -100,125 +65,6 @@ RandGetBytes (
   return Ret;
 }
 
-/**
-  Creates a 128bit random value that is fully forward and backward prediction resistant,
-  suitable for seeding a NIST SP800-90 Compliant.
-  This function takes multiple random numbers from PerformanceCounter to ensure reseeding
-  and performs AES-CBC-MAC over the data to compute the seed value.
-
-  @param[out]  SeedBuffer    Pointer to a 128bit buffer to store the random seed.
-
-  @retval TRUE        Random seed generation succeeded.
-  @retval FALSE      Failed to request random bytes.
-
-**/
-STATIC
-BOOLEAN
-EFIAPI
-RandGetSeed128 (
-  OUT UINT8        *SeedBuffer
-  )
-{
-  BOOLEAN     Ret;
-  UINT8       RandByte[16];
-  UINT8       Key[16];
-  UINT8       Ffv[16];
-  UINT8       Xored[16];
-  UINT32      Index;
-  UINT32      Index2;
-  AES_KEY     AESKey;
-
-  //
-  // Chose an arbitrary key and zero the feed_forward_value (FFV)
-  //
-  for (Index = 0; Index < 16; Index++) {
-    Key[Index] = (UINT8) Index;
-    Ffv[Index] = 0;
-  }
-
-  AES_set_encrypt_key (Key, 16 * 8, &AESKey);
-
-  //
-  // Perform CBC_MAC over 32 * 128 bit values, with 10us gaps between 128 bit value
-  // The 10us gaps will ensure multiple reseeds within the system time with a large
-  // design margin.
-  //
-  for (Index = 0; Index < 32; Index++) {
-    MicroSecondDelay (10);
-    Ret = RandGetBytes (16, RandByte);
-    if (!Ret) {
-      return Ret;
-    }
-
-    //
-    // Perform XOR operations on two 128-bit value.
-    //
-    for (Index2 = 0; Index2 < 16; Index2++) {
-      Xored[Index2] = RandByte[Index2] ^ Ffv[Index2];
-    }
-
-    AES_encrypt (Xored, Ffv, &AESKey);
-  }
-
-  for (Index = 0; Index < 16; Index++) {
-    SeedBuffer[Index] = Ffv[Index];
-  }
-
-  return Ret;
-}
-
-/**
-  Generate high-quality entropy source.
-
-  @param[in]   Length        Size of the buffer, in bytes, to fill with.
-  @param[out]  Entropy       Pointer to the buffer to store the entropy data.
-
-  @retval EFI_SUCCESS        Entropy generation succeeded.
-  @retval EFI_NOT_READY      Failed to request random data.
-
-**/
-STATIC
-BOOLEAN
-EFIAPI
-RandGenerateEntropy (
-  IN UINTN         Length,
-  OUT UINT8        *Entropy
-  )
-{
-  BOOLEAN     Ret;
-  UINTN       BlockCount;
-  UINT8       Seed[16];
-  UINT8       *Ptr;
-
-  BlockCount = Length / 16;
-  Ptr        = (UINT8 *) Entropy;
-
-  //
-  // Generate high-quality seed for DRBG Entropy
-  //
-  while (BlockCount > 0) {
-    Ret = RandGetSeed128 (Seed);
-    if (!Ret) {
-      return Ret;
-    }
-    CopyMem (Ptr, Seed, 16);
-
-    BlockCount--;
-    Ptr = Ptr + 16;
-  }
-
-  //
-  // Populate the remained data as request.
-  //
-  Ret = RandGetSeed128 (Seed);
-  if (!Ret) {
-    return Ret;
-  }
-  CopyMem (Ptr, Seed, (Length % 16));
-
-  return Ret;
-}
-
 /*
  * Add random bytes to the pool to acquire requested amount of entropy
  *
@@ -238,7 +84,7 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool)
     buffer = rand_pool_add_begin(pool, bytes_needed);
 
     if (buffer != NULL) {
-      Ret = RandGenerateEntropy(bytes_needed, buffer);
+      Ret = RandGetBytes(bytes_needed, buffer);
       if (FALSE == Ret) {
         rand_pool_add_end(pool, 0, 0);
       } else {
@@ -257,13 +103,8 @@ size_t rand_pool_acquire_entropy(RAND_POOL *pool)
  */
 int rand_pool_add_nonce_data(RAND_POOL *pool)
 {
-  struct {
-    UINT64  Rand;
-    UINT64  TimerValue;
-  } data = { 0 };
-
-  RandGetBytes(8, (UINT8 *)&(data.Rand));
-  data.TimerValue = GetPerformanceCounter();
+  UINT8 data[16];
+  RandGetBytes(sizeof(data), data);
 
   return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0);
 }
@@ -275,13 +116,8 @@ int rand_pool_add_nonce_data(RAND_POOL *pool)
  */
 int rand_pool_add_additional_data(RAND_POOL *pool)
 {
-  struct {
-    UINT64  Rand;
-    UINT64  TimerValue;
-  } data = { 0 };
-
-  RandGetBytes(8, (UINT8 *)&(data.Rand));
-  data.TimerValue = GetPerformanceCounter();
+  UINT8 data[16];
+  RandGetBytes(sizeof(data), data);
 
   return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0);
 }
@@ -313,4 +149,3 @@ void rand_pool_cleanup(void)
 void rand_pool_keep_random_devices_open(int keep)
 {
 }
-
diff --git a/CryptoPkg/Library/OpensslLib/rand_pool_noise.c b/CryptoPkg/Library/OpensslLib/rand_pool_noise.c
deleted file mode 100644
index 212834e27acc..000000000000
--- a/CryptoPkg/Library/OpensslLib/rand_pool_noise.c
+++ /dev/null
@@ -1,29 +0,0 @@
-/** @file
-  Provide rand noise source.
-
-Copyright (c) 2019, Intel Corporation. All rights reserved.<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 dbbe5386a10c..4baad565564c 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -571,22 +571,9 @@
   $(OPENSSL_PATH)/ssl/statem/statem_local.h
 # Autogenerated files list ends here
   buildinf.h
-  rand_pool_noise.h
   ossl_store.c
   rand_pool.c
 
-[Sources.Ia32]
-  rand_pool_noise_tsc.c
-
-[Sources.X64]
-  rand_pool_noise_tsc.c
-
-[Sources.ARM]
-  rand_pool_noise.c
-
-[Sources.AARCH64]
-  rand_pool_noise.c
-
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
@@ -594,7 +581,7 @@
 [LibraryClasses]
   BaseLib
   DebugLib
-  TimerLib
+  RngLib
   PrintLib
 
 [LibraryClasses.ARM]
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index 616ccd9f62d1..3557711bd85a 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
@@ -520,22 +520,9 @@
   $(OPENSSL_PATH)/crypto/x509v3/v3_admis.h
 # Autogenerated files list ends here
   buildinf.h
-  rand_pool_noise.h
   ossl_store.c
   rand_pool.c
 
-[Sources.Ia32]
-  rand_pool_noise_tsc.c
-
-[Sources.X64]
-  rand_pool_noise_tsc.c
-
-[Sources.ARM]
-  rand_pool_noise.c
-
-[Sources.AARCH64]
-  rand_pool_noise.c
-
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
@@ -543,7 +530,7 @@
 [LibraryClasses]
   BaseLib
   DebugLib
-  TimerLib
+  RngLib
   PrintLib
 
 [LibraryClasses.ARM]
diff --git a/CryptoPkg/Library/OpensslLib/rand_pool_noise.h b/CryptoPkg/Library/OpensslLib/rand_pool_noise.h
deleted file mode 100644
index 75acc686a9f1..000000000000
--- a/CryptoPkg/Library/OpensslLib/rand_pool_noise.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/** @file
-  Provide rand noise source.
-
-Copyright (c) 2019, Intel Corporation. All rights reserved.<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.27.0.windows.1

Re: [PATCH v4 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Ard Biesheuvel]

On 8/11/20 4:21 AM, matthewfcarlson@gmail.com wrote:
> From: Matthew Carlson <macarl@microsoft.com>
> 

How am I supposed to review this change? The commit log is empty and I 
was not cc'ed on the cover letter.

In general, please try to muster up the energy to write at least one 
sentence that describes *why* the patch is needed, complementing the 
subject line, which in this case summarizes correctly *what* the patch does.

Thanks,
Ard.



> 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>
> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> ---
>   OvmfPkg/OvmfPkgIa32.dsc    | 1 +
>   OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>   OvmfPkg/OvmfPkgX64.dsc     | 1 +
>   OvmfPkg/OvmfXen.dsc        | 1 +
>   4 files changed, 4 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 9178ffeb71cb..118fd1aff246 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -116,6 +116,7 @@
>   [LibraryClasses]
>     PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>     TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>     ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>     PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>     BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index a665f78f0dc7..6b9da5b996ff 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -120,6 +120,7 @@
>   [LibraryClasses]
>     PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>     TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>     ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>     PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>     BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 17f345acf4ee..3a354eb3a2bd 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -120,6 +120,7 @@
>   [LibraryClasses]
>     PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>     TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>     ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>     PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>     BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 782803cb2787..f97e2b7e07d0 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -110,6 +110,7 @@
>   [LibraryClasses]
>     PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>     TimerLib|MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>     ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>     PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>     BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> 

Re: [edk2-devel] [PATCH v4 0/5] Use RngLib instead of TimerLib for OpensslLib

[Guomin Jiang]

I will review the patch by next week(8/21).

Thanks
Guomin

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Matthew Carlson
> Sent: Tuesday, August 11, 2020 10:22 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v4 0/5] Use RngLib instead of TimerLib for
> OpensslLib
> 
> From: Matthew Carlson <macarl@microsoft.com>
> 
> Ref: https://github.com/tianocore/edk2/pull/845
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> 
> 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                 | 203 ++------------------
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.c           |  29 ---
>  CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c       |  43 -----
>  MdePkg/Library/BaseRngLibDxe/RngDxeLib.c                 | 200
> +++++++++++++++++++
>  MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c          | 187
> ++++++++++++++++++
>  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/BaseRngLibDxe/BaseRngLibDxe.inf           |  38 ++++
>  MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  40 ++++
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  17 ++
>  MdePkg/MdePkg.dsc                                        |   5 +-
>  OvmfPkg/OvmfPkgIa32.dsc                                  |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                               |   1 +
>  OvmfPkg/OvmfPkgX64.dsc                                   |   1 +
>  OvmfPkg/OvmfXen.dsc                                      |   1 +
>  18 files changed, 513 insertions(+), 314 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/BaseRngLibDxe/RngDxeLib.c
>  create mode 100644 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
>  delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.h
>  create mode 100644 MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.inf
>  create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>  create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni
> 
> --
> 2.27.0.windows.1
> 
> 
> 

Re: [edk2-devel] [PATCH v4 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Laszlo Ersek]

Hi Ard!

On 08/11/20 10:22, Ard Biesheuvel wrote:
> On 8/11/20 4:21 AM, matthewfcarlson@gmail.com wrote:
>> From: Matthew Carlson <macarl@microsoft.com>
>>
>
> How am I supposed to review this change? The commit log is empty and I
> was not cc'ed on the cover letter.

Cover letter:

  [edk2-devel] [PATCH v4 0/5] Use RngLib instead of TimerLib for OpensslLib

  https://edk2.groups.io/g/devel/message/63944
  http://mid.mail-archive.com/20200811022200.1087-1-matthewfcarlson@gmail.com

Bugzilla:

  https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Unfortunately, the cover letter doesn't much explain the approach
either. The latest comments in the BZ should be helpful though.

My understanding is that the timer-based "pseudo-random" generation is
factored out of "CryptoPkg/Library/OpensslLib/rand_pool_noise*" to the
new BaseRngLibTimerLib instance (see patches #1 and #5). In the middle,
platforms native to the edk2 tree and currently using "rand_pool_noise*"
are diverted to the new lib instance. (Patches #3 and #4.)

So I think the intent is to introduce no change in behavior for those
platforms, only make OpensslLib depend on the RngLib class.

Patch#2 adds BaseRngLibDxe, which depends on gEfiRngProtocolGuid.

I think the structure of the series is correct.

--*--

In edk2, we have two RNG protocol implementations,
"OvmfPkg/VirtioRngDxe" and "SecurityPkg/RandomNumberGenerator/RngDxe".
While it would be nice to use the "BaseRngLibDxe" instance in OvmfPkg
and ArmVirtPkg, *in the longer term*, I have some doubts:

- I don't know whether or how "SecurityPkg/RandomNumberGenerator/RngDxe"
applies to virtual machines.

- OvmfPkg/VirtioRngDxe does not produce gEfiRngProtocolGuid if there is
no virtio-rng-(pci|device) device configured in QEMU. So a strict depex
would not work; we'd again need some kind of OR depex.

- The ArmVirtQemu and OVMF PlatformBootManagerLib instances connect
virtio-rng-(pci|device) devices after signaling EndOfDxe. That's good
enough for boot loaders and the Linux kernel's UEFI stub, but possibly
not good enough for platform DXE drivers that need randomness before
EndOfDxe.

- The "BaseRngLibDxe" instance from patch#2 only accepts one of the
"Sp80090Ctr256", "Sp80090Hmac256", and "Sp80090Hash256" algorithms, and
"OvmfPkg/VirtioRngDxe" provides none of those.
("SecurityPkg/RandomNumberGenerator/RngDxe" seems to provide
"Sp80090Ctr256".)

But, anyway, these are just longer-term points for OvmfPkg and
ArmVirtPkg; they aren't a problem with this patch set.

> In general, please try to muster up the energy to write at least one
> sentence that describes *why* the patch is needed, complementing the
> subject line, which in this case summarizes correctly *what* the patch
> does.

Agreed.

And, in addition to the minimally one-sentence commit message body, each
commit message should reference
<https://bugzilla.tianocore.org/show_bug.cgi?id=1871>.


I'd be very happy if you could review this patch series; personally I
can only formally review patches #3 and #4.

Thanks!
Laszlo

Re: [edk2-devel] [PATCH v4 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg

[Laszlo Ersek]

On 08/11/20 04:21, Matthew Carlson wrote:
> From: Matthew Carlson <macarl@microsoft.com>
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.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..ddfcd0cf9eee 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -42,6 +42,7 @@
>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>  
>    BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>    SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
>    BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
>    SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> 

In addition to the documentation updates requested by Ard and myself
under the OvmfPkg patch (non-empty commit message body, BZ reference),
I'd like to request that we add the RngLib resolution near the
OpensslLib resolution(s). For example, right after:

!if $(NETWORK_TLS_ENABLE) == TRUE
  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
!else
  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
!endif

Thanks!
Laszlo

Re: [edk2-devel] [PATCH v4 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Laszlo Ersek]

On 08/11/20 04:21, Matthew Carlson wrote:
> From: Matthew Carlson <macarl@microsoft.com>
> 
> 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>
> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>  OvmfPkg/OvmfPkgX64.dsc     | 1 +
>  OvmfPkg/OvmfXen.dsc        | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 9178ffeb71cb..118fd1aff246 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -116,6 +116,7 @@
>  [LibraryClasses]
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>    ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>    PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index a665f78f0dc7..6b9da5b996ff 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -120,6 +120,7 @@
>  [LibraryClasses]
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>    ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>    PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 17f345acf4ee..3a354eb3a2bd 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -120,6 +120,7 @@
>  [LibraryClasses]
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>    ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>    PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 782803cb2787..f97e2b7e07d0 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -110,6 +110,7 @@
>  [LibraryClasses]
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    TimerLib|MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf
> +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>    ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
>    PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
> 

(1) This patch does not cover "OvmfPkg/Bhyve/BhyvePkgX64.dsc", which
also resolves "OpensslLib".

(2) Please add the RngLib resolution just after the "OpensslLib"
resolution(s), in each of the five DSC files.

Thank you,
Laszlo

Re: [edk2-devel] [PATCH v4 0/5] Use RngLib instead of TimerLib for OpensslLib

[Matthew Carlson]

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

Thanks Guomin!

I sent out a version 5 of this series.

--
- Matthew Carlson

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

Re: [edk2-devel] [PATCH v4 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Matthew Carlson]

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

Thank you for the helpful comments Lazlo!

I sent out an updated series (v5) that fixes the things you mentioned. I added more description, so hopefully that helps.

Sorry I didn't notice BhyvePkg, I thought you couldn't have packages under other packages, so I didn't think to check for other DSC's. It should be fixed up like the other Ovmf DSC's.

I've been following your excellent guide for sending mailing list patches (Lazlo's Guide). ( https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers ) Is there a better way to get all the CC's from all the patches other than just copy and pasting them all? Perhaps the GetMaintainers.py where you specify multiple commits? Specifying a range didn't produce the desired behavior.

--
- Matthew Carlson

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

Re: [edk2-devel] [PATCH v4 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Laszlo Ersek]

(+Rebecca)

On 08/12/20 04:27, macarl via [] wrote:
> Thank you for the helpful comments Lazlo!
> 
> I sent out an updated series (v5) that fixes the things you mentioned. I added more description, so hopefully that helps.
> 
> Sorry I didn't notice BhyvePkg, I thought you couldn't have packages under other packages, so I didn't think to check for other DSC's. It should be fixed up like the other Ovmf DSC's.

Right, with Bhyve we indeed broke the DEC spec first, having two DEC
files under OvmfPkg (this was reported by Sean). The issue was fixed in
commit e557442e3f7e; since then, bhyve is not a new package nested under
OvmfPkg (which is invalid), just a separate platform DSC.

Arguably, the "Pkg" infix in the following file names:

Bhyve/BhyvePkgDefines.fdf.inc
Bhyve/BhyvePkgX64.dsc
Bhyve/BhyvePkgX64.fdf

remains a bit confusing, and should indeed be removed:

Bhyve/BhyveDefines.fdf.inc
Bhyve/BhyveX64.dsc
Bhyve/BhyveX64.fdf

Rebecca, could you please submit a patch with such renames?

> I've been following your excellent guide for sending mailing list patches (Lazlo's Guide). ( https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers ) Is there a better way to get all the CC's from all the patches other than just copy and pasting them all? Perhaps the GetMaintainers.py where you specify multiple commits? Specifying a range didn't produce the desired behavior.

For collecting the full CC set for the cover letter, one possibility is:

$ git log master..topic \
  | grep '^Cc:' \
  | sort -u

and then cut n' paste the result of this command into the cover letter.

(I assume even on Windows the above command should work in WSL or Cygwin.)

Thanks!
Laszlo

Re: [edk2-devel] [PATCH v4 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Rebecca Cran]

On 8/12/20 4:05 AM, Laszlo Ersek wrote:

> Arguably, the "Pkg" infix in the following file names:
>
> Bhyve/BhyvePkgDefines.fdf.inc
> Bhyve/BhyvePkgX64.dsc
> Bhyve/BhyvePkgX64.fdf
>
> remains a bit confusing, and should indeed be removed:
>
> Bhyve/BhyveDefines.fdf.inc
> Bhyve/BhyveX64.dsc
> Bhyve/BhyveX64.fdf
>
> Rebecca, could you please submit a patch with such renames?

Yes, I'll submit a patch in the next couple of days - I'm still catching 
up from traveling this past week.


-- 
Rebecca Cran