Re: [edk2-devel] 回复: [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci

2024-05-11 Thread Ard Biesheuvel
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] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng

2024-05-11 Thread Ard Biesheuvel
On Thu, 9 May 2024 at 07:56, Doug Flick via groups.io
 wrote:
>
> Removed from gEfiRngAlgorithmRaw an incorrect assumption that
> Raw cannot return less than 256 bits. The DRNG Algorithms
> should always use a 256 bit seed as per nist standards
> however a caller is free to request less than 256 bits.
> >
> > //
> >// When a DRBG is used on the output of a entropy source,
> >// its security level must be at least 256 bits according to UEFI Spec.
> >//
> >if (RNGValueLength < 32) {
> >  return EFI_INVALID_PARAMETER;
> >}
> >
>
> AARCH64 platforms do not have this limitation and this brings both
> implementations into alignment with each other and the spec.
>
> Cc: Jiewen Yao 
>
> Signed-off-by: Doug Flick [MSFT] 

Reviewed-by: Ard Biesheuvel 

As I commented in the other thread, it is not the job of the raw
EFI_RNG_PROTOCOL to ensure that its callers never do anything silly.
Refusing requests for less than 32 bytes is pointless and arbitrary,
as only avoids one very particular potential mistake.


> ---
>  SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c | 8 
>  1 file changed, 8 deletions(-)
>
> diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c 
> b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
> index 7e06e16e4be5..5723ed695747 100644
> --- a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
> +++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
> @@ -116,14 +116,6 @@ RngGetRNG (
>// The "raw" algorithm is intended to provide entropy directly
>//
>if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmRaw)) {
> -//
> -// When a DRBG is used on the output of a entropy source,
> -// its security level must be at least 256 bits according to UEFI Spec.
> -//
> -if (RNGValueLength < 32) {
> -  return EFI_INVALID_PARAMETER;
> -}
> -
>  Status = GenerateEntropy (RNGValueLength, RNGValue);
>  return Status;
>}
> --
> 2.34.1
>
>
>
> 
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#118722): https://edk2.groups.io/g/devel/message/118722
> Mute This Topic: https://groups.io/mt/105996584/1131722
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [a...@kernel.org]
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118830): https://edk2.groups.io/g/devel/message/118830
Mute This Topic: https://groups.io/mt/105996584/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-