Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
On Fri, May 17, 2024 at 09:27:53AM GMT, Ard Biesheuvel wrote: > On Fri, 17 May 2024 at 05:27, Doug Flick via groups.io > wrote: > > > > On ARM, we can actually do better than this: I have taken Doug's v2 and > > applied some changes on top to make it work with ArmVirtQemu. > > > > https://github.com/ardbiesheuvel/edk2/tree/doug-v2 > > > > Ard, would you be comfortable with this patch series if I take the commits > > you're suggesting? I'm being asked to see what it would take to get these > > commits in for this release. > > I won't object to that, but I'd like Gerd's take as well, given that a > similar concern appears to apply to OVMF/x86 IIUC. I think including RngDxe in OvmfPkg is not an option. That would be a silent regression on the random number quality delivered by EFI_RNG_PROTOCOL because OvmfPkg uses BaseRngLibTimerLib. Switching to BaseRngLib is an easy way out for physical platforms with a sufficient recent processor. OVMF can not assume the rdrand instruction is available, so that is not possible. So short-term (i.e. 2024-05 stable tag) the only option I see is depending on virtio-rng. Which is a regression too (network booting without '-device virtio-rng-pci' breaks), but it is an obvious failure with an easy fix. Not an ideal solution, but much better than a regression which can easily go unnoticed. Longer term it probably makes sense to have a EFI_RNG_PROTOCOL driver using the rdrand instruction and runtime detection whenever the instruction is available or not. Either by adapting RngDxe accordingly, or by having an OVMF-specific driver handling the runtime detection. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119016): https://edk2.groups.io/g/devel/message/119016 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
On Fri, 17 May 2024 at 05:27, Doug Flick via groups.io wrote: > > On ARM, we can actually do better than this: I have taken Doug's v2 and > applied some changes on top to make it work with ArmVirtQemu. > > https://github.com/ardbiesheuvel/edk2/tree/doug-v2 > > Ard, would you be comfortable with this patch series if I take the commits > you're suggesting? I'm being asked to see what it would take to get these > commits in for this release. I won't object to that, but I'd like Gerd's take as well, given that a similar concern appears to apply to OVMF/x86 IIUC. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118991): https://edk2.groups.io/g/devel/message/118991 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
> > On ARM, we can actually do better than this: I have taken Doug's v2 > and applied some changes on top to make it work with ArmVirtQemu. > > https://github.com/ardbiesheuvel/edk2/tree/doug-v2 > Ard, would you be comfortable with this patch series if I take the commits you're suggesting? I'm being asked to see what it would take to get these commits in for this release. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118973): https://edk2.groups.io/g/devel/message/118973 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
On Mon, May 13, 2024 at 10:23 AM Gerd Hoffmann via groups.io wrote: > > On Sat, May 11, 2024 at 10:40:23AM GMT, Ard Biesheuvel wrote: > > As I pointed out before, on the ARM side there are a few intersecting > > issues with these changes. (On x86, this is mostly avoided due to the > > fact that RDRAND is universally supported) (citation needed. since 2012 on Intel's side, 2015 on AMD, but with lots of broken implementations along the way) > > Well, it's not that easy on x86 either. > > Current state of affairs is that the time based LibRng is used, all > OvmfPkg / ArmVirtPkg have this: > > RngLib|MdeModulePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf > > So, this is what will be used if something uses LibRng. Note that OVMF can't switch to BaseRngLib (and use rdrand/whatever ARM is supplying for RNG), because you'll crash a bunch of systems: https://github.com/tianocore/edk2/blob/4b6ee06a090d956f80b4a92fb9bf03098a372f39/MdePkg/Library/BaseRngLib/Rand/RdRand.c#L125-L131 I had submitted a patch that dealt with this a while back (and tried to detect broken impls such as AMD zen returning all-1s), but it got ghosted :) -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118893): https://edk2.groups.io/g/devel/message/118893 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
(cc some ARM folks) On Mon, 13 May 2024 at 11:23, Gerd Hoffmann wrote: > > On Sat, May 11, 2024 at 10:40:23AM GMT, Ard Biesheuvel wrote: > > As I pointed out before, on the ARM side there are a few intersecting > > issues with these changes. (On x86, this is mostly avoided due to the > > fact that RDRAND is universally supported) > > Well, it's not that easy on x86 either. > > Current state of affairs is that the time based LibRng is used, all > OvmfPkg / ArmVirtPkg have this: > > RngLib|MdeModulePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf > > So, this is what will be used if something uses LibRng. > > On x64 RngDxe will just use call LibRng too. So adding RngDxe will > effectively make BaseRngLibTimerLib available via EFI_RNG_PROTOCOL. > > In case '-device virtio-rng-pci' is present we now have *two* drivers > providing EFI_RNG_PROTOCOL. What will happen in this case? What we > surely not want is RngDxe being used in case we have a virtio-rng > device ... > On ARM, we can actually do better than this: I have taken Doug's v2 and applied some changes on top to make it work with ArmVirtQemu. https://github.com/ardbiesheuvel/edk2/tree/doug-v2 The ARM version of RngLib can be backed by either RNDR or TRNG, and exposes gEfiRngAlgorithmArmRndr, gEfiRngAlgorithmRaw, or both. If neither are supported, RngDxe will not be dispatched. Given that RNDR is implemented by the hardware, and TRNG by the hypervisor, and neither depend on the UEFI driver model (like virtio-rng), which is backed by the VMM, I think that in this case, there is no issue with dispatching both, even if that results in two implementations of the EFI_RNG_PROTOCOL, and no guarantees regarding which one you get when you locate the protocol. (Confidential VMs may want to avoid virtio-rng as it is provided by the host but let's disregard that for the time being) The upshot is that existing ARM deployments that do not use the 'max' CPU in TCG mode, or are on a fairly old version of KVM will lose network support unless they enable the virtio-rng-pci device. This is a situation I can live with, but it does require the changes I am proposing on the branch above. On x86, we should avoid BaseRngLibTimerLib as well - it is a bit ugly to expose two EFI_RNG_PROTOCOL instances, but we could at least ensure that it doesn't matter which one you grab. I intend to look more deeply into this in the future, and maybe compare notes with Pierre, as this has been a rather complicated delivery IIRC. Perhaps it would be better for RngDxe to consume a raw entropy source and implement the DRBG directly rather than expose the raw protocol (which I think should not have been introduced in the first place) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118872): https://edk2.groups.io/g/devel/message/118872 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
On Sat, May 11, 2024 at 10:40:23AM GMT, Ard Biesheuvel wrote: > As I pointed out before, on the ARM side there are a few intersecting > issues with these changes. (On x86, this is mostly avoided due to the > fact that RDRAND is universally supported) Well, it's not that easy on x86 either. Current state of affairs is that the time based LibRng is used, all OvmfPkg / ArmVirtPkg have this: RngLib|MdeModulePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf So, this is what will be used if something uses LibRng. On x64 RngDxe will just use call LibRng too. So adding RngDxe will effectively make BaseRngLibTimerLib available via EFI_RNG_PROTOCOL. In case '-device virtio-rng-pci' is present we now have *two* drivers providing EFI_RNG_PROTOCOL. What will happen in this case? What we surely not want is RngDxe being used in case we have a virtio-rng device ... take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118861): https://edk2.groups.io/g/devel/message/118861 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
As I pointed out before, on the ARM side there are a few intersecting issues with these changes. (On x86, this is mostly avoided due to the fact that RDRAND is universally supported) - the RNDR instructions are not widely available yet, and support has not yet been added to ArmVirtQemu IIRC - the hypervisor TRNG service is only available when executing QEMU under KVM (true virtualization), or when running a firmware stack inside QEMU that implements it - QEMU itself does not expose this to guests when QEMU is acting as the hypervisor (TCG) - the virtio-rng device needs to be activated explicitly on the command line On the one hand, this means there are various ways to get entropy on /most/ systems, but it also means that the default use case of running QEMU on a non-ARM host (implying lack of KVM) without virtio-rng-pci will not have any EFI_RNG_PROTOCOL available. We might change this with RNDR but this will still leave some use cases behind (where a specific CPU is selected rather than 'max') If the result of this series is that systems with a EFI_RNG_PROTOCOL implementation cannot boot at all, this is a problem. If it means they cannot boot from the network, I'd be less worried. And while adding -device virtio-rng-pci to the CI command lines was long overdue, doing so doesn't fix other deployments of QEMU with the bundled firmware, so something has to be done. So we can at least clarify what the consequences are of attempting to run OVMF/ArmVirtQemu on a system that does not implement EFI_RNG_PROTOCOL at all? And then, make an informed decision on how to mitigate any resulting breakage? On Fri, 10 May 2024 at 19:13, Doug Flick via groups.io wrote: > > At a high level, this isn't my project and I would look towards the > maintainers to provide guidance about the direction they want to go. > > However, > > In my opinion, this is a debate on Security vs Compatibility. I'm biased more > towards security, and I've tried to make it easy for a platform to understand > what is happening with PcdEnforceSecureRngAlgorithms . If we default to > default the platform never has the chance to understand what Rng Algorithms > they provide and if that is a problem for them. Default is obviously the most > compatible but it's also the one that may or may not be backed by something > insecure. Which is why I would prefer if a platform acknowledges that they > know it's backed by something secure or if not its an active decision. > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118831): https://edk2.groups.io/g/devel/message/118831 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci
At a high level, this isn't my project and I would look towards the maintainers to provide guidance about the direction they want to go. However, In my opinion, this is a debate on Security vs Compatibility. I'm biased more towards security, and I've tried to make it easy for a platform to understand what is happening with `PcdEnforceSecureRngAlgorithms` . If we default to `default` the platform never has the chance to understand what Rng Algorithms they provide and if that is a problem for them. Default is obviously the most compatible but it's also the one that may or may not be backed by something insecure. Which is why I would prefer if a platform acknowledges that they know it's backed by something secure or if not its an active decision. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118824): https://edk2.groups.io/g/devel/message/118824 Mute This Topic: https://groups.io/mt/106013302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
