From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web12.5035.1573742433175100136 for ; Thu, 14 Nov 2019 06:40:33 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: jian.j.wang@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Nov 2019 06:40:31 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,304,1569308400"; d="scan'208";a="406337792" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga006.fm.intel.com with ESMTP; 14 Nov 2019 06:40:31 -0800 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 14 Nov 2019 06:40:31 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 14 Nov 2019 06:40:30 -0800 Received: from shsmsx107.ccr.corp.intel.com ([169.254.9.63]) by shsmsx102.ccr.corp.intel.com ([169.254.2.108]) with mapi id 14.03.0439.000; Thu, 14 Nov 2019 22:40:29 +0800 From: "Wang, Jian J" To: "devel@edk2.groups.io" , "lersek@redhat.com" , Ard Biesheuvel CC: "Justen, Jordan L" , "Gao, Liming" , "Ni, Ray" Subject: Re: [edk2-devel] [PATCH 08/11] OvmfPkg: specify RngLib instances in dsc files Thread-Topic: [edk2-devel] [PATCH 08/11] OvmfPkg: specify RngLib instances in dsc files Thread-Index: AQHVmtvBOKYP5K/O4kyUc+QpdU2PZaeKueaQ Date: Thu, 14 Nov 2019 14:40:28 +0000 Message-ID: References: <20191114021743.3876-1-jian.j.wang@intel.com> <20191114021743.3876-9-jian.j.wang@intel.com> <492770b6-9010-0513-618e-a525d0c63064@redhat.com> In-Reply-To: <492770b6-9010-0513-618e-a525d0c63064@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzNiMWI1YmItMDJmMS00YzUyLWE3YmItODUyNWM5Yjg2MjZjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiSEV0RWR2Y3VDd1NNUlNzbW1QaDF4SUJZR2tXNUpndklsOWY0dGwreFh3Z3Vpc3R4TWxLemdENlV1OGgrTEdnbiJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: jian.j.wang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Laszlo Er= sek > Sent: Thursday, November 14, 2019 7:08 PM > To: Wang, Jian J ; Ard Biesheuvel > > Cc: devel@edk2.groups.io; Justen, Jordan L ; = Gao, > Liming ; Ni, Ray > Subject: Re: [edk2-devel] [PATCH 08/11] OvmfPkg: specify RngLib instance= s in > dsc files >=20 > Jian, Ard, >=20 > commenting for both OvmfPkg and ArmVirtPkg: >=20 > On 11/14/19 03:17, Wang, Jian J wrote: > > Per BZ1871, OpensslLib will depend on RngLib instead of TimerLib. > > Update OVMF dsc files to accommodate the coming changes. It's supposed > > that only TlsDxe needs random number. The DxeRngLibRngProtocol is adde= d > > for it. For all other drivers, BaseRngLibNull is used by default. > > > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1871 > > Cc: Jordan Justen > > Cc: Laszlo Ersek > > Cc: Ard Biesheuvel > > Cc: Liming Gao > > Cc: Ray Ni > > Signed-off-by: Jian J Wang > > --- > > OvmfPkg/OvmfPkgIa32.dsc | 5 +++++ > > OvmfPkg/OvmfPkgIa32X64.dsc | 5 +++++ > > OvmfPkg/OvmfPkgX64.dsc | 5 +++++ > > OvmfPkg/OvmfXen.dsc | 5 +++++ > > 4 files changed, 20 insertions(+) > > > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > > index d350b75630..5a709a95b2 100644 > > --- a/OvmfPkg/OvmfPkgIa32.dsc > > +++ b/OvmfPkg/OvmfPkgIa32.dsc > > @@ -217,6 +217,7 @@ > > > > [LibraryClasses.common] > > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > > + RngLib|MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf > > > > [LibraryClasses.common.SEC] > > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf > > @@ -786,6 +787,10 @@ > > > > NULL|OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf > > } > > + NetworkPkg/TlsDxe/TlsDxe.inf { > > + > > + > RngLib|SecurityPkg/RandomNumberGenerator/DxeRngLibRngProtocol/DxeRng > LibRngProtocol.inf > > + } > > !endif > > OvmfPkg/VirtioNetDxe/VirtioNet.inf > > >=20 > This is not right for either OvmfPkg or ArmVirtPkg, because: >=20 > - the virtual hardware is dynamic and might change from boot to boot, >=20 > - EFI_RNG_PROTOCOL, implemented by VirtioRngDxe, only supports > EFI_RNG_ALGORITHM_RAW. >=20 > I propose the following approach. >=20 > (1) Jian, please do introduce all of the RngLib instances that you are > introducing in this v1 series, namely: >=20 > - the null lib instance, > - the RDSEED lib instance, > - the DXE / protocol lib instance that directly requires NIST/ANSI > conformant algorithm support from the RNG protocol. >=20 I agree. > (2) Jian, please add a *further* lib instance: a time based one. This > instance should basically extract the current functionality from > "rand_pool_noise.c" and "rand_pool_noise_tsc.c". (The code that you are > removing in patch #10.) >=20 > It's also fine with me if, rather than factoring out the > rand_pool_noise*.c logic, you add a new "CPU jitter" based RngLib > instance. (Ard might disagree with me about this alternative -- the > point is that we should have a library instance that provides *some* > (even if barely tolerable) randomness, but with no dependency on > *optional* hardware. In other words, this lib instance should depend on > *guaranteed* platform hardware only. TSC, TimerLib, etc.) >=20 Either is fine with me. I'll let you and Ard to make decision. > (3) For OvmfPkg and ArmVirtPkg, please write patches that: >=20 > - provide a default resolution to the Null instance, >=20 > - resolve RngLib to the time-based instance, for TlsDxe *only*. >=20 Ok. I'll update those dsc in v2. > At that point, we will have made ArmVirtPkg and OvmfPkg *independent* of > the rest of this series -- ArmVirtPkg and OvmfPkg will not block this > patch series, they will also not suffer any regressions, and we can go > ahead and implement a separate RngLib instance for TlsDxe ourselves. >=20 >=20 > Now, let me lay out my proposal for *that* RngLib instance. (I'm willing > to write it a good chunk of it, but I will need help from Ard minimally > in the crypto code.) >=20 > - The CONSTRUCTOR function should register an End-of-Dxe notification > function. In that function, (a) a boolean flag (such as "mEndOfDxe") > should be flipped from FALSE to TRUE, and (b) a pointer to > EFI_RNG_PROTOCOL should be saved (if such a protocol exists), such as > "mRngProtocol". >=20 > - each RngLib API in this lib instance should hang, if "mEndOfDxe" is > FALSE at the time of the call (--> CpuDeadLoop() etc) >=20 > - otherwise, if "mRngProtocol" is not NULL, it should be used to fetch > *raw* entropy. That raw entropy should be mixed sufficiently for NIST / > ANSI conformance. (I hope this statement makes sense.) This is where I'd > absolutely need help from Ard. >=20 > - otherwise, on IA32/X64 only, if CPUID indicates that RDSEED is > supported by the processor, call RDSEED. >=20 > - otherwise, on IA32/X64 only, if CPUID indicates that RDRAND is > supported by the processor, call RDRAND. >=20 > - otherwise, on all arches, scrape some timer-based entropy, "from the > bottom of the barrel" (possibly in an arch-specific way, e.g. we could > use TSC on IA32/X64). >=20 > - The platform BDS code for OVMF and ArmVirtQemu* should bind the > virtio-rng device to VirtioRngDxe *before* signaling End-of-Dxe. >=20 >=20 > The basic idea is that this RngLib instance should give the platform a > *chance* to produce an EFI_RNG_PROTOCOL instance until End-of-Dxe. If > that happens, then continue consuming raw entropy from *that* source, > and mix it manually for NIST / ANSI conformance. Otherwise, gracefully > degrade to the next strongest entropy source that's dynamically detected > on the platform (RDSEED or RDRAND, per CPUID -- on IA32/X64 only --, and > then timer-based -- on all platforms). >=20 I'm curious that you want to do the "degrade" dynamically at boot time not build time, right? Thanks, Jian > Thanks > Laszlo >=20 >=20 >=20