Add function which returns random array?

[newpavlov]

getrandom is often used to generate seed for user-space PRNG. #279 adds unsafe raw API which allows to remove redundant initialization of buffers. So I wonder if it could be worth to introduce the following safe wrappers:

pub fn getrandom_array<const N: usize>() -> Result<[u8; N], Error> {
    let mut buf = mem::MaybeUninit::<[u8; N]>::uninit();
    unsafe { getrandom_raw(buf.as_mut_ptr().cast(), N).map(|()| buf.assume_init()) }
}

pub fn getrandom_u32() -> Result<u32, Error> {
    getrandom_array().map(u32::from_ne_bytes)
}

pub fn getrandom_u64() -> Result<u64, Error> {
    getrandom_array().map(u64::from_ne_bytes)
}

// and maybe other wrappers for u16, u128, i32, i64, etc.

Of course, getrandom_array implies MSRV bump to 1.51, which could be done in sync with rand v0.9.

[josephlr]

Personally, I don't think adding the API is worth dealing with a v0.3 or some sort of conditional compilation. This seems like the type of thing best left to rand_core. The API suggested here is essentially just impl RngCore for OsRng

I think it would b e better to just change the rand_core implementation to be:

impl RngCore for OsRng {
    fn next_u32(&mut self) -> u32 {
        let mut u = mem::MaybeUninit::uninit();
        unsafe { getrandom_raw(u.as_mut_ptr().cast(), mem::size_of::<u32>()) }.unwrap();
        u.assume_init()
    }

    fn next_u64(&mut self) -> u64 {
        let mut u = mem::MaybeUninit::uninit();
        unsafe { getrandom_raw(u.as_mut_ptr().cast(), mem::size_of::<u64>()) }.unwrap();
        u.assume_init()
    }

    fn fill_bytes(&mut self, dest: &mut [u8]) {
        self.try_fill_bytes(dest).unwrap()
    }

    fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> {
        Ok(getrandom(dest)?)
    }
}

Then if rand_core wants to add methods for dealing with uninitialized memory or arrays, we can add that.

[newpavlov]

I think the methods could be useful for users which depend directly on getrandom without relying on rand_core. Plus it would reduce amount of unsafe in rand_core (by moving it to getrandom, but still).

I agree that probably it's not worth to release together with rand v0.9, but it could be worth to consider during the next breaking release (which could be 1.0, not 0.3).

[briansmith]

I think it is a very useful feature.

This seems like the type of thing best left to rand_core

I disagree. I'd rather not need to depend on rand_core to get this functionality. I would rather reimplement getrandom_array on top of whatever API is offered. But, I shouldn't have to implement it myself, especially because it requires the use of unsafe or inefficient or limiting constructs. Also, some projects ban rand_core due to the fact that it encourages the use of non-secure random generators.

Of course, getrandom_array implies MSRV bump to 1.51, which could be done in sync with rust-random/rand#1165.

Rust 1.51.1 was released over 1.5 years ago (March 2021). I support increasing the MSRV to 1.51 to get this functionality.

[briansmith]

pub fn getrandom_array<const N: usize>() -> Result<[u8; N], Error> {
    let mut buf = mem::MaybeUninit::<[u8; N]>::uninit();
    unsafe { getrandom_raw(buf.as_mut_ptr().cast(), N).map(|()| buf.assume_init()) }
}

This is exactly what rust-lang/rust#80149 would best use.

pub fn getrandom_u32() -> Result<u32, Error> {
    getrandom_array().map(u32::from_ne_bytes)
}

pub fn getrandom_u64() -> Result<u64, Error> {
    getrandom_array().map(u64::from_ne_bytes)
}

I'm not really sure that these are justified to add.

[briansmith]

pub fn getrandom_u32() -> Result<u32, Error> {
    getrandom_array().map(u32::from_ne_bytes)
}

pub fn getrandom_u64() -> Result<u64, Error> {
    getrandom_array().map(u64::from_ne_bytes)
}

I looked more deeply into some applications that would use these and I found that having functions that return u32/u64/etc. don't cover many of the use cases. In all the cases I cared about, I needed different or additional mappings, e.g. mapping to Wrapping<T> or equivalent. Further, it seems very rare to generate a single random u128 or any smaller value; instead, we usually need to generate an array of such values. So I suggest we avoid implementing those or at least consider them out of scope of this feature.

[briansmith]

I support adding such an API. My goal for advocating for such functionality is to eliminate all uses of unsafe from typical uses of common consumers of getrandom. Accordingly, I did some more research to advance this.

Consider this basic function, which I think is the one most agreeable to the most people:

#[inline(always)]
pub fn getrandm_array<const N: usize>() -> Result<[u8; N], Error> {
    let mut uninit: MaybeUninit<[u8; N]> = MaybeUninit::uninit();
    // TODO: `uninit.as_bytes_mut()` when that is available.
    {
        // SAFETY: MaybeUninit<u8> is always valid, even for padding bytes
        let as_bytes_mut = unsafe {
            core::slice::from_raw_parts_mut(uninit.as_mut_ptr() as *mut MaybeUninit<u8>, N)
        };
        getrandom::getrandom_uninit(as_bytes_mut)?;
    }
    // SAFETY: `dest` has been fully initialized by `imp::getrandom_inner`
    // since it returned `Ok`.
    Ok(unsafe { uninit.assume_init() })
}

