RDRAND has notoriously been broken many times over its lifespan.
Add a smoketest to RDRAND, in order to better sniff out potential
security concerns.

Also add a proper CPUID test in order to support older CPUs which may
not have it; it was previously being tested but then promptly ignored.

Many thanks to Jason Donenfeld for relicensing his linux RDRAND detection
code to MIT and the public domain.

On Tue, Nov 22, 2022 at 2:21 PM Jason A. Donenfeld <Jason@...> wrote:

<..>

I (re)wrote that function in Linux. I hereby relicense it as MIT, and
also place it into public domain. Do with it what you will now.

Jason

Signed-off-by: Pedro Falcato <pedro.falcato@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Jason A. Donenfeld <Jason@...>
---
MdePkg/Library/BaseRngLib/Rand/RdRand.c | 83 ++++++++++++++++++++++---
1 file changed, 75 insertions(+), 8 deletions(-)

diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
index 070d41e2555f..c8f837c315f1 100644
--- a/MdePkg/Library/BaseRngLib/Rand/RdRand.c
+++ b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
@@ -2,6 +2,7 @@
Random number generator services that uses RdRand instruction access
to provide high-quality random numbers.

+Copyright (c) 2022, Pedro Falcato. All rights reserved.<BR>
Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>

@@ -22,6 +23,72 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

STATIC BOOLEAN mRdRandSupported;

