public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib
@ 2020-07-31 20:27 Matthew Carlson
  2020-07-31 20:27 ` [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-07-31 20:27 UTC (permalink / raw)
  To: devel

From: Matthew Carlson <macarl@microsoft.com>

Fixes Bugzilla#1871
https://github.com/tianocore/edk2/pull/845


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

 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                 | 216 ++++++++++++++++++++
 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c          | 154 ++++++++++++++
 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 |  38 ++++
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  17 ++
 MdePkg/MdePkg.dsc                                        |   5 +-
 13 files changed, 489 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


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

* [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
  2020-07-31 20:27 [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
@ 2020-07-31 20:27 ` Matthew Carlson
  2020-08-01  0:25   ` Yao, Jiewen
       [not found]   ` <1626FD395A4E4B04.26980@groups.io>
  2020-07-31 20:27 ` [PATCH v3 2/3] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-07-31 20:27 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Xiaoyu Lu, 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..b3ff03b2aa13 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


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

* [PATCH v3 2/3] MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  2020-07-31 20:27 [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
  2020-07-31 20:27 ` [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
@ 2020-07-31 20:27 ` Matthew Carlson
  2020-07-31 20:27 ` [PATCH v3 3/3] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe Matthew Carlson
  2020-07-31 21:17 ` [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Michael D Kinney
  3 siblings, 0 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-07-31 20:27 UTC (permalink / raw)
  To: devel; +Cc: 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.

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

diff --git a/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
new file mode 100644
index 000000000000..8244a740555c
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
@@ -0,0 +1,154 @@
+/** @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>
+
+
+/**
+  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 10ms
+  for (Index = 0; Index < 2; Index ++) {
+    *RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+    MicroSecondDelay (5); // 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);
+    MicroSecondDelay (5); // 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);
+    MicroSecondDelay (5); // 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..0d20fd13c1da
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
@@ -0,0 +1,38 @@
+## @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 have a weak random
+#  algorithm.
+#
+#  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


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

* [PATCH v3 3/3] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
  2020-07-31 20:27 [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
  2020-07-31 20:27 ` [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
  2020-07-31 20:27 ` [PATCH v3 2/3] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
@ 2020-07-31 20:27 ` Matthew Carlson
  2020-07-31 21:17 ` [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Michael D Kinney
  3 siblings, 0 replies; 11+ messages in thread
From: Matthew Carlson @ 2020-07-31 20:27 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, 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       | 216 ++++++++++++++++++++
 MdePkg/Library/BaseRngLibDxe/BaseRngLibDxe.inf |  38 ++++
 MdePkg/MdePkg.dsc                              |   4 +-
 3 files changed, 257 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseRngLibDxe/RngDxeLib.c b/MdePkg/Library/BaseRngLibDxe/RngDxeLib.c
new file mode 100644
index 000000000000..45bd6bad8f7a
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibDxe/RngDxeLib.c
@@ -0,0 +1,216 @@
+/** @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
+
+MU_CHANGE: New file
+
+**/
+#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_SUCCESS;
+  EFI_RNG_PROTOCOL* RngProtocol = NULL;
+
+  if (Buffer == NULL)
+  {
+      DEBUG((DEBUG_ERROR, "[%a] Buffer == NULL.\n", __FUNCTION__));
+      return EFI_INVALID_PARAMETER;
+  }
+
+  if (gBS == NULL)
+  {
+      DEBUG((DEBUG_ERROR, "[%a] GenerateRandomNumber, gBS == NULL.  Called too soon.\n", __FUNCTION__));
+      return EFI_LOAD_ERROR;
+  }
+
+  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


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

* Re: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib
  2020-07-31 20:27 [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
                   ` (2 preceding siblings ...)
  2020-07-31 20:27 ` [PATCH v3 3/3] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe Matthew Carlson
@ 2020-07-31 21:17 ` Michael D Kinney
  2020-08-03  7:03   ` Liming Gao
  3 siblings, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2020-07-31 21:17 UTC (permalink / raw)
  To: devel@edk2.groups.io, matthewfcarlson@gmail.com,
	Kinney, Michael D

Hi Matt,

1) BaseRngLibTimerLib
  a) The comments incorrectly list delays in ms instead of us.
  b) Did you consider use of GetPerformanceCounterProperties()?
     I also do not seen an explanation of the delay values used.
     (why not smaller or larger values).  At a minimum, the file
     header should state it only works if the rate of the perf
     counter from TimerLib is much greater than 1MHz.
2) BaseRngLibDxe
   a) This is not a lib of type Base.  I recommend the name
      DxeRngLibRngProtocol.
   b) Has a "MU_CHANGE" comment that can be removed
   c) GenerateRandomNumberViaNist800Algorithm() assigns values in
      declaration.  Init should be moved into statements.
   d) How would gBS aver be NULL?  The INF lists the BootServicesTableLib
      as a dependency, so the constructer is always run before the
      services are used.  I think these checks can be removed.
   e) Minor code style issues.  if statements should have { as end of line.

With the addition of DxeRngLibRngProtocol to MdePkg, I think the CryptoPkg
DSC can be updated to use this RngLib instance from the Crypto DXE mododule.

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Matthew Carlson
> Sent: Friday, July 31, 2020 1:27 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v3 0/3] Use RngLib instead
> of TimerLib for OpensslLib
> 
> From: Matthew Carlson <macarl@microsoft.com>
> 
> Fixes Bugzilla#1871
> https://github.com/tianocore/edk2/pull/845
> 
> 
> Matthew Carlson (3):
>   CryptoPkg: OpensslLib: Use RngLib to generate entropy
> in rand_pool
>   MdePkg: TimerRngLib: Added RngLib that uses TimerLib
>   MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
> 
>  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
> | 216 ++++++++++++++++++++
>  MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
> | 154 ++++++++++++++
>  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.in
> f |  38 ++++
> 
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> i |  17 ++
>  MdePkg/MdePkg.dsc
> |   5 +-
>  13 files changed, 489 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.in
> f
>  create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> i
> 
> --
> 2.27.0.windows.1
> 
> 
> 


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

* Re: [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
  2020-07-31 20:27 ` [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
@ 2020-08-01  0:25   ` Yao, Jiewen
       [not found]   ` <1626FD395A4E4B04.26980@groups.io>
  1 sibling, 0 replies; 11+ messages in thread
From: Yao, Jiewen @ 2020-08-01  0:25 UTC (permalink / raw)
  To: matthewfcarlson@gmail.com, devel@edk2.groups.io; +Cc: Wang, Jian J, Lu, XiaoyuX

Hi
I have read https://bugzilla.tianocore.org/show_bug.cgi?id=1871
I would like to give R-B, because the code matches what described in Bugzilla.

Before that, I would like double confirm on the randomness requirement.
According to https://software.intel.com/content/www/us/en/develop/blogs/the-difference-between-rdrand-and-rdseed.html, the RDSEED is a "Non-deterministic random bit generator", while RDRAND is a "Cryptographically secure pseudorandom number generator"

Before this patch:
rand_pool_acquire_entropy()-> RandGetSeed128()->MicroSecondDelay()+RandGetBytes()->GetRandomNoise64()->AsmReadTsc()+MicroSecondDelay().
rand_pool_add_nonce_data()->GetPerformanceCounter()+RandGetBytes()
It seems return TSC and TimerCounter.

After this patch:
rand_pool_acquire_entropy()->RandGetBytes()->GetRandomNumber64()->AsmRdRand64().
rand_pool_add_nonce_data()->RandGetBytes()
It becomes pseudorandom.

So the meaning of the function seems changed.
I have not checked the randomness requirement for those two functions yet.
But could anyone confirm that a pseudorandom value returned is OK?

Or should we use RDSEED for non-deterministic value?

Thank you
Yao Jiewen


> -----Original Message-----
> From: matthewfcarlson@gmail.com <matthewfcarlson@gmail.com>
> Sent: Saturday, August 1, 2020 4:27 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Lu, XiaoyuX <xiaoyux.lu@intel.com>; Matthew Carlson
> <matthewfcarlson@gmail.com>
> Subject: [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy
> in rand_pool
> 
> 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..b3ff03b2aa13 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


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

* Re: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib
  2020-07-31 21:17 ` [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Michael D Kinney
@ 2020-08-03  7:03   ` Liming Gao
  2020-08-03 17:21     ` Sean
  0 siblings, 1 reply; 11+ messages in thread
From: Liming Gao @ 2020-08-03  7:03 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D,
	matthewfcarlson@gmail.com

Hi Matt: 
  With this change, which library instance should be used for the platform? BaseRngLibTimerLib?

  And, this patch introduces the dependency for RngLib. Then, NetworkPkg and FmpDevicePkg package level build will break. Will you update them?

Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
Sent: 2020年8月1日 5:17
To: devel@edk2.groups.io; matthewfcarlson@gmail.com; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib

Hi Matt,

1) BaseRngLibTimerLib
  a) The comments incorrectly list delays in ms instead of us.
  b) Did you consider use of GetPerformanceCounterProperties()?
     I also do not seen an explanation of the delay values used.
     (why not smaller or larger values).  At a minimum, the file
     header should state it only works if the rate of the perf
     counter from TimerLib is much greater than 1MHz.
2) BaseRngLibDxe
   a) This is not a lib of type Base.  I recommend the name
      DxeRngLibRngProtocol.
   b) Has a "MU_CHANGE" comment that can be removed
   c) GenerateRandomNumberViaNist800Algorithm() assigns values in
      declaration.  Init should be moved into statements.
   d) How would gBS aver be NULL?  The INF lists the BootServicesTableLib
      as a dependency, so the constructer is always run before the
      services are used.  I think these checks can be removed.
   e) Minor code style issues.  if statements should have { as end of line.

With the addition of DxeRngLibRngProtocol to MdePkg, I think the CryptoPkg DSC can be updated to use this RngLib instance from the Crypto DXE mododule.

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Matthew 
> Carlson
> Sent: Friday, July 31, 2020 1:27 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib 
> for OpensslLib
> 
> From: Matthew Carlson <macarl@microsoft.com>
> 
> Fixes Bugzilla#1871
> https://github.com/tianocore/edk2/pull/845
> 
> 
> Matthew Carlson (3):
>   CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
>   MdePkg: TimerRngLib: Added RngLib that uses TimerLib
>   MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
> 
>  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
> | 216 ++++++++++++++++++++
>  MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
> | 154 ++++++++++++++
>  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.in
> f |  38 ++++
> 
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> i |  17 ++
>  MdePkg/MdePkg.dsc
> |   5 +-
>  13 files changed, 489 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.in
> f
>  create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> i
> 
> --
> 2.27.0.windows.1
> 
> 
> 





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

* Re: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib
  2020-08-03  7:03   ` Liming Gao
@ 2020-08-03 17:21     ` Sean
  2020-08-03 22:20       ` Michael D Kinney
  0 siblings, 1 reply; 11+ messages in thread
From: Sean @ 2020-08-03 17:21 UTC (permalink / raw)
  To: devel, liming.gao, Kinney, Michael D, matthewfcarlson@gmail.com

I would rather see the the BaseCryptLib and TlsLib instance in all 
packages other than CryptoPkg should use the NULL instance of the libs. 
This would avoid the dependency.  Any concerns of going this route?

Thanks
Sean


On 8/3/2020 12:03 AM, Liming Gao wrote:
> Hi Matt:
>    With this change, which library instance should be used for the platform? BaseRngLibTimerLib?
> 
>    And, this patch introduces the dependency for RngLib. Then, NetworkPkg and FmpDevicePkg package level build will break. Will you update them?
> 
> Thanks
> Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
> Sent: 2020年8月1日 5:17
> To: devel@edk2.groups.io; matthewfcarlson@gmail.com; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib
> 
> Hi Matt,
> 
> 1) BaseRngLibTimerLib
>    a) The comments incorrectly list delays in ms instead of us.
>    b) Did you consider use of GetPerformanceCounterProperties()?
>       I also do not seen an explanation of the delay values used.
>       (why not smaller or larger values).  At a minimum, the file
>       header should state it only works if the rate of the perf
>       counter from TimerLib is much greater than 1MHz.
> 2) BaseRngLibDxe
>     a) This is not a lib of type Base.  I recommend the name
>        DxeRngLibRngProtocol.
>     b) Has a "MU_CHANGE" comment that can be removed
>     c) GenerateRandomNumberViaNist800Algorithm() assigns values in
>        declaration.  Init should be moved into statements.
>     d) How would gBS aver be NULL?  The INF lists the BootServicesTableLib
>        as a dependency, so the constructer is always run before the
>        services are used.  I think these checks can be removed.
>     e) Minor code style issues.  if statements should have { as end of line.
> 
> With the addition of DxeRngLibRngProtocol to MdePkg, I think the CryptoPkg DSC can be updated to use this RngLib instance from the Crypto DXE mododule.
> 
> Best regards,
> 
> Mike
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Matthew
>> Carlson
>> Sent: Friday, July 31, 2020 1:27 PM
>> To: devel@edk2.groups.io
>> Subject: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib
>> for OpensslLib
>>
>> From: Matthew Carlson <macarl@microsoft.com>
>>
>> Fixes Bugzilla#1871
>> https://github.com/tianocore/edk2/pull/845
>>
>>
>> Matthew Carlson (3):
>>    CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
>>    MdePkg: TimerRngLib: Added RngLib that uses TimerLib
>>    MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
>>
>>   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
>> | 216 ++++++++++++++++++++
>>   MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
>> | 154 ++++++++++++++
>>   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.in
>> f |  38 ++++
>>
>> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
>> i |  17 ++
>>   MdePkg/MdePkg.dsc
>> |   5 +-
>>   13 files changed, 489 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.in
>> f
>>   create mode 100644
>> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
>> i
>>
>> --
>> 2.27.0.windows.1
>>
>>
>>
> 
> 
> 
> 
> 
> 
> 

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

* Re: [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib
  2020-08-03 17:21     ` Sean
@ 2020-08-03 22:20       ` Michael D Kinney
  0 siblings, 0 replies; 11+ messages in thread
From: Michael D Kinney @ 2020-08-03 22:20 UTC (permalink / raw)
  To: Sean Brogan, devel@edk2.groups.io, Gao, Liming,
	matthewfcarlson@gmail.com, Kinney, Michael D

Hi Sean,

Use of BaseCryptLibNull and TlsLibNull in packages other than
CryptoPkg does reduce build times for package verification builds.
So that sounds like a good idea to reduce CI overhead.

I would add that platform packages that build bootable FW images
should not use the Null instances.  Platforms need to make sure
they also choose the right RngLib instance and we would not
recommend use of the timer based RngLib.

Mike

> -----Original Message-----
> From: Sean Brogan <spbrogan@outlook.com>
> Sent: Monday, August 3, 2020 10:21 AM
> To: devel@edk2.groups.io; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; matthewfcarlson@gmail.com
> Subject: Re: [edk2-devel] [PATCH v3 0/3] Use RngLib
> instead of TimerLib for OpensslLib
> 
> I would rather see the the BaseCryptLib and TlsLib
> instance in all
> packages other than CryptoPkg should use the NULL
> instance of the libs.
> This would avoid the dependency.  Any concerns of going
> this route?
> 
> Thanks
> Sean
> 
> 
> On 8/3/2020 12:03 AM, Liming Gao wrote:
> > Hi Matt:
> >    With this change, which library instance should be
> used for the platform? BaseRngLibTimerLib?
> >
> >    And, this patch introduces the dependency for
> RngLib. Then, NetworkPkg and FmpDevicePkg package level
> build will break. Will you update them?
> >
> > Thanks
> > Liming
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Michael D Kinney
> > Sent: 2020年8月1日 5:17
> > To: devel@edk2.groups.io; matthewfcarlson@gmail.com;
> Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v3 0/3] Use RngLib
> instead of TimerLib for OpensslLib
> >
> > Hi Matt,
> >
> > 1) BaseRngLibTimerLib
> >    a) The comments incorrectly list delays in ms
> instead of us.
> >    b) Did you consider use of
> GetPerformanceCounterProperties()?
> >       I also do not seen an explanation of the delay
> values used.
> >       (why not smaller or larger values).  At a
> minimum, the file
> >       header should state it only works if the rate of
> the perf
> >       counter from TimerLib is much greater than 1MHz.
> > 2) BaseRngLibDxe
> >     a) This is not a lib of type Base.  I recommend
> the name
> >        DxeRngLibRngProtocol.
> >     b) Has a "MU_CHANGE" comment that can be removed
> >     c) GenerateRandomNumberViaNist800Algorithm()
> assigns values in
> >        declaration.  Init should be moved into
> statements.
> >     d) How would gBS aver be NULL?  The INF lists the
> BootServicesTableLib
> >        as a dependency, so the constructer is always
> run before the
> >        services are used.  I think these checks can be
> removed.
> >     e) Minor code style issues.  if statements should
> have { as end of line.
> >
> > With the addition of DxeRngLibRngProtocol to MdePkg, I
> think the CryptoPkg DSC can be updated to use this
> RngLib instance from the Crypto DXE mododule.
> >
> > Best regards,
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Matthew
> >> Carlson
> >> Sent: Friday, July 31, 2020 1:27 PM
> >> To: devel@edk2.groups.io
> >> Subject: [edk2-devel] [PATCH v3 0/3] Use RngLib
> instead of TimerLib
> >> for OpensslLib
> >>
> >> From: Matthew Carlson <macarl@microsoft.com>
> >>
> >> Fixes Bugzilla#1871
> >> https://github.com/tianocore/edk2/pull/845
> >>
> >>
> >> Matthew Carlson (3):
> >>    CryptoPkg: OpensslLib: Use RngLib to generate
> entropy in rand_pool
> >>    MdePkg: TimerRngLib: Added RngLib that uses
> TimerLib
> >>    MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
> >>
> >>   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
> >> | 216 ++++++++++++++++++++
> >>   MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
> >> | 154 ++++++++++++++
> >>   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.in
> >> f |  38 ++++
> >>
> >>
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> >> i |  17 ++
> >>   MdePkg/MdePkg.dsc
> >> |   5 +-
> >>   13 files changed, 489 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.in
> >> f
> >>   create mode 100644
> >>
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.un
> >> i
> >>
> >> --
> >> 2.27.0.windows.1
> >>
> >>
> >>
> >
> >
> >
> >
> >
> > 
> >

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

* Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
       [not found]   ` <1626FD395A4E4B04.26980@groups.io>
@ 2020-08-13 14:44     ` Yao, Jiewen
       [not found]     ` <162ADB0D165E4BBB.11996@groups.io>
  1 sibling, 0 replies; 11+ messages in thread
From: Yao, Jiewen @ 2020-08-13 14:44 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, matthewfcarlson@gmail.com
  Cc: Wang, Jian J, Lu, XiaoyuX

Hi Matthew Carlson
Do you have any thought on the feedback below?

Do you make any update in your patch V6?



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Saturday, August 1, 2020 8:26 AM
> To: matthewfcarlson@gmail.com; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to
> generate entropy in rand_pool
> 
> Hi
> I have read https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> I would like to give R-B, because the code matches what described in Bugzilla.
> 
> Before that, I would like double confirm on the randomness requirement.
> According to
> https://software.intel.com/content/www/us/en/develop/blogs/the-difference-
> between-rdrand-and-rdseed.html, the RDSEED is a "Non-deterministic random
> bit generator", while RDRAND is a "Cryptographically secure pseudorandom
> number generator"
> 
> Before this patch:
> rand_pool_acquire_entropy()-> RandGetSeed128()-
> >MicroSecondDelay()+RandGetBytes()->GetRandomNoise64()-
> >AsmReadTsc()+MicroSecondDelay().
> rand_pool_add_nonce_data()->GetPerformanceCounter()+RandGetBytes()
> It seems return TSC and TimerCounter.
> 
> After this patch:
> rand_pool_acquire_entropy()->RandGetBytes()->GetRandomNumber64()-
> >AsmRdRand64().
> rand_pool_add_nonce_data()->RandGetBytes()
> It becomes pseudorandom.
> 
> So the meaning of the function seems changed.
> I have not checked the randomness requirement for those two functions yet.
> But could anyone confirm that a pseudorandom value returned is OK?
> 
> Or should we use RDSEED for non-deterministic value?
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: matthewfcarlson@gmail.com <matthewfcarlson@gmail.com>
> > Sent: Saturday, August 1, 2020 4:27 AM
> > To: devel@edk2.groups.io
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>;
> > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Matthew Carlson
> > <matthewfcarlson@gmail.com>
> > Subject: [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate
> entropy
> > in rand_pool
> >
> > 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..b3ff03b2aa13 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
> 
> 
> 


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

* Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
       [not found]     ` <162ADB0D165E4BBB.11996@groups.io>
@ 2020-08-13 15:10       ` Yao, Jiewen
  0 siblings, 0 replies; 11+ messages in thread
From: Yao, Jiewen @ 2020-08-13 15:10 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, matthewfcarlson@gmail.com
  Cc: Wang, Jian J, Lu, XiaoyuX

OK. I just see you describe that in v6 0/5 (not in v6 3/5 or Bugzilla 1871).

However, 0/5 will not be committed and information might be lost.

Would you please add your response in https://bugzilla.tianocore.org/show_bug.cgi?id=1871 ?



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, August 13, 2020 10:44 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>;
> matthewfcarlson@gmail.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to
> generate entropy in rand_pool
> 
> Hi Matthew Carlson
> Do you have any thought on the feedback below?
> 
> Do you make any update in your patch V6?
> 
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> > Sent: Saturday, August 1, 2020 8:26 AM
> > To: matthewfcarlson@gmail.com; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to
> > generate entropy in rand_pool
> >
> > Hi
> > I have read https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> > I would like to give R-B, because the code matches what described in Bugzilla.
> >
> > Before that, I would like double confirm on the randomness requirement.
> > According to
> > https://software.intel.com/content/www/us/en/develop/blogs/the-
> difference-
> > between-rdrand-and-rdseed.html, the RDSEED is a "Non-deterministic random
> > bit generator", while RDRAND is a "Cryptographically secure pseudorandom
> > number generator"
> >
> > Before this patch:
> > rand_pool_acquire_entropy()-> RandGetSeed128()-
> > >MicroSecondDelay()+RandGetBytes()->GetRandomNoise64()-
> > >AsmReadTsc()+MicroSecondDelay().
> > rand_pool_add_nonce_data()->GetPerformanceCounter()+RandGetBytes()
> > It seems return TSC and TimerCounter.
> >
> > After this patch:
> > rand_pool_acquire_entropy()->RandGetBytes()->GetRandomNumber64()-
> > >AsmRdRand64().
> > rand_pool_add_nonce_data()->RandGetBytes()
> > It becomes pseudorandom.
> >
> > So the meaning of the function seems changed.
> > I have not checked the randomness requirement for those two functions yet.
> > But could anyone confirm that a pseudorandom value returned is OK?
> >
> > Or should we use RDSEED for non-deterministic value?
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -----Original Message-----
> > > From: matthewfcarlson@gmail.com <matthewfcarlson@gmail.com>
> > > Sent: Saturday, August 1, 2020 4:27 AM
> > > To: devel@edk2.groups.io
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>;
> > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Matthew Carlson
> > > <matthewfcarlson@gmail.com>
> > > Subject: [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate
> > entropy
> > > in rand_pool
> > >
> > > 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..b3ff03b2aa13 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
> >
> >
> >
> 
> 
> 


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

end of thread, other threads:[~2020-08-13 15:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-31 20:27 [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Matthew Carlson
2020-07-31 20:27 ` [PATCH v3 1/3] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool Matthew Carlson
2020-08-01  0:25   ` Yao, Jiewen
     [not found]   ` <1626FD395A4E4B04.26980@groups.io>
2020-08-13 14:44     ` [edk2-devel] " Yao, Jiewen
     [not found]     ` <162ADB0D165E4BBB.11996@groups.io>
2020-08-13 15:10       ` Yao, Jiewen
2020-07-31 20:27 ` [PATCH v3 2/3] MdePkg: TimerRngLib: Added RngLib that uses TimerLib Matthew Carlson
2020-07-31 20:27 ` [PATCH v3 3/3] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe Matthew Carlson
2020-07-31 21:17 ` [edk2-devel] [PATCH v3 0/3] Use RngLib instead of TimerLib for OpensslLib Michael D Kinney
2020-08-03  7:03   ` Liming Gao
2020-08-03 17:21     ` Sean
2020-08-03 22:20       ` Michael D Kinney

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