[Patch v2 0/2] Use RngLib instead of TimerLib for OpensslLib

[Patch v2 0/2] Use RngLib instead of TimerLib for OpensslLib

[Matthew Carlson]

From: Matthew Carlson <macarl@microsoft.com>

This fixes bugzilla 1871.
See PR here: https://github.com/tianocore/edk2/pull/831

Matthew Carlson (2):
  CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
  MdePkg: TimerRngLib: Added RngLib that uses TimerLib

 CryptoPkg/Library/OpensslLib/rand_pool.c           | 202 ++------------------
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c     |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |  43 -----
 MdePkg/Library/BaseRngLibTimer/RngLibTimer.c       | 153 +++++++++++++++
 CryptoPkg/CryptoPkg.dsc                            |   2 +
 CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  15 +-
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
 CryptoPkg/Library/OpensslLib/rand_pool_noise.h     |  29 ---
 MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf |  37 ++++
 MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni |  17 ++
 MdePkg/MdePkg.dsc                                  |   1 +
 11 files changed, 230 insertions(+), 313 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/BaseRngLibTimer/RngLibTimer.c
 delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.h
 create mode 100644 MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf
 create mode 100644 MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni

-- 
2.27.0.windows.1

[Patch v2 1/2] 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           | 202 ++------------------
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c     |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |  43 -----
 CryptoPkg/CryptoPkg.dsc                            |   2 +
 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..b3ce71543bab 100644
--- a/CryptoPkg/Library/OpensslLib/rand_pool.c
+++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
@@ -11,43 +11,7 @@ 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
@@ -56,8 +20,8 @@ GetRandNoise64FromPerformanceCounter(
   @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 +37,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 +64,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 +83,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 +102,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 +115,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 +148,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..446a89a477e1 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]
   #
@@ -119,6 +120,7 @@
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
+  RngLib|SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
 
 [LibraryClasses.common.DXE_SMM_DRIVER]
   ReportStatusCodeLib|MdeModulePkg/Library/SmmReportStatusCodeLib/SmmReportStatusCodeLib.inf
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

[Patch v2 2/2] 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/BaseRngLibTimer/RngLibTimer.c       | 153 ++++++++++++++++++++
 MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf |  37 +++++
 MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni |  17 +++
 MdePkg/MdePkg.dsc                                  |   1 +
 4 files changed, 208 insertions(+)

