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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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