From: Pierre Gondois <pierre.gondois@arm.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151
The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
To allow the RngDxe to detect when such implementation is used,
a GetRngGuid() function was added in a previous patch.
The EFI_RNG_PROTOCOL can advertise multiple algorithms through
Guids. The PcdCpuRngSupportedAlgorithm is currently used to
advertise the RngLib in the Arm implementation.
The issues of doing that are:
- the RngLib implementation might not use CPU instructions,
cf. the BaseRngLibTimerLib
- most platforms don't set PcdCpuRngSupportedAlgorithm
A GetRngGuid() was added to the RngLib in a previous patch,
allowing to identify the algorithm implemented by the RngLib.
Make use of this function and place the unsage algorithm
at the last position in the mAvailableAlgoArray.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
.../RngDxe/AArch64/AArch64Algo.c | 55 +++++++++++++------
.../RandomNumberGenerator/RngDxe/ArmRngDxe.c | 6 +-
.../RandomNumberGenerator/RngDxe/RngDxe.inf | 5 +-
3 files changed, 45 insertions(+), 21 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index e8be217f8a8c..a270441ebba0 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -10,6 +10,8 @@
#include <Library/DebugLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/ArmTrngLib.h>
+#include <Library/RngLib.h>
+#include <Guid/RngAlgorithm.h>
#include "RngDxeInternals.h"
@@ -28,9 +30,13 @@ GetAvailableAlgorithms (
VOID
)
{
- UINT64 DummyRand;
- UINT16 MajorRevision;
- UINT16 MinorRevision;
+ EFI_STATUS Status;
+ UINT16 MajorRevision;
+ UINT16 MinorRevision;
+ GUID RngGuid;
+ BOOLEAN UnSafeAlgo;
+
+ UnSafeAlgo = FALSE;
// Rng algorithms 2 times, one for the allocation, one to populate.
mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
@@ -38,24 +44,29 @@ GetAvailableAlgorithms (
return EFI_OUT_OF_RESOURCES;
}
- // Check RngGetBytes() before advertising PcdCpuRngSupportedAlgorithm.
- if (!EFI_ERROR (RngGetBytes (sizeof (DummyRand), (UINT8 *)&DummyRand))) {
- CopyMem (
- &mAvailableAlgoArray[mAvailableAlgoArrayCount],
- PcdGetPtr (PcdCpuRngSupportedAlgorithm),
- sizeof (EFI_RNG_ALGORITHM)
- );
- mAvailableAlgoArrayCount++;
-
- DEBUG_CODE_BEGIN ();
- if (IsZeroGuid (PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
+ // Identify RngLib algorithm.
+ Status = GetRngGuid (&RngGuid);
+ if (!EFI_ERROR (Status)) {
+ if (IsZeroGuid (&RngGuid) ||
+ CompareGuid (&RngGuid, &gEdkiiRngAlgorithmUnSafe))
+ {
+ // Treat zero GUID as an unsafe algorithm
DEBUG ((
DEBUG_WARN,
- "PcdCpuRngSupportedAlgorithm should be a non-zero GUID\n"
+ "RngLib uses an Unsafe algorithm and "
+ "must not be used for production builds.\n"
));
+ // Set the UnSafeAlgo flag to indicate an unsafe algorithm was found
+ // so that it can be added at the end of the algorithm list.
+ UnSafeAlgo = TRUE;
+ } else {
+ CopyMem (
+ &mAvailableAlgoArray[mAvailableAlgoArrayCount],
+ &RngGuid,
+ sizeof (RngGuid)
+ );
+ mAvailableAlgoArrayCount++;
}
-
- DEBUG_CODE_END ();
}
// Raw algorithm (Trng)
@@ -68,5 +79,15 @@ GetAvailableAlgorithms (
mAvailableAlgoArrayCount++;
}
+ // Add unsafe algorithm at the end of the list.
+ if (UnSafeAlgo) {
+ CopyMem (
+ &mAvailableAlgoArray[mAvailableAlgoArrayCount],
+ &gEdkiiRngAlgorithmUnSafe,
+ sizeof (EFI_RNG_ALGORITHM)
+ );
+ mAvailableAlgoArrayCount++;
+ }
+
return EFI_SUCCESS;
}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
index ce49ff7ae661..78a18c5e1177 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
@@ -78,6 +78,7 @@ RngGetRNG (
{
EFI_STATUS Status;
UINTN Index;
+ GUID RngGuid;
if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
return EFI_INVALID_PARAMETER;
@@ -102,7 +103,10 @@ RngGetRNG (
}
FoundAlgo:
- if (CompareGuid (RNGAlgorithm, PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
+ Status = GetRngGuid (&RngGuid);
+ if (!EFI_ERROR (Status) &&
+ CompareGuid (RNGAlgorithm, &RngGuid))
+ {
Status = RngGetBytes (RNGValueLength, RNGValue);
return Status;
}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index d6c2d30195bf..27d3e39a675b 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -75,13 +75,12 @@ [Guids]
gEfiRngAlgorithmX9313DesGuid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
gEfiRngAlgorithmX931AesGuid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
gEfiRngAlgorithmRaw ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
+ gEfiRngAlgorithmArmRndr ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
+ gEdkiiRngAlgorithmUnSafe ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
[Protocols]
gEfiRngProtocolGuid ## PRODUCES
-[Pcd.AARCH64]
- gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm ## CONSUMES
-
[Depex]
TRUE
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107015): https://edk2.groups.io/g/devel/message/107015
Mute This Topic: https://groups.io/mt/100213737/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Hi Pierre, Thank you for this patch. On Tue, Jul 18, 2023 at 04:52 AM, PierreGondois wrote: > > + gEfiRngAlgorithmArmRndr ## SOMETIMES_PRODUCES ## GUID = > # Unique ID of the algorithm for RNG Can you check if the line above should be part of this patch, please? Otherwise, this patch looks good to me. In either case, Reviewed-by: Sami Mujawar <sami.mujawar@arm.com> Regards, Sami Mujawar -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107609): https://edk2.groups.io/g/devel/message/107609 Mute This Topic: https://groups.io/mt/100213737/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Sami, Yes indeed, this line should be removed, do you want me to send a new version of this patch ? Regards, Pierre On 8/7/23 07:16, Sami Mujawar wrote: > Hi Pierre, > > Thank you for this patch. > > On Tue, Jul 18, 2023 at 04:52 AM, PierreGondois wrote: > > + gEfiRngAlgorithmArmRndr ## SOMETIMES_PRODUCES ## GUID = > # Unique ID of the algorithm for RNG > > Can you check if the line above should be part of this patch, please? > Otherwise, this patch looks good to me. > > In either case, > Reviewed-by: Sami Mujawar <sami.mujawar@arm.com> > > Regards, > > Sami Mujawar -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107688): https://edk2.groups.io/g/devel/message/107688 Mute This Topic: https://groups.io/mt/100213737/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Pierre, Please send an updated version with the r-b collected. That way it would be less work for the maintainer. Regards, Sami Mujawar ________________________________ From: Pierre Gondois <pierre.gondois@arm.com> Sent: Thursday, August 10, 2023 6:49:40 PM To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io <devel@edk2.groups.io> Subject: Re: [edk2-devel] [PATCH v5 8/9] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib Hi Sami, Yes indeed, this line should be removed, do you want me to send a new version of this patch ? Regards, Pierre On 8/7/23 07:16, Sami Mujawar wrote: > Hi Pierre, > > Thank you for this patch. > > On Tue, Jul 18, 2023 at 04:52 AM, PierreGondois wrote: > > + gEfiRngAlgorithmArmRndr ## SOMETIMES_PRODUCES ## GUID = > # Unique ID of the algorithm for RNG > > Can you check if the line above should be part of this patch, please? > Otherwise, this patch looks good to me. > > In either case, > Reviewed-by: Sami Mujawar <sami.mujawar@arm.com> > > Regards, > > Sami Mujawar -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107699): https://edk2.groups.io/g/devel/message/107699 Mute This Topic: https://groups.io/mt/100213737/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.