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 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 > > Sent: Thursday, August 13, 2020 6:44 AM > > To: devel@edk2.groups.io > > Cc: Ard Biesheuvel ; Anthony Perard > > ; Yao, Jiewen ; Wang, > > Jian J ; Julien Grall ; Justen, > Jordan L > > ; Laszlo Ersek ; Gao, > Liming > > ; Leif Lindholm ; Kinney, > Michael D > > ; Lu, XiaoyuX ; Liu, > > Zhiguang ; Sean Brogan > > ; Matthew Carlson > > > > Subject: [PATCH v6 0/5] Use RngLib instead of TimerLib for OpensslLib > > > > From: Matthew Carlson > > > > 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 > > Cc: Anthony Perard > > Cc: Jiewen Yao > > Cc: Jian J Wang > > Cc: Julien Grall > > Cc: Jordan Justen > > Cc: Laszlo Ersek > > Cc: Liming Gao > > Cc: Leif Lindholm > > Cc: Michael D Kinney > > Cc: Xiaoyu Lu > > Cc: Zhiguang Liu > > Cc: Sean Brogan > > > > Signed-off-by: Matthew Carlson > > > > > > 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 > >