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: matthewfcarl...@gmail.com <matthewfcarl...@gmail.com> > Sent: Thursday, August 13, 2020 6:44 AM > To: devel@edk2.groups.io > Cc: Ard Biesheuvel <ard.biesheu...@arm.com>; Anthony Perard > <anthony.per...@citrix.com>; Yao, Jiewen <jiewen....@intel.com>; Wang, > Jian J <jian.j.w...@intel.com>; Julien Grall <jul...@xen.org>; Justen, Jordan > L > <jordan.l.jus...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Gao, Liming > <liming....@intel.com>; Leif Lindholm <l...@nuviainc.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Lu, XiaoyuX <xiaoyux...@intel.com>; Liu, > Zhiguang <zhiguang....@intel.com>; Sean Brogan > <sean.bro...@microsoft.com>; Matthew Carlson > <matthewfcarl...@gmail.com> > Subject: [PATCH v6 0/5] Use RngLib instead of TimerLib for OpensslLib > > From: Matthew Carlson <mac...@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.biesheu...@arm.com> > Cc: Anthony Perard <anthony.per...@citrix.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Julien Grall <jul...@xen.org> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Liming Gao <liming....@intel.com> > Cc: Leif Lindholm <l...@nuviainc.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Xiaoyu Lu <xiaoyux...@intel.com> > Cc: Zhiguang Liu <zhiguang....@intel.com> > Cc: Sean Brogan <sean.bro...@microsoft.com> > > Signed-off-by: Matthew Carlson <matthewfcarl...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#64227): https://edk2.groups.io/g/devel/message/64227 Mute This Topic: https://groups.io/mt/76157312/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
