public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@linaro.org>
To: Jian J Wang <jian.j.wang@intel.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
	Xiaoyu Lu <xiaoyux.lu@intel.com>,
	 Laszlo Ersek <lersek@redhat.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	 Chao Zhang <chao.b.zhang@intel.com>,
	Liming Gao <liming.gao@intel.com>, Ray Ni <ray.ni@intel.com>
Subject: Re: [PATCH 10/11] CryptoPkg/OpensslLib: use RngLib to get high quality random entropy
Date: Thu, 14 Nov 2019 07:42:03 +0000	[thread overview]
Message-ID: <CAKv+Gu9ng_Gcjzpo_k58Kn=p6GMDb6SPVMtg6Qfd+phXLuk_Jw@mail.gmail.com> (raw)
In-Reply-To: <20191114021743.3876-11-jian.j.wang@intel.com>

On Thu, 14 Nov 2019 at 02:18, Jian J Wang <jian.j.wang@intel.com> wrote:
>
> Per BZ1871, OpensslLib should use RngLib to get high quality of random
> entropy. This patch remove all code depending on TimerLib for this job,
> and add RngLib interface to implement the rand interface required by
> openssl.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  CryptoPkg/CryptoPkg.dsc                       |   1 +
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf   |  15 +-
>  .../Library/OpensslLib/OpensslLibCrypto.inf   |  15 +-
>  CryptoPkg/Library/OpensslLib/rand_pool.c      | 253 ++----------------
>  .../Library/OpensslLib/rand_pool_noise.c      |  29 --
>  .../Library/OpensslLib/rand_pool_noise.h      |  29 --
>  .../Library/OpensslLib/rand_pool_noise_tsc.c  |  43 ---
>  7 files changed, 32 insertions(+), 353 deletions(-)
>  delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.c
>  delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.h
>  delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c
>
> diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
> index ec43c1f0a4..54f892996a 100644
> --- a/CryptoPkg/CryptoPkg.dsc
> +++ b/CryptoPkg/CryptoPkg.dsc
> @@ -44,6 +44,7 @@
>
>    IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.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 b28dd9e480..4c535dc1e6 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -23,7 +23,6 @@
>
>  [Sources]
>    buildinf.h
> -  rand_pool_noise.h
>    $(OPENSSL_PATH)/e_os.h
>  # Autogenerated files list starts here
>    $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
> @@ -602,18 +601,6 @@
>    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
> @@ -621,7 +608,7 @@
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> -  TimerLib
> +  RngLib
>    PrintLib
>
>  [LibraryClasses.ARM]
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index 1b6ff5ed54..51159a6f2d 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> @@ -547,22 +547,9 @@
>    $(OPENSSL_PATH)/crypto/objects/obj_xref.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
> @@ -570,7 +557,7 @@
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> -  TimerLib
> +  RngLib
>    PrintLib
>
>  [LibraryClasses.ARM]
> diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c b/CryptoPkg/Library/OpensslLib/rand_pool.c
> index 9d2a4ad138..f57c238fc4 100644
> --- a/CryptoPkg/Library/OpensslLib/rand_pool.c
> +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
> @@ -11,213 +11,9 @@ 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;
> -}
> -
> -/**
> -  Calls RandomNumber64 to fill
> -  a buffer of arbitrary size with random bytes.
> -
> -  @param[in]   Length        Size of the buffer, in bytes,  to fill with.
> -  @param[out]  RandBuffer    Pointer to the buffer to store the random result.
> -
> -  @retval EFI_SUCCESS        Random bytes generation succeeded.
> -  @retval EFI_NOT_READY      Failed to request random bytes.
> -
> -**/
> -STATIC
> -BOOLEAN
> -EFIAPI
> -RandGetBytes (
> -  IN UINTN         Length,
> -  OUT UINT8        *RandBuffer
> -  )
> -{
> -  BOOLEAN     Ret;
> -  UINT64      TempRand;
> -
> -  Ret = FALSE;
> -
> -  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);
> -    }
> -    if (!Ret) {
> -      return Ret;
> -    }
> -    if (Length >= sizeof (TempRand)) {
> -      *((UINT64*) RandBuffer) = TempRand;
> -      RandBuffer += sizeof (UINT64);
> -      Length -= sizeof (TempRand);
> -    } else {
> -      CopyMem (RandBuffer, &TempRand, Length);
> -      Length = 0;
> -    }
> -  }
> -
> -  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 arbitary 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;
> -}
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/RngLib.h>
>
>  /*
>   * Add random bytes to the pool to acquire requested amount of entropy
> @@ -229,17 +25,30 @@ RandGenerateEntropy (
>   */
>  size_t rand_pool_acquire_entropy(RAND_POOL *pool)
>  {
> -  BOOLEAN  Ret;
> -  size_t bytes_needed;
> -  unsigned char * buffer;
> +  BOOLEAN         ret;
> +  size_t          bytes_needed;
> +  size_t          len;
> +  unsigned char   *buffer;
> +  UINT64          data[2];
>
>    bytes_needed = rand_pool_bytes_needed(pool, 1 /*entropy_factor*/);
>    if (bytes_needed > 0) {
>      buffer = rand_pool_add_begin(pool, bytes_needed);
>
>      if (buffer != NULL) {
> -      Ret = RandGenerateEntropy(bytes_needed, buffer);
> -      if (FALSE == Ret) {
> +      ret = TRUE;
> +      while (bytes_needed > 0 && ret) {
> +        ret = GetRandomNumber128 (data);
> +        if (ret) {
> +          len = MIN (bytes_needed, sizeof(data));
> +          CopyMem (buffer, data, len);
> +
> +          bytes_needed  -= len;
> +          buffer        += len;
> +        }
> +      }
> +
> +      if (FALSE == ret) {
>          rand_pool_add_end(pool, 0, 0);
>        } else {
>          rand_pool_add_end(pool, bytes_needed, 8 * bytes_needed);
> @@ -257,13 +66,11 @@ 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 };
> +  UINT64    data[2];
>
> -  RandGetBytes(8, (UINT8 *)&(data.Rand));
> -  data.TimerValue = GetPerformanceCounter();
> +  if (!GetRandomNumber128 (data)) {
> +    return 0;
> +  }
>
>    return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0);
>  }
> @@ -275,13 +82,11 @@ 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 };
> +  UINT64    data[2];
>
> -  RandGetBytes(8, (UINT8 *)&(data.Rand));
> -  data.TimerValue = GetPerformanceCounter();
> +  if (!GetRandomNumber128 (data)) {
> +    return 0;
> +  }
>
>    return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0);
>  }
> diff --git a/CryptoPkg/Library/OpensslLib/rand_pool_noise.c b/CryptoPkg/Library/OpensslLib/rand_pool_noise.c
> deleted file mode 100644
> index c16ed8b454..0000000000
> --- 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 PerformaceCounter to
> -  // generate noise.
> -  //
> -  return FALSE;
> -}
> diff --git a/CryptoPkg/Library/OpensslLib/rand_pool_noise.h b/CryptoPkg/Library/OpensslLib/rand_pool_noise.h
> deleted file mode 100644
> index 75acc686a9..0000000000
> --- 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__
> 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 4158106231..0000000000
> --- 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;
> -}
> --
> 2.17.1.windows.2
>

  reply	other threads:[~2019-11-14  7:42 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14  2:17 [PATCH 00/11] Use proper entropy sources Wang, Jian J
2019-11-14  2:17 ` [PATCH 01/11] NetworkPkg/NetworkPkg.dsc: specify RngLib instance for build Wang, Jian J
2019-11-14  2:17 ` [PATCH 02/11] SignedCapsulePkg/SignedCapsulePkg.dsc: specify RngLib instances Wang, Jian J
2019-11-14  2:17 ` [PATCH 03/11] FmpDevicePkg/FmpDevicePkg.dsc: specify RngLib instances in dsc files Wang, Jian J
2019-11-14  2:17 ` [PATCH 04/11] MdePkg/BaseLib: add interface to wrap rdseed IA instruction Wang, Jian J
2019-11-14  4:17   ` [edk2-devel] " Michael D Kinney
2019-11-14  4:40     ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 05/11] SecurityPkg/RngLibRdSeed: add an instance of RngLib to make use rdseed Wang, Jian J
2019-11-14  4:24   ` [edk2-devel] " Michael D Kinney
2019-11-14  4:38     ` Wang, Jian J
2019-11-15 13:28       ` Ard Biesheuvel
2019-11-15 17:21         ` Michael D Kinney
2019-11-15 17:35           ` Ard Biesheuvel
2019-11-16  2:17             ` Wang, Jian J
2019-11-15 22:19         ` Laszlo Ersek
2019-11-14  2:17 ` [PATCH 06/11] SecurityPkg/DxeRngLibRngProtocol: add RNG protocol version of RngLib Wang, Jian J
2019-11-14 11:15   ` [edk2-devel] " Laszlo Ersek
2019-11-14 14:52     ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 07/11] SecurityPkg/SecurityPkg.dsc: add new RngLib instances for build Wang, Jian J
2019-11-14  2:17 ` [PATCH 08/11] OvmfPkg: specify RngLib instances in dsc files Wang, Jian J
2019-11-14 11:07   ` [edk2-devel] " Laszlo Ersek
2019-11-14 14:40     ` Wang, Jian J
2019-11-14 14:51       ` Laszlo Ersek
2019-11-14 14:55         ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 09/11] ArmVirtPkg/ArmVirt.dsc.inc: " Wang, Jian J
2019-11-14  7:41   ` [edk2-devel] " Ard Biesheuvel
2019-11-14  8:03     ` Wang, Jian J
2019-11-14  8:14       ` Ard Biesheuvel
2019-11-14  8:31         ` Wang, Jian J
2019-11-14 10:36   ` Laszlo Ersek
2019-11-14 14:26     ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 10/11] CryptoPkg/OpensslLib: use RngLib to get high quality random entropy Wang, Jian J
2019-11-14  7:42   ` Ard Biesheuvel [this message]
2019-11-14  2:17 ` [PATCH 11/11] FmpDevicePkg/FmpDevicePkg.dsc: remove TimerLib instance Wang, Jian J
2019-11-14  4:21 ` [edk2-devel] [PATCH 00/11] Use proper entropy sources Michael D Kinney
2019-11-14  5:15   ` Wang, Jian J

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu9ng_Gcjzpo_k58Kn=p6GMDb6SPVMtg6Qfd+phXLuk_Jw@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox