From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.8625.1668767765620391318 for ; Fri, 18 Nov 2022 02:36:05 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=gYWIbtGu; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 8F46362425 for ; Fri, 18 Nov 2022 10:36:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F2692C433C1 for ; Fri, 18 Nov 2022 10:36:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1668767764; bh=+jCOmOPjY5UqvuQhErpEgCmfqOuTLQr9MKsFIg6blzA=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=gYWIbtGu+3kRkD/0yd6xT8eGw/6Rhnj1P2uzQjesFTojmmiyQNsoqukHj5g5uoxm1 i3fuiadr67FqLm7DPdpZJyLI686BLR1+yCUXCPgT8CCWiPUlomxnOOytNb1oi80iiU mhhPFb4H+/hiSRRmGLopBzqo06B30KCMW5EAS7wWOKItEeQmh5x8CVKkAq0QTyGz9x TCd2cMv8kleQ8NsNfmkF8V2vv3wv11js712mdjcwt3a+oAfytPAkXLPFR0PhgvvWD0 PXII/4nkntQ4vRnPjnemM+E6VdTQw3+EoyQd29bIm5mVj692tkT/UKPTNWP1VpfGyV bv4VXBb16Y98Q== Received: by mail-lj1-f179.google.com with SMTP id k19so6277809lji.2 for ; Fri, 18 Nov 2022 02:36:03 -0800 (PST) X-Gm-Message-State: ANoB5pmNFQgcZlfIoaFSYU5WpiPIb/mix8XvHiaoyyeWHU+yMvRtjHas dZcbIbTJ69YhPvOmJDpUp0Q2HoJBmQstkiXthmA= X-Google-Smtp-Source: AA0mqf59JFM7z/2lkfZU8Dj9aShX73asA/GZLnxnz8D2AgrvguaaLctSlQ1lYIn88Bs7SZU7+FbzdNr6y5yYOJLETVU= X-Received: by 2002:a05:651c:1601:b0:277:3a1:e86d with SMTP id f1-20020a05651c160100b0027703a1e86dmr2529520ljq.152.1668767761948; Fri, 18 Nov 2022 02:36:01 -0800 (PST) MIME-Version: 1.0 References: <20221116150149.2200368-1-Pierre.Gondois@arm.com> In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 18 Nov 2022 11:35:50 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/RngDxe: Fix Rng algo selection for Arm To: devel@edk2.groups.io, pierre.gondois@arm.com Cc: Sami Mujawar , Ard Biesheuvel , Liming Gao , Jiewen Yao , Jian J Wang , nd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 18 Nov 2022 at 11:32, PierreGondois wrote: > > > > On 11/18/22 11:14, Ard Biesheuvel wrote: > > On Fri, 18 Nov 2022 at 11:10, Sami Mujawar wrote= : > >> > >> Hi Ard, > >> > >> Please find my response inline marked [SAMI]. > >> > >> Regards, > >> > >> Sami Mujawar > >> > >> =EF=BB=BFOn 18/11/2022, 09:56, "Ard Biesheuvel" wrot= e: > >> > >> 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 through > >> > Guids. The PcdCpuRngSupportedAlgorithm contains a Guid that > >> > can be configured. It represents the algorithm used in RngLib. > >> > PcdCpuRngSupportedAlgorithm is set to the Zero Guid for KvmTool= . > >> > > >> > When running KvmTool on a platform platform only having the Rng= Lib, > >> > the only Guid available for EFI_RNG_PROTOCOL will be the zero G= uid. > >> > > >> > 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, Zero G= uids > >> > should not be skipped (a.). > >> > If no algorithm is found, don't prevent from booting on DEBUG b= uilds > >> > (b.). > >> > > >> > Allow Zero Guids to be selected and don't ASSERT if no algorith= m is > >> > found. Also simplify the selection of the Rng algorithm when th= e > >> > 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 hav= e > >> anything to back it up? > >> [SAMI] From a Guest firmware implementation perspective, we do not kno= w 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 implement= ed on the host platform. If none of these are present, we would want to fal= l back to VIRTIO RNG. > >> > >> Considering this, I think we should not register the EFI_RNG_PRTOCOL i= f no supported algorithms are present. > >> > >> The other argument would be that the protocol allows discovery of supp= orted 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 prese= nt. > >> > >> 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 would= be > two EFI_RNG_PRTOCOL registered on different handles with different > algorithms available. I am not sure of what should happen then. > Fair point. So in the QEMU case, I should test - whether EFI_RNG_PROTOCOL exists - whether it implements the raw flavor and only in that case, avoid virtio-rng (which only supports raw as well) > But about the inital point, EFI_RNG_PRTOCOL should indeed not be register= ed > if no algorithm is available. > Indeed. 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?