embedded source compatibility about getrandom HOT 18 CLOSED

Well first, the spin crate is unmaintained... that aside...

I suppose this accomplishes the goal of having something that potentially works for embedded, however I think it’s complex solution as opposed to passing a &mut MyTrng where MyTrng: RngCore.

I’d probably still choose to use RngCore over getrandom for anything intended for embedded use.

One nit here:

 &mut Device = guard.get_or_insert(Device::new()?);

This is assuming device initialization as a side effect, which isn’t how the embedded-hal impls I’ve used work. Instead you receive all of the device’s “pins” (one of which you’d use to talk to a TRNG) passed in as an owned value.

To make that work with what you’ve proposed, you’d need a more once_cell-like API for registering an existing owned TRNG device.

from getrandom.

I’d probably still choose to use RngCore over getrandom for anything intended for embedded use.

I'd agree that's the best bet if you're using something directly. The main reason for the custom impl is to handle cases where getrandom is a transitive dependency, because in that case you might not be able to pass around state.

from getrandom.

One solution here would be a special feature enabling an alternative mode:

#[cfg(feature = "getrandom_custom")]
extern "C" {
    fn getrandom_custom(dest: *mut u8, len: usize) -> u32;
}

pub fn getrandom(dest: &mut [u8]) -> Result<(), Error> {
    #[cfg(feature = "getrandom_custom")] {
        use core::num::NonZeroU32;
        let ret = unsafe { getrandom_custom(dest.as_mut_ptr(), dest.len()) };
        return if let Some(err) = NonZeroU32::new(ret) {
            Err(Error::from(err))
        } else {
            Ok(())
        };
    }
    getrandom_inner(dest)
}

This would allow any single crate in the build to replace the entropy source, which has both advantages (easier testing with dummy entropy, easy of usage of other randomness source libs) and disadvantages (relatively easy to accidentally or possibly even maliciously mess this up; no single place to check whether the alternative is enabled).

Another solution (which doesn't need any support here) would be to make a replacement crate with the same API (copy error types, provide fn getrandom), then use a patch section in the top Cargo.toml to replace this.

[patch.crates-io]
getrandom = { path = "my/replacement/getrandom" }

from getrandom.

There is another issue with no_std support: the error type should implement std::error::Error iff using std. Currently the lib does so on all platforms except SGX, i.e. this is automatic and requires std on all platforms which should support std. IIRC we did once have a reason to disable support on Mac OS.

In contrast, rand_core makes std-support a crate feature, and causes confusing errors if you get this wrong in the build.

from getrandom.

Note: there is a long discussion on this topic in rand#579 with several sub-topics:

• a hook to set an entropy source
• synchronisation/shared memory
• managing an entropy pool
• use of lang items

At any rate, development appears to be stalled due to lack of demand.

from getrandom.

@dhardy I opened an issue on the embedded WG to call attention to the issue of no_std support (and pinged some current embedded rand_core users) rust-embedded/wg#353

from getrandom.

@dhardy now that #58 is merged, do we still want this to stay open? Should it be renamed to focus on just providing user-defined custom implementations?

from getrandom.

Some of the ideas aired above may still be relevant in the future, e.g. use of an entropy pool (via another crate). This issue is too broad I guess, ranging from std dependencies to how to get entropy on embedded platforms. Perhaps I should rename it to focus on that?

from getrandom.

#58 haven't changed that much regarding no_std/embedded because it's still using

from getrandom.

@mati865
This module is feature gated behind the std feature. You forgot to use default-features = false for rand (it enables std feature by default), after that you can enable getrandom feature manually. But note that you will not have ThreadRng with such config, so you will have to cascade std feature to rand and switch between ThreadRng and OsRng depending on std.

from getrandom.

@newpavlov I totally missed that gate and couldn't find default features field for this crate. I haven't thought about other rand crates enabling it.
Thank you for the explanation and sorry for the noise.

from getrandom.

Do we think that Custom RNGs (#109) and the cpu feature (#133) are sufficient to close this issue?

from getrandom.

@josephlr I think both of those PRs are irrelevant to the embedded use case, as none of them cover embedded targets.

Per the discussion on rust-embedded/wg#353 I’m wondering of the getrandom API is fundamentally incompatible with embedded-hal style TRNG peripherals, since getrandom generates randomness as a side effect, and accessing a TRNG will typically involve passing in an owned peripheral device.

I think the existing RngCore trait probably provides the most embedded-friendly API.

from getrandom.

#109 (in addition to dealing with WASM targets) also adds a generic way for an external crate to register a custom getrandom implementation. This concept of an externally defined hook was specifically intended to work with embedded targets.

For example, if there was some external device that you want to get randomness from, you would write a custom crate which would depend on getrandom like:

name = "mydevice-getrandom"
[dependencies]
getrandom = { version = "0.2", features = ["custom"] }

And your code would then specify a function to be called. A sample lib.rs would look like:

use spin::Mutex;
use getrandom::{register_custom_getrandom, Error};

struct Device{ /* device specific state */ }

impl Device {
  fn new() -> Result<Self, Error> {
    /* Initialize and Create the device */
  }
  fn read(&mut self, buf: &mut [u8]) -> Result<(), Error> {
    /* Read randomness from the device */
  }
}

// Global instance of device
static DEVICE: Mutex<Option<Device>> = Mutex::new(None);

fn my_getrandom(buf: &mut[u8]) -> Result<(), Error> {
  let mut guard = DEVICE.lock();
  let dev: &mut Device = guard.get_or_insert(Device::new()?);
  dev.read(buf)
}

register_custom_getrandom!(my_getrandom);

The above code uses the no_std-compatible spin::Mutex, but you could also use lock_api::Mutex if you wanted to avoid spin locks and use platform specific synchronization primitives.

@tarcieri, Is there a reason that wouldn't work for your use case?

from getrandom.

While the embedded-hal crate specifically seems to encourage Global Singletons (per their example code), I realize that this does not map onto all workflows.

Alternatively, if you had some device-specific mydevice crate which exported an api like:

struct RNGDevice{/* private */}

impl RNGDevice {
  fn get_mut() -> Result<&mut Self, Error>;
  fn read(&mut self, buf: &mut [u8]) -> Result<(), Error>;
}

You could implement a mydevice-getrandom crate like:

use mydevice::RNGDevice;
use getrandom::{register_custom_getrandom, Error};

fn my_getrandom(buf: &mut[u8]) -> Result<(), Error> {
  RNGDevice::get_mut()?.read(buf)
}

register_custom_getrandom!(my_getrandom);

from getrandom.

Appologies for the late reply. @josephlr's example above can certainly be adapted to keep the global singleton but perform initialisation via a user-called function — it just means that getrandom must block or fail if called before this happens.

However, granted, this is a lot more complexity than strictly required. getrandom was intended for initialising ThreadRng and similar constructs which just work — but attempting to make this compatible with non-std targets may simply be misguided (since the latter also requires thread-local memory or global memory with a lock, and the whole construct carries a lot more complexity than is desirable on embedded).

Should we then close this issue as not-worth-delivering?

from getrandom.

#109 seems good enough to address the immediate issue

from getrandom.

Guess I'll close this then.

from getrandom.