acidanthera / bugtracker

Acidanthera Bugtracker
385 stars 45 forks source link

Replace BaseRngLib with OcRngLib #307

Closed vit9696 closed 5 years ago

vit9696 commented 5 years ago

BaseRngLib from UDK uses rdrand CPU instruction to implement random number generation. This instruction is not available on Sandy Bridge CPUs or earlier.

The only place where we actually use BaseRngLib is AptioMemoryFix (https://github.com/search?utf8=✓&q=org%3Aacidanthera+BaseRngLib&type=Code), which has a fallback to rdtsc (https://github.com/acidanthera/AptioFixPkg/blob/f91dc96/Platform/AptioMemoryFix/CustomSlide.c#L125).

However, BaseRngLib asserts on library construction in this case, which prevents booting in DEBUG and NOOPT versions: https://github.com/tianocore/edk2/blob/c8b6f16/MdePkg/Library/BaseRngLib/BaseRng.c#L55.

The proposed solution is to implement our own library, which will implement BaseRngLib functionality with a TSC fallback. It is discussible on how to implement it from security standpoint, as it is not obvious whether BaseRngLib is meant to provide secure random number generation for our purposes.

Other than that we also need to consider implementing stack canary, which also needs random seed generation very early. See https://github.com/acidanthera/OcSupportPkg/blob/18685a5/Library/OcGuardLib/Canary.c#L20. Perhaps this is a separate issue.

mhaeuser commented 5 years ago

The support check used by BaseRngLib to ASSERT and by AptioMemoryFix to determine RdRand support only applies to Intel. While certain AMD models support this instruction, the RdRand bit in the utilised CPUID leaf remains reserved.