回复: [edk2-devel] [PATCH v4 1/1] MdePkg/BaseRngLib: Add a smoketest for RDRAND and check CPUID


gaoliming
 

Pedro:
To keep the same behavior, I suggest to only update
BaseRngLibConstructor() API with TestRdRand, don't touch other APIs.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pedro Falcato
发送时间: 2022年11月23日 6:31
收件人: devel@edk2.groups.io
抄送: Pedro Falcato <pedro.falcato@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao <gaoliming@...>;
Zhiguang Liu <zhiguang.liu@...>; Jason A . Donenfeld
<Jason@...>
主题: [edk2-devel] [PATCH v4 1/1] MdePkg/BaseRngLib: Add a smoketest for
RDRAND and check CPUID

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.

Testing algorithm inspired by linux's arch/x86/kernel/cpu/rdrand.c
:x86_init_rdrand() per commit 049f9ae9..

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
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4163

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@...>
---
v4: Added a doxygen comment to the TestRdRand() function
v3: Addressed mailing list comments
v2: Replaced the original v1 naive detection with a better, although still
not
great, detection.
MdePkg/Library/BaseRngLib/Rand/RdRand.c | 99
+++++++++++++++++++++++--
1 file changed, 91 insertions(+), 8 deletions(-)

diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c
b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
index 070d41e2555f..ff99436dbbbd 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,88 @@ 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
+
+//
+// Add a define for native-word RDRAND, just for the test.
+//
+#ifdef MDE_CPU_X64
+#define AsmRdRand AsmRdRand64
+#else
+#define AsmRdRand AsmRdRand32
+#endif
+
+/**
+ Tests RDRAND for broken implementations.
+
+ @retval TRUE RDRAND is reliable (and hopefully safe).
+ @retval FALSE RDRAND is unreliable and should be disabled,
despite CPUID.
+
+**/
+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.
+ //
+
+ //
+ // Our algorithm samples rdrand $RDRAND_TEST_SAMPLES times and
expects
+ // a different result $RDRAND_MIN_CHANGE times for reliable RDRAND
usage.
+ //
+ UINTN Prev;
+ UINT8 Idx;
+ UINT8 TestIteration;
+ UINT32 Changed;
+
+ Changed = 0;
+
+ for (TestIteration = 0; TestIteration < RDRAND_TEST_SAMPLES;
TestIteration++) {
+ UINTN 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 (AsmRdRand (&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;
+}
+
+#undef AsmRdRand
+
/**
The constructor function checks whether or not RDRAND instruction is
supported
by the host hardware.
@@ -46,10 +129,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 +154,7 @@ ArchGetRandomNumber16 (
OUT UINT16 *Rand
)
{
+ ASSERT (mRdRandSupported);
return AsmRdRand16 (Rand);
}

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

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

@@ -120,11 +209,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