diff --git a/MdePkg/Library/BaseRngLibTimer/RngLibTimer.c b/MdePkg/Library/BaseRngLibTimer/RngLibTimer.c
new file mode 100644
index 000000000000..1b0f7f04c01d
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimer/RngLibTimer.c
@@ -0,0 +1,153 @@
+/** @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 <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/TimerLib.h>
+#include <Uefi.h>
+
+/**
+  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
+  for (Index = 0; Index < 2; Index ++) {
+    *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+    MicroSecondDelay (4);
+    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
+  for (Index = 0; Index < 4; Index ++) {
+    *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+    MicroSecondDelay (2);
+    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
+  for (Index = 0; Index < 8; Index ++) {
+    *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+    MicroSecondDelay (1);
+    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);
+
+  //
+  // Read first 64 bits
+  //
+  if (!GetRandomNumber64 (Rand)) {
+    return FALSE;
+  }
+
+  //
+  // Read second 64 bits
+  //
+  return GetRandomNumber64 (++Rand);
+}
diff --git a/MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf b/MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf
new file mode 100644
index 000000000000..c5561fe04c53
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf
@@ -0,0 +1,37 @@
+## @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.
+#
+#  Copyright (c) Microsoft Corporation. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = BaseRngLibTimer
+  MODULE_UNI_FILE                = BaseRngLibTimer.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
+  DebugLib
+  TimerLib
diff --git a/MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni b/MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni
new file mode 100644
index 000000000000..e2939f441f4a
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni
@@ -0,0 +1,17 @@
+// /** @file
+// Instance of RNG (Random Number Generator) Library.
+//
+// BaseRng Library that uses CPU RdRand instruction access to provide
+// high-quality 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-quality random numbers"
+
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 472fa3777412..e1151cf2e0b9 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -62,6 +62,7 @@
   MdePkg/Library/BasePostCodeLibPort80/BasePostCodeLibPort80.inf
   MdePkg/Library/BasePrintLib/BasePrintLib.inf
   MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
+  MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf
   MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
-- 
2.27.0.windows.1

Re: [edk2-devel] [Patch v2 0/2] Use RngLib instead of TimerLib for OpensslLib

[Michael D Kinney]

Hi Matthew,

A few comments:

1) MdePkg/Libray/BaseRngLibTimer
  a) Since this implementation of the RngLib class is layered
     on top of any TimerLib instance a platform selects, the 
     dir/name of the lib should be BasRngLibTimerLib.
  b) BaseRngLibTimer.inf - I see the comment that it should not 
     be used in a production system.  I think you should add the
     reason why that it should not be used in production system
     which is that using the performance counter from a TimerLib
     is not a true source of random values.
  c) BaseRngLibTimer.uni - The file header description is incorrect.
     It refers to CPU RdRand.  The UNI string also makes reference
     to low-quality random numbers.  I am not sure if there is a
     clear definition of low-quality random numbers.  Should update
     to match description in INF.
  d) RngLibTimer.c - This is a library of type BASE.  Should include
     <Base.h> and not <Uefi.h>.  Also <Base.h> should be listed first.
  e) RngLibTimer.c - I see use of calls to MicroSecondDelay() for 
     small values such as 1, 2, 4 without any comments that explain 
     why this call is made and how the values were selected.  One 
     aspect of this is that depending on the rate of the counter
     used by the GetPerformanceCounter(), the use of these small
     values to MicroSecondDelay() may all produce results where the
     counter is only incremented by 1.  This algorithms is more
     effective if the rate of the counter is much larger than 1MHz.
     Should the number microseconds that are used be based on 
     the results from GetPerofrmanceCounterProperties()?

2) CryptoPkg/CryptoPkg.dsc.  The update to this DSC file adds a 
   dependency on the SecurityPkg.  The SecurityPkg can depend on
   the CryptoPkg, but the CryptoPkg can not depend on the SecurityPkg.
   For a package verification build, perhaps we should always use
   BaseRngLibNull.  Platform DSC files can choose to use the real
   RngLib instances.

3) rand_pool.c/RandGetBytes() - Typo in function header.  Return
   values are TRUE and FALSE.

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Matthew Carlson
> Sent: Thursday, July 30, 2020 10:21 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch v2 0/2] Use RngLib instead
> of TimerLib for OpensslLib
> 
> From: Matthew Carlson <macarl@microsoft.com>
> 
> This fixes bugzilla 1871.
> See PR here: https://github.com/tianocore/edk2/pull/831
> 
> Matthew Carlson (2):
>   CryptoPkg: OpensslLib: Use RngLib to generate entropy
> in rand_pool
>   MdePkg: TimerRngLib: Added RngLib that uses TimerLib
> 
>  CryptoPkg/Library/OpensslLib/rand_pool.c           |
> 202 ++------------------
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.c     |
> 29 ---
>  CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |
> 43 -----
>  MdePkg/Library/BaseRngLibTimer/RngLibTimer.c       |
> 153 +++++++++++++++
>  CryptoPkg/CryptoPkg.dsc                            |
> 2 +
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf        |
> 15 +-
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |
> 15 +-
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.h     |
> 29 ---
>  MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf |
> 37 ++++
>  MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni |
> 17 ++
>  MdePkg/MdePkg.dsc                                  |
> 1 +
>  11 files changed, 230 insertions(+), 313 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/BaseRngLibTimer/RngLibTimer.c
>  delete mode 100644
> CryptoPkg/Library/OpensslLib/rand_pool_noise.h
>  create mode 100644
> MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.inf
>  create mode 100644
> MdePkg/Library/BaseRngLibTimer/BaseRngLibTimer.uni
> 
> --
> 2.27.0.windows.1
> 
> 
> 

Re: [edk2-devel] [Patch v2 0/2] Use RngLib instead of TimerLib for OpensslLib

[Matthew Carlson]

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

Hey Mike!

Thanks for the comments. I've addressed the feedback below.

1. All good comments. I think I've addressed them all. New patch series should be coming soon.

2. This is a problem for BaseCryptLibOnProtocol. We can't use a null lib for RngLib since it explodes when it tries to seed OpenSSL with entropy. I think for now, let's stick with established dependencies and figure this out at a later date. I added an RngLib that uses the DXE RNG Protocol.

3. I don't see a typo?

Calls RandomNumber64 to fill
a buffer of arbitrary size with random bytes.

@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 True        Random bytes generation succeeded.
@retval False       Failed to request random bytes.

--
- Matthew Carlson

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

Re: [edk2-devel] [Patch v2 0/2] Use RngLib instead of TimerLib for OpensslLib

[Michael D Kinney]

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

Hi Matt,

I know you can use the Null lib and have the module be function.  The Package DSC is for build verification.  Does not have to be functional.  We already have examples in the Security using a Null BaseCryptLib to improve build verification performance, but of course those security modules as built would not run.

I know one goal is to build functional binaries from the CryptoPkg.  Perhaps that should be a different DSC file?

The typo is True -> TRUE and False -> FALSE

Mike

From: macarl via [] <macarl=microsoft.com@[]>
Sent: Friday, July 31, 2020 1:16 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [Patch v2 0/2] Use RngLib instead of TimerLib for OpensslLib


Hey Mike!

Thanks for the comments. I've addressed the feedback below.

1. All good comments. I think I've addressed them all. New patch series should be coming soon.

2. This is a problem for BaseCryptLibOnProtocol. We can't use a null lib for RngLib since it explodes when it tries to seed OpenSSL with entropy. I think for now, let's stick with established dependencies and figure this out at a later date. I added an RngLib that uses the DXE RNG Protocol.

3. I don't see a typo?

  Calls RandomNumber64 to fill

  a buffer of arbitrary size with random bytes.



  @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 True        Random bytes generation succeeded.

  @retval False       Failed to request random bytes.



--
- Matthew Carlson

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

Re: [edk2-devel] [Patch v2 0/2] Use RngLib instead of TimerLib for OpensslLib

[Matthew Carlson]

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

I included a null lib and I was including the wrong version of Rng anyway.

I think that would make sense to split them into two different DSC files.

Typo fixed, good spot!
--
- Matthew Carlson

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