From 19438cff973bfb35a1ef12fab45fabb28b63fe64 Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Fri, 11 Aug 2023 16:33:09 +0200 Subject: SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib 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 Reviewed-by: Sami Mujawar Acked-by: Ard Biesheuvel Acked-by: Jiewen Yao Tested-by: Kun Qin --- .../RngDxe/AArch64/AArch64Algo.c | 55 +++++++++++++++------- .../RandomNumberGenerator/RngDxe/ArmRngDxe.c | 6 ++- .../RandomNumberGenerator/RngDxe/RngDxe.inf | 4 +- 3 files changed, 44 insertions(+), 21 deletions(-) (limited to 'SecurityPkg') diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c index e8be217..a270441 100644 --- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c +++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c @@ -10,6 +10,8 @@ #include #include #include +#include +#include #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 ce49ff7..78a18c5 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 d6c2d30..8704a64 100644 --- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf +++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf @@ -75,13 +75,11 @@ 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 + gEdkiiRngAlgorithmUnSafe ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG [Protocols] gEfiRngProtocolGuid ## PRODUCES -[Pcd.AARCH64] - gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm ## CONSUMES - [Depex] TRUE -- cgit v1.1