delete some terrible code that was written to work around cargo bugs (#…

…2503)

* delete some terrible code that was written to work around cargo bugs

many years ago, we encountered this bug:
rust-lang/cargo#6571

This bug meant that if we wanted to have a single Rng type that worked
on all platforms, including SGX, then it could not use the standard
library on any platform, because cargo would evaluate dependencies
without respect to what platform you are on.

However, it was really important for our code that we had such an
abstraction layer. This was important for writing enclave impl code
that could run in SGX and also in unit tests for instance.

The way we solved this issue was that, we took the current version
of `ThreadRng` which is the generically recommendable cryptographic
RNG type from `rand` crate, and made the smallest possible changes
to it until it would build without the standard library. The main
change was replacing `std::thread_local!` with the `#[thread_local]`
attribute, which turned out to be straightforward. However, it creates
an annoying maintenance burden on us, and there has been a lot of
churn in the rand crate since then.

Since the `resolver = 2` stuff was stabilized, the original problem
is no longer the case. It's fine to have crates that pull in `std`
in one platform but not another. So we can now just use `ThreadRng`
as the no-RDRAND fallback, like we wanted all along.

* fixup from review comments

* remove unnecessary dependencies

crypto/rand/Cargo.toml

@@ -4,16 +4,17 @@ version = "2.0.0"
 authors = ["MobileCoin"]
 edition = "2021"
 description = '''
-This crate provides a no-std compatible rng called `RdRandRng`.
+This crate provides a no-std compatible rng called `McRng`.
 
 On targets with +rdrand target feature, it uses the intel RDRAND instruction
 to get randomness directly from the CPU, bypassing a dependency on the OS, libc,
-etc. For servers using SGX, this rng works in trusted and untrusted code equally
-well without changes, which is convenient.
+etc. For servers using SGX, this rng works in no_std trusted and untrusted code
+equally well without changes, which is convenient.
 
-For targets without rdrand, we provide a fallback implementation similar to the
-standard rust `rand` crate's `thread_rng`, using the `getrandom` crate OsRng to
-seed a thread-local rng. `RdRandRng` is in all configurations a zero-width type.
+For targets without rdrand, `McRng` is `ThreadRng`.
+On wasm, `ThreadRng` is not available, so `McRng` is `OsRng`.
+
+`McRng` is in all configurations a zero-width type.
 '''
 
 [features]
@@ -22,7 +23,5 @@ std = ["rand_core/std", "rand/std", "rand/std_rng"]
 
 [dependencies]
 cfg-if = "1.0"
-getrandom = { version = "0.2", default_features = false }
 rand = { version = "0.8", default-features = false }
 rand_core = { version = "0.6", default-features = false }
-rand_hc = "0.3"

crypto/rand/README.md

@@ -1,28 +1,30 @@
 mc-crypto-rand
 ======
 
-`mcrand` crate provides an rng type which is:
+`mc-crypto-rand` crate provides an rng type which is:
 (1) no_std compatible
 (2) uses RDRAND when available, in and out of the enclave on sgx servers,
 (3) uses something like rand::ThreadRng on any other platforms
 
 `mc-crypto-rand` does not require any cargo feature configuration to be used correctly,
-which simplifies the build
+which simplifies the build.
 
 When `+rdrand` rustc target feature is enabled, we use x86 intrinsics directly to
 do rdrand correctly, and do not pull in getrandom or any other library.
 
-when this target feature is not enabled, we do something that is almost the same as
-`rand::ThreadRng`, but we use the nightly `#[thread_local]` api instead of `std::thread_local!`
-macro. In fact we rely on rand for the implementation details, but we turn of `std` feature of rand.
+When this target feature is not enabled, we use `rand::ThreadRng`, except on WASM where
+we use `rand::OsRng`.
+
+Note that these fallbacks require the standard library. We don't currently
+have any targets that don't have RDRAND and also can't use the standard library.
 
 Example usage:
 
 ```
-use mc_crypto_rand::{RdRandRng, RngCore}
+use mc_crypto_rand::{McRng, RngCore}
 
 pub fn my_func() -> (u64, u64) {
-    let mut rng = RdRandRng{};
+    let mut rng = McRng{};
     let k0 = rng.next_u64();
     let k1 = rng.next_u64();
     (k0, k1)

crypto/rand/src/fallback.rs

@@ -1,97 +1,5 @@
 // Copyright (c) 2018-2022 The MobileCoin Foundation
 
-use core::option::Option;
-use rand::rngs::adapter::ReseedingRng;
-use rand_core::{impls, CryptoRng, Error, RngCore, SeedableRng};
+use rand::rngs::ThreadRng;
 
-// Using Hc128Rng because it was (at one point) the StdRng of rand crate, up to
-// version 0.6 We don't want to depend on rand because it depends on std, and
-// specifically, its implementation of ThreadRng depends on rust stdlib
-// thread_local! macro
-use rand_hc::Hc128Core as Core;
-
-// Number of generated bytes after which to reseed `ThreadRng`.
-// This constant is taken from rand and is not public.
-const THREAD_RNG_RESEED_THRESHOLD: u64 = 1024 * 64;
-
-type InnerRng = ReseedingRng<Core, OsRng>;
-
-// Compare this impl with rand::thread_rng
-#[thread_local]
-static mut THREAD_LOCAL_RNG: Option<InnerRng> = None;
-
-// Get the thread local instance, initializing it if that hasn't happened yet in
-// this thread
-fn get_thread_local_rng() -> &'static mut InnerRng {
-    // unsafe is required because we are accessing a static mut,
-    // but it is thread_local so this is not actually a problem.
-    unsafe {
-        THREAD_LOCAL_RNG.get_or_insert_with(|| {
-            let r = Core::from_rng(OsRng)
-                .unwrap_or_else(|err| panic!("could not initialize thread_rng: {}", err));
-            ReseedingRng::new(r, THREAD_RNG_RESEED_THRESHOLD, OsRng)
-        })
-    }
-}
-
-// The ZWT we give to users
-// Similar to rand::ThreadRng
-#[derive(Default)]
-pub struct McRng;
-
-// Forward implementation from InnerRng
-impl RngCore for McRng {
-    #[inline]
-    fn next_u32(&mut self) -> u32 {
-        get_thread_local_rng().next_u32()
-    }
-    fn next_u64(&mut self) -> u64 {
-        get_thread_local_rng().next_u64()
-    }
-    fn fill_bytes(&mut self, dest: &mut [u8]) {
-        get_thread_local_rng().fill_bytes(dest)
-    }
-    fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> {
-        get_thread_local_rng().try_fill_bytes(dest)
-    }
-}
-
-impl CryptoRng for McRng {}
-
-/// OsRng
-///
-/// We have to copy OsRng from rand_core unfortunately, because cargo:
-/// To get rand_core::OsRng, we must turn on `rand_core/getrandom` cargo
-/// feature, but this appears to turns on `libc/std`.
-/// There is therefore no way to get rand_core::OsRng without turning on the
-/// standard library. Fortunately this body of code is trivial
-///
-/// This may just be a versioning issue? it may be that if rand_core upgrades
-/// the version of getrandom that they rely on, we will be able to use
-/// rand_core/getrandom without getting std, and then wouldn't have to carry
-/// this.
-#[derive(Clone, Copy, Debug, Default)]
-struct OsRng;
-
-impl CryptoRng for OsRng {}
-
-impl RngCore for OsRng {
-    fn next_u32(&mut self) -> u32 {
-        impls::next_u32_via_fill(self)
-    }
-
-    fn next_u64(&mut self) -> u64 {
-        impls::next_u64_via_fill(self)
-    }
-
-    fn fill_bytes(&mut self, dest: &mut [u8]) {
-        if let Err(e) = self.try_fill_bytes(dest) {
-            panic!("Error: {}", e);
-        }
-    }
-
-    fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> {
-        getrandom::getrandom(dest).map_err(|e| e.code())?;
-        Ok(())
-    }
-}
+pub type McRng = ThreadRng;

crypto/rand/src/lib.rs

@@ -1,16 +1,14 @@
 // Copyright (c) 2018-2022 The MobileCoin Foundation
 
 #![no_std]
-// The fallback code needs the unstable [thread_local] attribute
-#![cfg_attr(not(target_feature = "rdrand"), feature(thread_local))]
 
 pub extern crate rand_core;
 
 pub use rand_core::{CryptoRng, RngCore};
 
 use cfg_if::cfg_if;
 
-// Not using cfg_attr( ..., path = fallback.rs) because it appears to confused
+// Not using cfg_attr( ..., path = fallback.rs) because it appears to confuse
 // rustfmt
 cfg_if! {
     if #[cfg(target_feature = "rdrand")] {