回复: [edk2-devel] [PATCH v10 0/5] Use RngLib instead of TimerLib for OpensslLib

["gaoliming" <gaoliming@byosoft.com.cn>]

Mattthew:
  Thanks for your update. The changes in MdePkg are good to me. 

 Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+64916+4905953+8761045@groups.io
> <bounce+27952+64916+4905953+8761045@groups.io> 代表 Matthew
> Carlson
> 发送时间: 2020年9月2日 1:37
> 收件人: devel@edk2.groups.io
> 抄送: Ard Biesheuvel <ard.biesheuvel@arm.com>; Anthony Perard
> <anthony.perard@citrix.com>; Jiewen Yao <jiewen.yao@intel.com>; Jian J
> Wang <jian.j.wang@intel.com>; Julien Grall <julien@xen.org>; Jordan Justen
> <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>; Liming Gao
> <liming.gao@intel.com>; Leif Lindholm <leif@nuviainc.com>; Michael D
> Kinney <michael.d.kinney@intel.com>; Xiaoyu Lu <xiaoyux.lu@intel.com>;
> Zhiguang Liu <zhiguang.liu@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Matthew Carlson
> <matthewfcarlson@gmail.com>
> 主题: [edk2-devel] [PATCH v10 0/5] Use RngLib instead of TimerLib for
> OpensslLib
> 
> From: Matthew Carlson <macarl@microsoft.com>
> 
> Hello all,
> 
> This patch contains a fix for Bugzilla 1871.
> There's been a good bit of community discussion around the topic,
> so below follows a general overview of the discussion and what this patch
> does.
> 
> This is the seventh iteration of this patch series, focused on code style
and a
> few functions being renamed to comply with style.
> 
> Back in Devel message#40590
> (https://edk2.groups.io/g/devel/message/40590)
> around the patch series that updates OpenSSL to 1.1.1b, a comment was
> made
> that suggested that platforms be in charge of the entropy/randomness that
> is provided to OpenSSL as currently the entropry source seems to be a
> hand-rolled random number generator that uses the PerformanceCounter
> from
> TimerLib. This causes OpenSSL to depend on TimerLib, which is often
platform
> specific. In addition to being a potentially weaker source of randomness,
> this also poses a challenge to compile BaseCryptLibOnProtocol with a
> platform-
> agnostic version of TimerLib that works universally.
> 
> The solution here is to allow platform to specify their source of entropy
in
> addition to providing two new RngLibs: one that uses the TimerLib as well
as
> one that uses RngProtocol to provide randomness. Then the decision to use
> RDRAND or other entropy sources is up to the platform. Mixing various
> entropy
> sources is the onus of the platform. It has been suggested on Devel#40590
> and
> BZ#1871 that there should be mixing of the PerformanceCounter and
> RDRAND using
> something similar to the yarrow alogirthm that FreeBSD uses for example.
> This
> patch series doesn't offer an RngLib that offers that sort of mixing as
the
> ultimate source of random is defined by the platform.
> 
> This patch series offers three benefits:
> 1. Dependency reduction: Removes the need for a platform specific timer
> library.  We publish a single binary used on numerous platforms for
> crypto and the introduced timer lib dependency caused issues because we
> could not fulfill our platform needs with one library instance.
> 
> 2. Code maintenance: Removing this additional code and leveraging an
> existing
> library within Edk2 means less code to maintain.
> 
> 3. Platform defined quality: A platform can choose which instance to use
and
> the implications of that instance.
> 
> This patch series seeks to address five seperate issues.
>   1) Use RngLib interface to generate random entropy in rand_pool
>   2) Remove dependency on TimerLib in OpensslLib
>   3) Add a new version of RngLib implemented by TimerLib
>   4) Add a new version of RngLib implemented by EFI_RNG_PROTOCOL
>   5) Add RngLib to platforms in EDK2 such as ArmVirtPkg and OvmfPkg
> 
> Since this changes the dependencies of OpenSSL, this has the potential of
> being
> a breaking change for platforms in edk2-platforms. The easiest solution is
just
> to use the RngLib that uses the TimerLib as this closely mimics the
behavior of
> OpenSSL prior to this patch series. There is also a null version of RngLib
for
> CI environments that need this change
> (https://edk2.groups.io/g/devel/message/50432). Though it should be
> pointed out
> that in CI environments, the null version of BaseCryptLib or OpenSSL
should
> be
> used.
> 
> In addition, it has been suggested that
> 1) Add AsmRdSeed to BaseLib.
> 2) Update BaseRngLib to use AsmRdSeed() for the random number,
> if RdSeed is supported (CPUID BIT18)
> 
> However, this is largely out of scope for this particular patch series and
> will likely need to be in a follow-up series later.
> 
> It is my understanding that the OpenSSL code uses the values provided as a
> randomness pool rather than a seed or random numbers itself, so the
> requirements for randomness are not quite as stringent as other
applications.
> 
> For the ArmVirtPkg and OvmfPkg platforms, the patch series here just adds
in
> the TimerLib based RngLib as that is similar to the functionality of
before.
> It is added as a common library so any custom RngLib defined in the DSC
> should take precedence over the TimerLibRngLib.
> 
> Ref: https://github.com/tianocore/edk2/pull/845
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> 
> Patch Series History:
> v10 - addressed comments from Liming removing magic numbers and adding
> DebugLib to TimerRngLib
> v8 - addressed comments from Ard and Mike around code style for DxeRngLib
> and BaseRngLibTimerLib
> v7 - addressed comments from Lazlo and Ard for further fixes around
OvmfPkg
> v6 - addressed comments from Lazlo and Ard for fixes around OvmfPkg
> v5 - moved additions for OvmfPkg and ArmVirtPkg to correct positions
> v4 - added more information to various commit messages
> v3 - addressed comments from Mike K around fixes to BaseRngLibTimer
> delays
> v2 - renamed some libraries to fit with naming conventions
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> 
> Signed-off-by: Matthew Carlson <matthewfcarlson@gmail.com>
> 
> Matthew Carlson (5):
>   MdePkg: TimerRngLib: Added RngLib that uses TimerLib
>   MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
>   OvmfPkg: Add RngLib based on TimerLib for Crypto
>   ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg
>   CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool
> 
>  CryptoPkg/Library/OpensslLib/rand_pool.c                 | 269
> +++++---------------
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.c           |  29 ---
>  CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c       |  43 ----
>  MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c          | 189
> ++++++++++++++
>  MdePkg/Library/DxeRngLib/DxeRngLib.c                     | 199
> +++++++++++++++
>  ArmVirtPkg/ArmVirt.dsc.inc                               |   1 +
>  CryptoPkg/CryptoPkg.ci.yaml                              |   4 +-
>  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/BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  36 +++
>  MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  15 ++
>  MdePkg/Library/DxeRngLib/DxeRngLib.inf                   |  38 +++
>  MdePkg/Library/DxeRngLib/DxeRngLib.uni                   |  15 ++
>  MdePkg/MdePkg.dsc                                        |
> 5 +-
>  OvmfPkg/Bhyve/BhyveX64.dsc                               |   1
> +
>  OvmfPkg/OvmfPkgIa32.dsc                                  |   1
> +
>  OvmfPkg/OvmfPkgIa32X64.dsc                               |   1
> +
>  OvmfPkg/OvmfPkgX64.dsc                                   |   1
> +
>  OvmfPkg/OvmfXen.dsc                                      |
> 1 +
>  21 files changed, 570 insertions(+), 338 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/BaseRngLibTimerLib/RngLibTimer.c
>  create mode 100644 MdePkg/Library/DxeRngLib/DxeRngLib.c
>  delete mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.h
>  create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
>  create mode 100644
> MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni
>  create mode 100644 MdePkg/Library/DxeRngLib/DxeRngLib.inf
>  create mode 100644 MdePkg/Library/DxeRngLib/DxeRngLib.uni
> 
> --
> 2.28.0.windows.1
> 
> 
>