From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.8899.1668768652947819640 for ; Fri, 18 Nov 2022 02:50:53 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D73CE23A; Fri, 18 Nov 2022 02:50:58 -0800 (PST) Received: from [10.34.100.128] (pierre123.nice.arm.com [10.34.100.128]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B56463F587; Fri, 18 Nov 2022 02:50:50 -0800 (PST) Message-ID: <9408fbe2-3ea4-ef00-20b0-4615bda60f89@arm.com> Date: Fri, 18 Nov 2022 11:50:44 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/RngDxe: Fix Rng algo selection for Arm To: Ard Biesheuvel , devel@edk2.groups.io Cc: Sami Mujawar , Ard Biesheuvel , Liming Gao , Jiewen Yao , Jian J Wang , nd References: <20221116150149.2200368-1-Pierre.Gondois@arm.com> From: "PierreGondois" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable On 11/18/22 11:35, Ard Biesheuvel wrote: > On Fri, 18 Nov 2022 at 11:32, PierreGondois wr= ote: >> >> >> >> On 11/18/22 11:14, Ard Biesheuvel wrote: >>> On Fri, 18 Nov 2022 at 11:10, Sami Mujawar wro= te: >>>> >>>> Hi Ard, >>>> >>>> Please find my response inline marked [SAMI]. >>>> >>>> Regards, >>>> >>>> Sami Mujawar >>>> >>>> =EF=BB=BFOn 18/11/2022, 09:56, "Ard Biesheuvel" wr= ote: >>>> >>>> On Wed, 16 Nov 2022 at 16:02, PierreGondois wrote: >>>> > >>>> > From: Pierre Gondois >>>> > >>>> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4151 >>>> > >>>> > The EFI_RNG_PROTOCOL can advertise multiple algorithms throu= gh >>>> > Guids. The PcdCpuRngSupportedAlgorithm contains a Guid that >>>> > can be configured. It represents the algorithm used in RngLi= b. >>>> > PcdCpuRngSupportedAlgorithm is set to the Zero Guid for KvmT= ool. >>>> > >>>> > When running KvmTool on a platform platform only having the = RngLib, >>>> > the only Guid available for EFI_RNG_PROTOCOL will be the zer= o Guid. >>>> > >>>> > To select the default algorithm in EFI_RNG_PROTOCOL.GetRng()= : >>>> > a. Zero Guids are skipped >>>> > b. If no algorithm is found, an ASSERT is triggered >>>> > >>>> > To allow using the RngLib to be used for the case above, Zer= o Guids >>>> > should not be skipped (a.). >>>> > If no algorithm is found, don't prevent from booting on DEBU= G builds >>>> > (b.). >>>> > >>>> > Allow Zero Guids to be selected and don't ASSERT if no algor= ithm is >>>> > found. Also simplify the selection of the Rng algorithm when= the >>>> > default one is selected by just picking up the first element= of >>>> > mAvailableAlgoArray. >>>> > >>>> > Reported-by: Sami Mujawar >>>> > Signed-off-by: Pierre Gondois >>>> >>>> I am still confused by this. >>>> >>>> Does this mean we might register the RNG protocol if we don't = have >>>> anything to back it up? >>>> [SAMI] From a Guest firmware implementation perspective, we do not k= now the available RNG source. >>>> It may be CPU RNG, Arm FW TRNG or VIRTIO RNG. >>>> I would assume either one of CPU RNG or Arm FW TRNG would be impleme= nted on the host platform. If none of these are present, we would want to= fall back to VIRTIO RNG. >>>> >>>> Considering this, I think we should not register the EFI_RNG_PRTOCOL= if no supported algorithms are present. >>>> >>>> The other argument would be that the protocol allows discovery of su= pported RNG source. But looking how this is consumed in Linux, I think it= is better to not register EFI_RNG_PRTOCOL if no supported algorithms are= present. >>>> >>>> Please do let me know your thoughts. >>> >>> Agreed. I am adding support for the TRNG to the QEMU firmware, and if >>> this is supported, there is really no point in attaching a driver to >>> virtio-rng as well. >>> >>> This means that checking for the existence of the EFI_RNG_PROTOCOL >>> should be sufficient to decide whether or not install another one. >> >> Hello, >> There would maybe be a case where the consumer explicitly requests the >> gEfiRngAlgorithmRaw GUID and the RngDxe doesn't detect it. So there wo= uld be >> two EFI_RNG_PRTOCOL registered on different handles with different >> algorithms available. I am not sure of what should happen then. >> >=20 > Fair point. So in the QEMU case, I should test > - whether EFI_RNG_PROTOCOL exists > - whether it implements the raw flavor >=20 > and only in that case, avoid virtio-rng (which only supports raw as wel= l) >=20 >> But about the inital point, EFI_RNG_PRTOCOL should indeed not be regis= tered >> if no algorithm is available. >> >=20 > Indeed. >=20 > And btw, I noticed that the TrngLib has a whole bunch of ASSERT()s > that trigger when trying to use it on QEMU - can we rip those out as > well please? Yes ok, I will do that.