Re: [PATCH v6 0/5] Use RngLib instead of TimerLib for OpensslLib

["Matthew Carlson" <matthewfcarlson@gmail.com>]

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

I'll file a new bugzilla.

https://bugzilla.tianocore.org/show_bug.cgi?id=2897
-Matthew Carlson


On Thu, Aug 13, 2020 at 8:15 AM Yao, Jiewen <jiewen.yao@intel.com> wrote:

> Thanks Matthew.
>
> I am OK, if you want to address the RDSEED in follow-up patch series.
>
> Would you please file a new Bugzilla to record this, so we won't lose the
> information ?
>
>
>
> > -----Original Message-----
> > From: matthewfcarlson@gmail.com <matthewfcarlson@gmail.com>
> > Sent: Thursday, August 13, 2020 6:44 AM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>; Anthony Perard
> > <anthony.perard@citrix.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang,
> > Jian J <jian.j.wang@intel.com>; Julien Grall <julien@xen.org>; Justen,
> Jordan L
> > <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>; Gao,
> Liming
> > <liming.gao@intel.com>; Leif Lindholm <leif@nuviainc.com>; Kinney,
> Michael D
> > <michael.d.kinney@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Liu,
> > Zhiguang <zhiguang.liu@intel.com>; Sean Brogan
> > <sean.brogan@microsoft.com>; Matthew Carlson
> > <matthewfcarlson@gmail.com>
> > Subject: [PATCH v6 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.
> >
> > 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
> >
> > 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                 | 203
> ++------------------
> >  CryptoPkg/Library/OpensslLib/rand_pool_noise.c           |  29 ---
> >  CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c       |  43 -----
> >  MdePkg/Library/BaseRngLibDxe/RngDxeLib.c                 | 200
> > +++++++++++++++++++
> >  MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c          | 187
> > ++++++++++++++++++
> >  ArmVirtPkg/ArmVirt.dsc.inc                               |   1 +
> >  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 |  40 ++++
> >  MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  17 ++
> >  MdePkg/MdePkg.dsc                                        |   5 +-
> >  OvmfPkg/Bhyve/BhyvePkgX64.dsc                            |   1 +
> >  OvmfPkg/OvmfPkgIa32.dsc                                  |   1 +
> >  OvmfPkg/OvmfPkgIa32X64.dsc                               |   1 +
> >  OvmfPkg/OvmfPkgX64.dsc                                   |   1 +
> >  OvmfPkg/OvmfXen.dsc                                      |   1 +
> >  19 files changed, 514 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
>
>

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