public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wang, Jian J" <jian.j.wang@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"lersek@redhat.com" <lersek@redhat.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Justen, Jordan L" <jordan.l.justen@intel.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	"Ni, Ray" <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH 08/11] OvmfPkg: specify RngLib instances in dsc files
Date: Thu, 14 Nov 2019 14:40:28 +0000	[thread overview]
Message-ID: <D827630B58408649ACB04F44C5100036259AFF13@SHSMSX107.ccr.corp.intel.com> (raw)
In-Reply-To: <492770b6-9010-0513-618e-a525d0c63064@redhat.com>

Laszlo,


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Thursday, November 14, 2019 7:08 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Cc: devel@edk2.groups.io; Justen, Jordan L <jordan.l.justen@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH 08/11] OvmfPkg: specify RngLib instances in
> dsc files
> 
> Jian, Ard,
> 
> commenting for both OvmfPkg and ArmVirtPkg:
> 
> 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 added
> > for it. For all other drivers, BaseRngLibNull is used by default.
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  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 @@
> >      <LibraryClasses>
> >        NULL|OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
> >    }
> > +  NetworkPkg/TlsDxe/TlsDxe.inf {
> > +    <LibraryClasses>
> > +
> RngLib|SecurityPkg/RandomNumberGenerator/DxeRngLibRngProtocol/DxeRng
> LibRngProtocol.inf
> > +  }
> >  !endif
> >    OvmfPkg/VirtioNetDxe/VirtioNet.inf
> >
> 
> This is not right for either OvmfPkg or ArmVirtPkg, because:
> 
> - the virtual hardware is dynamic and might change from boot to boot,
> 
> - EFI_RNG_PROTOCOL, implemented by VirtioRngDxe, only supports
> EFI_RNG_ALGORITHM_RAW.
> 
> I propose the following approach.
> 
> (1) Jian, please do introduce all of the RngLib instances that you are
> introducing in this v1 series, namely:
> 
> - 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.
> 

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.)
> 
> 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.)
> 

Either is fine with me. I'll let you and Ard to make decision.

> (3) For OvmfPkg and ArmVirtPkg, please write patches that:
> 
> - provide a default resolution to the Null instance,
> 
> - resolve RngLib to the time-based instance, for TlsDxe *only*.
> 

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.
> 
> 
> 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.)
> 
> - 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".
> 
> - each RngLib API in this lib instance should hang, if "mEndOfDxe" is
> FALSE at the time of the call (--> CpuDeadLoop() etc)
> 
> - 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.
> 
> - otherwise, on IA32/X64 only, if CPUID indicates that RDSEED is
> supported by the processor, call RDSEED.
> 
> - otherwise, on IA32/X64 only, if CPUID indicates that RDRAND is
> supported by the processor, call RDRAND.
> 
> - 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).
> 
> - The platform BDS code for OVMF and ArmVirtQemu* should bind the
> virtio-rng device to VirtioRngDxe *before* signaling End-of-Dxe.
> 
> 
> 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).
> 

I'm curious that you want to do the "degrade" dynamically at boot time not
build time, right?

Thanks,
Jian

> Thanks
> Laszlo
> 
> 
> 


  reply	other threads:[~2019-11-14 14:40 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14  2:17 [PATCH 00/11] Use proper entropy sources Wang, Jian J
2019-11-14  2:17 ` [PATCH 01/11] NetworkPkg/NetworkPkg.dsc: specify RngLib instance for build Wang, Jian J
2019-11-14  2:17 ` [PATCH 02/11] SignedCapsulePkg/SignedCapsulePkg.dsc: specify RngLib instances Wang, Jian J
2019-11-14  2:17 ` [PATCH 03/11] FmpDevicePkg/FmpDevicePkg.dsc: specify RngLib instances in dsc files Wang, Jian J
2019-11-14  2:17 ` [PATCH 04/11] MdePkg/BaseLib: add interface to wrap rdseed IA instruction Wang, Jian J
2019-11-14  4:17   ` [edk2-devel] " Michael D Kinney
2019-11-14  4:40     ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 05/11] SecurityPkg/RngLibRdSeed: add an instance of RngLib to make use rdseed Wang, Jian J
2019-11-14  4:24   ` [edk2-devel] " Michael D Kinney
2019-11-14  4:38     ` Wang, Jian J
2019-11-15 13:28       ` Ard Biesheuvel
2019-11-15 17:21         ` Michael D Kinney
2019-11-15 17:35           ` Ard Biesheuvel
2019-11-16  2:17             ` Wang, Jian J
2019-11-15 22:19         ` Laszlo Ersek
2019-11-14  2:17 ` [PATCH 06/11] SecurityPkg/DxeRngLibRngProtocol: add RNG protocol version of RngLib Wang, Jian J
2019-11-14 11:15   ` [edk2-devel] " Laszlo Ersek
2019-11-14 14:52     ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 07/11] SecurityPkg/SecurityPkg.dsc: add new RngLib instances for build Wang, Jian J
2019-11-14  2:17 ` [PATCH 08/11] OvmfPkg: specify RngLib instances in dsc files Wang, Jian J
2019-11-14 11:07   ` [edk2-devel] " Laszlo Ersek
2019-11-14 14:40     ` Wang, Jian J [this message]
2019-11-14 14:51       ` Laszlo Ersek
2019-11-14 14:55         ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 09/11] ArmVirtPkg/ArmVirt.dsc.inc: " Wang, Jian J
2019-11-14  7:41   ` [edk2-devel] " Ard Biesheuvel
2019-11-14  8:03     ` Wang, Jian J
2019-11-14  8:14       ` Ard Biesheuvel
2019-11-14  8:31         ` Wang, Jian J
2019-11-14 10:36   ` Laszlo Ersek
2019-11-14 14:26     ` Wang, Jian J
2019-11-14  2:17 ` [PATCH 10/11] CryptoPkg/OpensslLib: use RngLib to get high quality random entropy Wang, Jian J
2019-11-14  7:42   ` Ard Biesheuvel
2019-11-14  2:17 ` [PATCH 11/11] FmpDevicePkg/FmpDevicePkg.dsc: remove TimerLib instance Wang, Jian J
2019-11-14  4:21 ` [edk2-devel] [PATCH 00/11] Use proper entropy sources Michael D Kinney
2019-11-14  5:15   ` Wang, Jian J

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D827630B58408649ACB04F44C5100036259AFF13@SHSMSX107.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox