std: sys: random: uefi: Provide rdrand based fallback

[Ayush1325]

Some UEFI systems based on American Megatrends Inc. v3.3 do not provide RNG support 1. So fallback to rdrand in such cases.

Fixes #138252

cc @seijikun

[rustbot]

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[joboet]

Unfortunately, the rdrand and rdseed instructions are not always safe or secure to use and thus require some checks (see the getrandom crate's backend for some of the issues). So std should either perform those checks or alternatively only use rdrand for generating hashmap keys (which would resolve the reported issue).

[Ayush1325]

Unfortunately, the rdrand and rdseed instructions are not always safe or secure to use and thus require some checks (see the getrandom crate's backend for some of the issues). So std should either perform those checks or alternatively only use rdrand for generating hashmap keys (which would resolve the reported issue).

@seijikun Can you check if your hardware passes the checks in getrandom rand backend?

[seijikun]

Can you check if your hardware passes the checks in getrandom rand backend?

@Ayush1325 Sure! I don't have access to it at the moment, unfortunately - but I will do it monday morning.
You probably mean is_rdrand_good() and self_test(), right?

[Ayush1325]

Can you check if your hardware passes the checks in getrandom rand backend?

@Ayush1325 Sure! I don't have access to it at the moment, unfortunately - but I will do it monday morning. You probably mean is_rdrand_good() and self_test(), right?

is_rdrand_good calls self_test, so yup.

[seijikun]

@Ayush1325 Looks good:

[Ayush1325]

@Ayush1325 Looks good:

Great, I will update the PR soon

[Ayush1325]

@seijikun Can you take a look now?

[seijikun]

Sorry, took me a while to find out how this custom-std stuff works. This is how I did it:

git clone --filter="blob:none" https://github.com/Ayush1325/rust.git
cd rust
git checkout uefi-rand-fallback

./x setup dist
echo "[target.x86_64-unknown-linux-gnu]" >> ./bootstrap.toml
echo "no-std = false" >> ./bootstrap.toml
echo "[target.x86_64-unknown-uefi]" >> ./bootstrap.toml
echo "no-std = false" >> ./bootstrap.toml
echo "[build]" >> ./bootstrap.toml
echo "target=[\"x86_64-unknown-linux-gnu\", \"x86_64-unknown-uefi\"]" >> ./bootstrap.toml

./x build --stage 0
rustup toolchain link stage0 build/host/stage0-sysroot

And then, I built the project (consisting of ~3 code lines that just use a HashMap) using:

cargo +stage0 build --target x86_64-unknown-uefi --release

[seijikun]

@Ayush1325 There is currently still a bug in the implementation.
UEFI's locate_handles function has one very weird quirk. When the resulting list is empty (as in my case), it doesn't return an empty list but instead fails with EFI_NOT_FOUND. That's an error I have stumbled over more than I'd like to admit...

So the normal case has to be edited to look like this:

--- a/library/std/src/sys/random/uefi.rs
+++ b/library/std/src/sys/random/uefi.rs
@@ -3,22 +3,22 @@
 use crate::sys::pal::helpers;
 
 pub fn fill_bytes(bytes: &mut [u8]) {
-    let handles =
-        helpers::locate_handles(rng::PROTOCOL_GUID).expect("failed to generate random data");
-    for handle in handles {
-        if let Ok(protocol) = helpers::open_protocol::<rng::Protocol>(handle, rng::PROTOCOL_GUID) {
-            let r = unsafe {
-                ((*protocol.as_ptr()).get_rng)(
-                    protocol.as_ptr(),
-                    crate::ptr::null_mut(),
-                    bytes.len(),
-                    bytes.as_mut_ptr(),
-                )
-            };
-            if r.is_error() {
-                continue;
-            } else {
-                return;
+    if let Ok(handles) = helpers::locate_handles(rng::PROTOCOL_GUID) {
+        for handle in handles {
+            if let Ok(protocol) = helpers::open_protocol::<rng::Protocol>(handle, rng::PROTOCOL_GUID) {
+                let r = unsafe {
+                    ((*protocol.as_ptr()).get_rng)(
+                        protocol.as_ptr(),
+                        crate::ptr::null_mut(),
+                        bytes.len(),
+                        bytes.as_mut_ptr(),
+                    )
+                };
+                if r.is_error() {
+                    continue;
+                } else {
+                    return;
+                }
             }
         }
     }

[seijikun]

I made the mentioned change on top of your branch locally. And as soon as that is changed...:

use std::{collections::HashMap, time::Duration};

fn main() {
    let mut map = HashMap::new();
    map.insert("Ayush1325", "is awesome");
    println!("{:?}", map);
    std::thread::sleep(Duration::from_millis(31337));
}

... it works! Thank you!

[Ayush1325]

@Ayush1325 There is currently still a bug in the implementation. UEFI's locate_handles function has one very weird quirk. When the resulting list is empty (as in my case), it doesn't return an empty list but instead fails with EFI_NOT_FOUND. That's an error I have stumbled over more than I'd like to admit...

Thanks for reporting this. I have updated the PR (and also done some refactoring).

[seijikun]

Just tested against the newest commit: 5c59ba8
Works ✔️

[Ayush1325]

@joboet Does it look fine now?

[workingjubilee]

r? @joboet

[joboet]

This looks fine to me, only the zero-request case is handled incorrectly.

[joboet]

A zero-byte request should always succeed, but will currently panic if rdrand is not available as the UEFI protocol will return EFI_INVALID_PARAMETER.

Suggested change

	// Try EFI_RNG_PROTOCOL
	if bytes.is_empty() {
	return;
	}
	// Try EFI_RNG_PROTOCOL

[Ayush1325]

Done

[joboet]

You could also use array_chunks_mut to eliminate the copy_from_slice.

[Ayush1325]

Done