I found that such a function was useful for applications that operate on byte arrays, e.g. HMAC keys, AES keys, TLS ClientHello/ServerHello random values, etc.

However, for other applications, I found the function way too limiting because Rust doesn't have an API equivalent to impl<T, const M: usize, const N: usize> impl From<[T; M * N] for [[T; M]; N]; i.e. there's no analog to [T]::array_chunks() for arrays. Further, it is error-prone to implement such conversions especially when we consider the possibility of M * N overflowing isize::MAX. Consequently, I think a secondary function is needed:

#[inline(always)]
pub fn getrandm_arrays<const M: usize, const N: usize>() -> Result<[[u8; N]; M], Error> {
    let mut uninit: MaybeUninit<[[u8; N]; M]> = MaybeUninit::uninit();
    // TODO: `uninit.as_bytes_mut()` when that is available.
    {
        // SAFETY: MaybeUninit<u8> is always valid, even for padding bytes
        let as_bytes_mut = unsafe {
            core::slice::from_raw_parts_mut(uninit.as_mut_ptr() as *mut MaybeUninit<u8>, 
            core::mem::size_of::<[[u8; N]; M]>()
            )
        };
        getrandom::getrandom_uninit(as_bytes_mut)?;
    }
    // SAFETY: `dest` has been fully initialized by `imp::getrandom_inner`
    // since it returned `Ok`.
    Ok(unsafe { uninit.assume_init() })
}

The the above two functions, I can efficiently implement everything I need, in a pretty type-safe way, without using unsafe, with minimal inconvenience, e.g. let words = getrandm_arrays()?.map(|bytes| Wrapping(u64::from_ne_bytes(bytes)));.

Notice in particular that the compiler should verify that M * N is within bounds, without us needing to do anything.

I've also considered doing more elaborate support for types that are "plain old data" or more generally which are safe to transmute from an array of arbitrary bytes of the same length:

pub unsafe trait Pod {}
unsafe impl Pod for u64 {}
unsafe impl Pod for Wrapping<u64> {}
unsafe impl Pod for u8 {}
unsafe impl<T: Pod, const N: usize> Pod for [T; N] {}
/// ...

#[inline(always)]
pub fn getrandm_array_pod<T: Pod, const N: usize>() -> Result<[T; N], Error> {
    let mut uninit: MaybeUninit<[T; N]> = MaybeUninit::uninit();
    // TODO: `uninit.as_bytes_mut()` when that is available.
    {
        // SAFETY: MaybeUninit<u8> is always valid, even for padding bytes
        let as_bytes_mut = unsafe {
            core::slice::from_raw_parts_mut(uninit.as_mut_ptr() as *mut MaybeUninit<u8>, 
            core::mem::size_of::<[T; N]>()
            )
        };
        getrandom::getrandom_uninit(as_bytes_mut)?;
    }
    // SAFETY: `dest` has been fully initialized by `imp::getrandom_inner`
    // since it returned `Ok`.
    Ok(unsafe { uninit.assume_init() })
}

However, I found that at least Nightly Rustc didn't optimize uses of such a function any better than the equivalent that uses getrandm_arrays()?.map(...) to get the same effect without needing to introduce a Pod type abstraction. In fact, the currently Nightly Rust optimized this fancier version worse. Godbolt link.

Therefore, I propose we implement (only) getrandom_array and getrandom_arrays as definied above. I am definitely open to better names.

As far as MSRV goes, I persoally would prefer to set the MSRV to 1.51. However, if that's too drastic, I suggest we add a rust_1_51 feature or similar that guards this and any other feature that may require later versions of Rust. I think this would be easy enough for us to maintain.

[briansmith]

Also, I found that getrandom_array and getrandom_arrays must be inlined to produce the code we expect, which is why I suggest #[always(inline)].

[briansmith]

// SAFETY: MaybeUninit<u8> is always valid, even for padding bytes

I copied those comments from as_bytes_mut() but they don't make as much sense in this context. Also, those comments should emphasize "The Rust compiler is responsible for enforcing N <= isize::MAX (and M <= isize::MAX) when it typechecks since this function uses N (and M) as the size of a slice in its signature."

[briansmith]

Draft PR #293 builds on PR #291 to add a single getrandom_array function that subsumes all of the variants gerandom_array, getrandom_arrays, and getrandom_arrays_pod I mentioned above. It does use a generic Pod-like type, but only to allow arbitrary levels of byte array nesting, e.g. [[[u8; 4]; 4]; 16].

[briansmith]

Now I've reconsidered my comments above. I think that having generic functions like this in getrandom is better avoided. Almost all uses of this can be use something like zerocopy::FromBytes to get similar effects, with just a slight amount of clumsiness. So I think we should just close this as "Not Planned."

[josephlr]

@briansmith sounds good! And I agree, future integration or compatibility with zerocopy seems like the best bet.