Re: [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng
Hello,
The patch looks good to me:
Reviewed-by: Pierre Gondois
Regards,
Pierre
On 5/11/24 02:24, Yao, Jiewen wrote:
Thanks to confirm that.
I am OK on what you have said.
Since the ARM part is added by Pierre Gondois pierre.gond...@arm.com
<mailto:pierre.gond...@arm.com>, I will let him comment if there is any concern
on the change for ARM.
Thank you
Yao, Jiewen
*From:* Doug Flick via groups.io
*Sent:* Saturday, May 11, 2024 5:12 AM
*To:* Yao, Jiewen ; devel@edk2.groups.io
*Subject:* Re: [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove
incorrect limitation on GetRng
So, I'm trying to consult with some RNG experts because I'm by no means an
expert and anything I say should be taken with huge grain of salt. When I get
the experts take, I'll share it.
Basically, the way I read this code is that it by no means tries to enforce any
entropy requirement outside of what you ask for.
My understanding is the 256 Bit Entropy requirements comes from when you are
using a DRNG algorithm such as:
|#define EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID \|
|{0xa7af67cb, 0x603b, 0x4d42,\|
|{0xba, 0x21, 0x70, 0xbf, 0xb6, 0x29, 0x3f, 0x96}}|
||
|#define EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID \|
|{0xc5149b43, 0xae85, 0x4f53,\|
|{0x99, 0x82, 0xb9, 0x43, 0x35, 0xd3, 0xa9, 0xe7}}|
||
|#define EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID \|
|{0x44f0de6e, 0x4d8c, 0x4045, \|
|{0xa8, 0xc7, 0x4d, 0xd1, 0x68, 0x85, 0x6b, 0x9e}}|
"When a Deterministic Random Bit Generator (DRBG) is used on the output of a
(raw) entropy source, its security level must be at least 256 bits."
https://uefi.org/specs/UEFI/2.10/37_Secure_Technologies.html#random-number-generator-protocol
<https://uefi.org/specs/UEFI/2.10/37_Secure_Technologies.html#random-number-generator-protocol>
That is, the seed of these algorithms must be at a minimum 256 bits from your
entropy source.
Now when you call for instance EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
On an INTEL CPU it uses the Intel RDRAND Instruction
https://github.com/tianocore/edk2/blob/4b6ee06a090d956f80b4a92fb9bf03098a372f39/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c#L108C45-L108C51
<https://github.com/tianocore/edk2/blob/4b6ee06a090d956f80b4a92fb9bf03098a372f39/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c#L108C45-L108C51>
Which from what I can tell the generator takes pairs of 256-bit raw entropy
samples generated by the hardware entropy source and applies them to an
Advanced Encryption Standard (AES) (in CBC-MAC mode) conditioner which reduces
them to a single 256-bit conditioned entropy sample.
https://en.wikipedia.org/wiki/RDRAND <https://en.wikipedia.org/wiki/RDRAND>
https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html
<https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html>
Which means, if you are implementing these algorithms in software, you must
comply with the 256 bit entropy requirement for your source. However in our
case the CPU is performing that requirement for us.
Again I'm no expert. So if an expert is reading this and I'm completely wrong
please let me know :)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118869): https://edk2.groups.io/g/devel/message/118869
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]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng
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, )) {
> -//
> -// 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]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng
Thanks to confirm that.
I am OK on what you have said.
Since the ARM part is added by Pierre Gondois
pierre.gond...@arm.com<mailto:pierre.gond...@arm.com>, I will let him comment
if there is any concern on the change for ARM.
Thank you
Yao, Jiewen
From: Doug Flick via groups.io
Sent: Saturday, May 11, 2024 5:12 AM
To: Yao, Jiewen ; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove
incorrect limitation on GetRng
So, I'm trying to consult with some RNG experts because I'm by no means an
expert and anything I say should be taken with huge grain of salt. When I get
the experts take, I'll share it.
Basically, the way I read this code is that it by no means tries to enforce any
entropy requirement outside of what you ask for.
My understanding is the 256 Bit Entropy requirements comes from when you are
using a DRNG algorithm such as:
#define EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID \
{0xa7af67cb, 0x603b, 0x4d42,\
{0xba, 0x21, 0x70, 0xbf, 0xb6, 0x29, 0x3f, 0x96}}
#define EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID \
{0xc5149b43, 0xae85, 0x4f53,\
{0x99, 0x82, 0xb9, 0x43, 0x35, 0xd3, 0xa9, 0xe7}}
#define EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID \
{0x44f0de6e, 0x4d8c, 0x4045, \
{0xa8, 0xc7, 0x4d, 0xd1, 0x68, 0x85, 0x6b, 0x9e}}
"When a Deterministic Random Bit Generator (DRBG) is used on the output of a
(raw) entropy source, its security level must be at least 256 bits."
https://uefi.org/specs/UEFI/2.10/37_Secure_Technologies.html#random-number-generator-protocol
That is, the seed of these algorithms must be at a minimum 256 bits from your
entropy source.
Now when you call for instance EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
On an INTEL CPU it uses the Intel RDRAND Instruction
https://github.com/tianocore/edk2/blob/4b6ee06a090d956f80b4a92fb9bf03098a372f39/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c#L108C45-L108C51
Which from what I can tell the generator takes pairs of 256-bit raw entropy
samples generated by the hardware entropy source and applies them to an
Advanced Encryption Standard (AES) (in CBC-MAC mode) conditioner which reduces
them to a single 256-bit conditioned entropy sample.
https://en.wikipedia.org/wiki/RDRAND
https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html
Which means, if you are implementing these algorithms in software, you must
comply with the 256 bit entropy requirement for your source. However in our
case the CPU is performing that requirement for us.
Again I'm no expert. So if an expert is reading this and I'm completely wrong
please let me know :)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118828): https://edk2.groups.io/g/devel/message/118828
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]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng
So, I'm trying to consult with some RNG experts because I'm by no means an
expert and anything I say should be taken with huge grain of salt. When I get
the experts take, I'll share it.
Basically, the way I read this code is that it by no means tries to enforce any
entropy requirement outside of what you ask for.
My understanding is the 256 Bit Entropy requirements comes from when you are
using a DRNG algorithm such as:
```
#define EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID \
{0xa7af67cb, 0x603b, 0x4d42,\
{0xba, 0x21, 0x70, 0xbf, 0xb6, 0x29, 0x3f, 0x96}}
#define EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID \
{0xc5149b43, 0xae85, 0x4f53,\
{0x99, 0x82, 0xb9, 0x43, 0x35, 0xd3, 0xa9, 0xe7}}
#define EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID \
{0x44f0de6e, 0x4d8c, 0x4045, \
{0xa8, 0xc7, 0x4d, 0xd1, 0x68, 0x85, 0x6b, 0x9e}}
```
> "When a Deterministic Random Bit Generator (DRBG) is used on the output of a
> (raw) entropy source, its security level must be at least 256 bits."
https://uefi.org/specs/UEFI/2.10/37_Secure_Technologies.html#random-number-generator-protocol
That is, the seed of these algorithms must be at a minimum 256 bits from your
entropy source.
Now when you call for instance EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
On an INTEL CPU it uses the Intel RDRAND Instruction
https://github.com/tianocore/edk2/blob/4b6ee06a090d956f80b4a92fb9bf03098a372f39/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c#L108C45-L108C51
Which from what I can tell the generator takes pairs of 256-bit raw entropy
samples generated by the hardware entropy source and applies them to an
Advanced Encryption Standard (AES) (in CBC-MAC mode) conditioner which reduces
them to a single 256-bit conditioned entropy sample.
https://en.wikipedia.org/wiki/RDRAND
https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html
Which means, if you are implementing these algorithms in software, you must
comply with the 256 bit entropy requirement for your source. However in our
case the CPU is performing that requirement for us.
Again I'm no expert. So if an expert is reading this and I'm completely wrong
please let me know :)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118827): https://edk2.groups.io/g/devel/message/118827
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]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng
Hi Doug
First, I agree with you that "A caller is free to request less than 256 bit".
Second, I think we still need to meet 256 bit entropy requirement in UEFI spec,
right?
With above assumption, I checked how the callee is implemented when input
length is small.
https://github.com/tianocore/edk2/blob/master/SecurityPkg/RandomNumberGenerator/RngDxe/ArmTrng.c#L54-L59
EntropyBits = MIN ((RequiredEntropyBits - CollectedEntropyBits), MaxBits);
Status = GetArmTrngEntropy (
EntropyBits,
(Length - Index),
[Index]
);
It seems to me that the EntropyBits is also less than 256, when the input
requirement is less than 256 bit.
Would you please double check that, to see if the requirement is still
satisfied?
Please correct me if my understanding is wrong.
Thank you
Yao, Jiewen
> -Original Message-
> From: Doug Flick
> Sent: Thursday, May 9, 2024 1:56 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen
> Subject: [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on
> GetRng
>
> 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]
> ---
> 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, )) {
>
> -//
>
> -// 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 (#118811): https://edk2.groups.io/g/devel/message/118811
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]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng
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]
---
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, )) {
-//
-// 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
