From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web12.2690.1573729681026195324 for ; Thu, 14 Nov 2019 03:08:01 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=O+VZ0oi2; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1573729680; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=idR2XD9f3u5e0P14lZAStV/MKAjEuHECgi2PaqETw9M=; b=O+VZ0oi2PwM2dDb0y1rfE68Rtt2gJJ/9kYRZxntcKmx0CXlQBtFVgMBsbMRgLF8ihQSbI1 CTkXihUZHbuC+h0fmb793CSuIYYLomt+8eQcPvb/UYnm8q6aV2JEfcd5lb7NE+RWnUILRP pr5gNWHn7Gn5pSfi4ERB5fy+Yx/MIZ4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-168-9QDP8EztMdG2bTX_UmmIwg-1; Thu, 14 Nov 2019 06:07:56 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 166D2800593; Thu, 14 Nov 2019 11:07:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-80.ams2.redhat.com [10.36.117.80]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6AB1510375E8; Thu, 14 Nov 2019 11:07:53 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 08/11] OvmfPkg: specify RngLib instances in dsc files To: jian.j.wang@intel.com, Ard Biesheuvel Cc: devel@edk2.groups.io, Jordan Justen , Liming Gao , Ray Ni References: <20191114021743.3876-1-jian.j.wang@intel.com> <20191114021743.3876-9-jian.j.wang@intel.com> From: "Laszlo Ersek" Message-ID: <492770b6-9010-0513-618e-a525d0c63064@redhat.com> Date: Thu, 14 Nov 2019 12:07:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20191114021743.3876-9-jian.j.wang@intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-MC-Unique: 9QDP8EztMdG2bTX_UmmIwg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable 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. >=20 > 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(+) >=20 > 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 @@ > =20 > [LibraryClasses.common] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > + RngLib|MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf > =20 > [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/DxeR= ngLibRngProtocol.inf > + } > !endif > OvmfPkg/VirtioNetDxe/VirtioNet.inf > =20 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. (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.) (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*. 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). Thanks Laszlo