+//
+// Intel SDM says 10 tries is good enough for reliable RDRAND usage.
+//
+#define RDRAND_RETRIES 10
+
+#define RDRAND_TEST_SAMPLES 8
+
+#define RDRAND_MIN_CHANGE 5
+
+STATIC
+BOOLEAN
+TestRdRand (
+ VOID
+ )
+{
+ //
+ // Test for notoriously broken rdrand implementations that always return the same
+ // value, like the Zen 3 uarch (all-1s) or other several AMD families on suspend/resume (also all-1s).
+ // Note that this should be expanded to extensively test for other sorts of possible errata.
+ //
+
+ //
+ // Testing algorithm inspired by linux's arch/x86/kernel/cpu/rdrand.c:x86_init_rdrand
+ // as relicensed by the author, Jason Donenfeld, in the EDK2 mailing list.
+ // As is, the algorithm samples rdrand $RDRAND_TEST_SAMPLES times and expects
+ // a different result $RDRAND_MIN_CHANGE times for reliable RDRAND usage.
+ //
+ UINT32 Prev;
+ UINT8 Idx;
+ UINT8 TestIteration;
+ UINT32 Changed;
+
+ Changed = 0;
+
+ for (TestIteration = 0; TestIteration < RDRAND_TEST_SAMPLES; TestIteration++) {
+ UINT32 Sample;
+ //
+ // Note: We use a retry loop for rdrand. Normal users get this in BaseRng.c
+ // Any failure to get a random number will assume RDRAND does not work.
+ //
+ for (Idx = 0; Idx < RDRAND_RETRIES; Idx++) {
+ if (AsmRdRand32 (&Sample)) {
+ break;
+ }
+ }
+
+ if (Idx == RDRAND_RETRIES) {
+ DEBUG ((DEBUG_ERROR, "BaseRngLib/x86: CPU BUG: Failed to get an RDRAND random number - disabling\n"));
+ return FALSE;
+ }
+
+ if (TestIteration != 0) {
+ Changed += Sample != Prev;
+ }
+
+ Prev = Sample;
+ }
+
+ if (Changed < RDRAND_MIN_CHANGE) {
+ DEBUG ((DEBUG_ERROR, "BaseRngLib/x86: CPU BUG: RDRAND not reliable - disabling\n"));
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
/**
The constructor function checks whether or not RDRAND instruction is supported
by the host hardware.
@@ -46,10 +113,13 @@ BaseRngLibConstructor (
// CPUID. A value of 1 indicates that processor support RDRAND instruction.
//
AsmCpuid (1, 0, 0, &RegEcx, 0);
- ASSERT ((RegEcx & RDRAND_MASK) == RDRAND_MASK);

mRdRandSupported = ((RegEcx & RDRAND_MASK) == RDRAND_MASK);

+ if (mRdRandSupported) {
+ mRdRandSupported = TestRdRand ();
+ }
+
return EFI_SUCCESS;
}

@@ -68,6 +138,7 @@ ArchGetRandomNumber16 (
OUT UINT16 *Rand
)
{
+ ASSERT (mRdRandSupported);
return AsmRdRand16 (Rand);
}

@@ -86,6 +157,7 @@ ArchGetRandomNumber32 (
OUT UINT32 *Rand
)
{
+ ASSERT (mRdRandSupported);
return AsmRdRand32 (Rand);
}

@@ -104,6 +176,7 @@ ArchGetRandomNumber64 (
OUT UINT64 *Rand
)
{
+ ASSERT (mRdRandSupported);
return AsmRdRand64 (Rand);
}

@@ -120,11 +193,5 @@ ArchIsRngSupported (
VOID
)
{
- /*
- Existing software depends on this always returning TRUE, so for
- now hard-code it.
-
- return mRdRandSupported;
- */
- return TRUE;
+ return mRdRandSupported;
}
--
2.38.1

On Tue, Nov 22, 2022 at 4:32 PM Pedro Falcato <pedro.falcato@...> wrote:

+ // Testing algorithm inspired by linux's arch/x86/kernel/cpu/rdrand.c:x86_init_rdrand
+ // as relicensed by the author, Jason Donenfeld, in the EDK2 mailing list.
+ // As is, the algorithm samples rdrand $RDRAND_TEST_SAMPLES times and expects
+ // a different result $RDRAND_MIN_CHANGE times for reliable RDRAND usage.

You don't need to pepper my name all over the source. :)

+ for (TestIteration = 0; TestIteration < RDRAND_TEST_SAMPLES; TestIteration++) {
+ UINT32 Sample;
+ //
+ // Note: We use a retry loop for rdrand. Normal users get this in BaseRng.c
+ // Any failure to get a random number will assume RDRAND does not work.
+ //
+ for (Idx = 0; Idx < RDRAND_RETRIES; Idx++) {
+ if (AsmRdRand32 (&Sample)) {

The linux code will use a 64bit value on 64bit machines. I suggest you
do the same here -- use native word size. I think EFI calls this a
"UINTN".

Jason

On Tue, Nov 22, 2022 at 4:56 PM Pedro Falcato <pedro.falcato@...> wrote:

On Tue, Nov 22, 2022 at 3:39 PM Jason A. Donenfeld <Jason@...> wrote:

On Tue, Nov 22, 2022 at 4:32 PM Pedro Falcato <pedro.falcato@...> wrote:

+ // Testing algorithm inspired by linux's arch/x86/kernel/cpu/rdrand.c:x86_init_rdrand
+ // as relicensed by the author, Jason Donenfeld, in the EDK2 mailing list.
+ // As is, the algorithm samples rdrand $RDRAND_TEST_SAMPLES times and expects
+ // a different result $RDRAND_MIN_CHANGE times for reliable RDRAND usage.

You don't need to pepper my name all over the source. :)

I just wanted to properly credit you :) If you're not okay with it I can remove it in a v3.

Yes, please remove.

>> > + for (TestIteration = 0; TestIteration < RDRAND_TEST_SAMPLES;
TestIteration++) {

+ UINT32 Sample;
+ //
+ // Note: We use a retry loop for rdrand. Normal users get this in BaseRng.c
+ // Any failure to get a random number will assume RDRAND does not work.
+ //
+ for (Idx = 0; Idx < RDRAND_RETRIES; Idx++) {
+ if (AsmRdRand32 (&Sample)) {

The linux code will use a 64bit value on 64bit machines. I suggest you
do the same here -- use native word size. I think EFI calls this a
"UINTN".

Hmm, do you reckon it makes a difference?

Yes, it does. Please account for this. Use an ifdef or something if
you need. It's not very hard.

Jason