[PATCH v4 0/2] SecurityPkg: Fixes for RngDxe

[PATCH v4 0/2] SecurityPkg: Fixes for RngDxe

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

v1:
- https://edk2.groups.io/g/devel/message/96356
v2:
- https://edk2.groups.io/g/devel/message/96434
- Reformulate commit message.
- Do not warn if no algorithm is found as the message
  would be printed on non-Arm platforms.
v3:
- https://edk2.groups.io/g/devel/topic/95240503
- Add the following patches:
  1. ArmPkg/ArmTrngLib: Remove ASSERTs in ArmTrngLibConstructor()
     Requested by Ard.
     Cf https://edk2.groups.io/g/devel/message/96495
  2. SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTOCOL
     Do not install EFI_RNG_PROTOCOL if no RNG algorithm is available.
     Cf. https://edk2.groups.io/g/devel/message/96494
  3. SecurityPkg/RngDxe: Fix Rng algo selection for Arm
     Coming from v2 patch being split.
v4:
- Change description of:
   'SecurityPkg/RngDxe: Fix Rng algo selection for Arm'
  to:
   'SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm'
- Add patch:
    'SecurityPkg/RngDxe: Check for NULL PcdCpuRngSupportedAlgorithm'
- Remove as merged:
    SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTOCOL
    SecurityPkg/RngDxe: Correctly update mAvailableAlgoArrayCount

When adding support for the Firmware Trng interface, it was made
available through in the RngDxe module. In RngDxe, the algorithm
associated with PcdCpuRngSupportedAlgorithm (implemented by the
RngLib) was always advertised by default.

It was assumed that support for RngLib and RNDR instructions
should be kept in ArmVirtPkg. However this support did not exist
as all ArmVirtPkg use the BaseRngLibTimerLib.inf implementation
of the RngLib.

Do not advertise the RngLib and PcdCpuRngSupportedAlgorithm
if PcdCpuRngSupportedAlgorithm is NULL.

Also simplify the selection of the default algorithm.

Pierre Gondois (2):
  SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm
  SecurityPkg/RngDxe: Check for NULL PcdCpuRngSupportedAlgorithm

 .../RngDxe/AArch64/AArch64Algo.c                | 14 +++-----------
 .../RandomNumberGenerator/RngDxe/ArmRngDxe.c    | 17 ++++-------------
 2 files changed, 7 insertions(+), 24 deletions(-)

-- 
2.25.1

[PATCH v4 1/2] SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

The default algorithm to use is the first element of
mAvailableAlgoArray. There is no need to iterate through the
array to find a valid algorithm.

Simplify the Rng algorithm selection by returning the first
element of mAvailableAlgoArray.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .../RandomNumberGenerator/RngDxe/ArmRngDxe.c    | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
index ce49ff7ae661..b8a343e3d397 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
@@ -77,7 +77,6 @@ RngGetRNG (
   )
 {
   EFI_STATUS  Status;
-  UINTN       Index;
 
   if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
     return EFI_INVALID_PARAMETER;
@@ -87,21 +86,13 @@ RngGetRNG (
     //
     // Use the default RNG algorithm if RNGAlgorithm is NULL.
     //
-    for (Index = 0; Index < mAvailableAlgoArrayCount; Index++) {
-      if (!IsZeroGuid (&mAvailableAlgoArray[Index])) {
-        RNGAlgorithm = &mAvailableAlgoArray[Index];
-        goto FoundAlgo;
-      }
-    }
-
-    if (Index == mAvailableAlgoArrayCount) {
-      // No algorithm available.
-      ASSERT (Index != mAvailableAlgoArrayCount);
-      return EFI_DEVICE_ERROR;
+    if (mAvailableAlgoArrayCount != 0) {
+      RNGAlgorithm = &mAvailableAlgoArray[0];
+    } else {
+      return EFI_UNSUPPORTED;
     }
   }
 
-FoundAlgo:
   if (CompareGuid (RNGAlgorithm, PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
     Status = RngGetBytes (RNGValueLength, RNGValue);
     return Status;
-- 
2.25.1

[PATCH v4 2/2] SecurityPkg/RngDxe: Check for NULL PcdCpuRngSupportedAlgorithm

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151

The EFI_RNG_PROTOCOL can advertise multiple algorithms through
Guids. The PcdCpuRngSupportedAlgorithm contains a Guid that
can be configured. It represents the algorithm used in RngLib
and using CPU instructions.

When adding support for the Firmware Trng interface, it was made
available through in the RngDxe module. In RngDxe, the algorithm
associated with PcdCpuRngSupportedAlgorithm (implemented by the
RngLib) was always advertised by default.

It was assumed that support for RngLib and RNDR instructions
should be kept in ArmVirtPkg. However this support did not exist
as all ArmVirtPkg use the BaseRngLibTimerLib.inf implementation
of the RngLib.

Do not advertise the RngLib and PcdCpuRngSupportedAlgorithm
if PcdCpuRngSupportedAlgorithm is NULL.

Reported-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .../RngDxe/AArch64/AArch64Algo.c                   | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index e8be217f8a8c..c98e09363e25 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -39,23 +39,15 @@ GetAvailableAlgorithms (
   }
 
   // Check RngGetBytes() before advertising PcdCpuRngSupportedAlgorithm.
-  if (!EFI_ERROR (RngGetBytes (sizeof (DummyRand), (UINT8 *)&DummyRand))) {
+  if (!IsZeroGuid (PcdGetPtr (PcdCpuRngSupportedAlgorithm)) &&
+      !EFI_ERROR (RngGetBytes (sizeof (DummyRand), (UINT8 *)&DummyRand)))
+  {
     CopyMem (
       &mAvailableAlgoArray[mAvailableAlgoArrayCount],
       PcdGetPtr (PcdCpuRngSupportedAlgorithm),
       sizeof (EFI_RNG_ALGORITHM)
       );
     mAvailableAlgoArrayCount++;
-
-    DEBUG_CODE_BEGIN ();
-    if (IsZeroGuid (PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
-      DEBUG ((
-        DEBUG_WARN,
-        "PcdCpuRngSupportedAlgorithm should be a non-zero GUID\n"
-        ));
-    }
-
-    DEBUG_CODE_END ();
   }
 
   // Raw algorithm (Trng)
-- 
2.25.1