Re: [edk2-devel] [PATCH 0/3] OVMF: support EFI_RNG_PROTOCOL without virtio-rng

["Ard Biesheuvel" <ardb@kernel.org>]

On Wed, 11 Jan 2023 at 16:23, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 1/10/23 19:19, Jason A. Donenfeld via groups.io wrote:
> > Could we get this merged?
>
> Sorry to barge in -- I have *zero* complaints regarding this particular
> series, so whatever I'm about to say regards *further* BDS
> customizations. Please feel free to go ahead with merging this one, as
> far as I'm concerned.
>

Thanks.

> So, picking up the thread at
> <https://listman.redhat.com/archives/edk2-devel-archive/2022-November/055607.html>.
> The argument in that thread was made that "RDRAND-based protocol is
> better than nothing". However, the most recent idea, favoring the
> RDRAND-based protocol implementation over the virtio-rng-based one,
> seems to enable a degradation too, of EFI-time randomness.
>
> Most commonly, virtio-rng is fed on the host side from /dev/urandom,
> which *I think* means that the EFI_RNG_PROTOCOL from VirtioRngDxe will
> expose all the "good quality entropy", pre-boot, that the host-side
> Linux kernel collects from *multiple* sources. If the consumer of
> EFI_RNG_PROTOCOL in the guest doesn't do its own mixing, it sill gets
> the good stuff. That could potentially be degraded by relying on RDRAND
> only, in the guest.
>

Indeed.

> I can't propose any particular priority ordering mechanism for the
> platform firmware to produce exactly one EFI_RNG_PROTOCOL.
>
> Normally I'd suggest any viable mechanism for the platform to block or
> to delay "SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf" --
> introducing a new dynamic PCD for early exit, adding a new protocol
> dependency to its DEPEX, postponing its protocol installation to an
> event group notification function or a protocol installation
> notification. Note that RngDxe.inf is a DXE_DRIVER, so it produces its
> protocol in its entry point function, so for blocking it or
> short-circuiting it, one of these measures would be needed. It could
> even be turned into a UEFI_DRIVER, one that would bind a synthetic VenHw
> device path.
>
> But, I'm not proposing any of those right now, because I imagine there
> are advantages to having EFI_RNG_PROTOCOL in the DXE phase, that is,
> *before* the BDS phase.
>
> VirtioRngDxe is a UEFI_DRIVER module that follows the UEFI driver model;
> in other words, it won't do anything beyond exposing the
> EFI_DRIVER_BINDING_PROTOCOL until BDS connects it. I think that should
> be sufficient for most cases, even (for example) possibly providing
> randomness for TLS in UEFI HTTPS Boot. But I vaguely remember we had
> wished for randomness being available earlier than BDS.
> "SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf" can fill that
> role, VirtioRngDxe can't.
>
> So best would be if both could coexist, and VirtioRngDxe took effect
> *whenever* it were available. Of course the UEFI spec allows for a
> client to collect all instances of EFI_RNG_PROTOCOL, and then to call
> GetInfo() on each, but that's hardly enough for a client to pick the one
> it thinks is "more secure". So one way or another we might want to
> control this still at the platform level, where we can form ideas about
> both protocol providers, *and* perhaps even determine if we *actually
> need* pre-BDS randomness.
>
> BDS could try connecting the virtio-rng device. If that failed, it could
> try "unblocking" RngDxe. If RngDxe were a UEFI driver following the UEFI
> driver model (see the VenHw option above), this would not be hard to do,
> with a "fallback" gBS->ConnectController() call.
>
> (Regarding VenHw vs. VenMedia vs. VenMsg -- RngDxe uses an RNG that's
> built into the processor, wich is arguably "inside the resource domain"
> of the system. So VenHw seems the right choice.)
>
> RngDxe could perhaps be restructured for the addition of a new entry
> point (new INF file and new entry point C file), so that it remain
> compatible with existent platforms that already consume it (and want it
> to remain a DXE_DRIVER).
>
> BDS could also signal an event group or install a synthetic protocol, so
> that the notification function in RngDxe expose EFI_RNG_PROTOCOL in
> response.
>
> Unblocking a DXE_DRIVER's DEPEX from BDS seems more cumbersome, by
> installing a dependend-upon synthetic protocol; I believe we might have
> to call gDS->Dispatch() manually then.
>
> And if a dynamic PCD caused RngDxe to exit early, we couldn't undo that
> from BDS at all.
>

One option that might be feasible would be to modify VIrtioRngDxe so it:
- installs a RNG protocol implementation solely based on [Base]RngLib
when it is dispatched
- uninstalls it again when it binds to the first virtio-rng device
- reinstalls it when it unbinds from the last virtio-rng device it was bound to
- installs the virtio-rng backed flavor of the RNG protocol when
binding to a device
(- mixes the output of the latter with the RngLIb based implementation)

I think this would address all of these concerns, assuming that the
mixing is done correctly.

*However*, I am not convinced that any of this is worth the hassle,
tbh. If you don't trust your CPU, all bets are off anyway - the only
thing we'd need to cater for is an explicit opt-out for known broken
implementations of RdRand.