Expose API interface for MaybeUninit<u8> slice

[bdbai]

It is inefficient to zero-fill a large byte buffer before actually being initialized with random bytes. Thus, it would be better if getrandom exposes a function that takes a slice of MaybeUninit<u8> so that the user will not have to initialize the buffer.

[dhardy]

I've seen this type of request a couple of times before, with the rand project, but have never been convinced that this optimisation would be worth the effort. This is even more true of getrandom where a slow system call is required to get the random bytes.

[briansmith]

If a crate is depending on getrandom() and it wants to expose an interface where it fills a MaybeUninit<u8> slice then it would be good to be able to fill the part of that slice that needs the random bytes using getrandom directly, instead of needing to fill a temporary &[u8] and then copying that temporary &[u8] to the &[MaybeUninit<u8>].

[briansmith]

In particular, for security reasons, one may wish to ensure (to the extent Rust allows) that the generated random bytes are never copied anywhere except the target buffer.

[josephlr]

It is inefficient to zero-fill a large byte buffer before actually being initialized with random bytes.

@bdbai do you have any benchmarks on this? Do we know if there's a performance difference between:

• Zeroing a buffer then calling getrandom
• Calling getrandom on uninitialized bytes

It would be fine to do a comparison in C if getting benchmarks in Rust might be overly complex.

instead of needing to fill a temporary &[u8] and then copying that temporary &[u8] to the &[MaybeUninit<u8>].

In particular, for security reasons, one may wish to ensure (to the extent Rust allows) that the generated random bytes are never copied anywhere except the target buffer.

@briansmith I'm more sensitive to the security concern here (we definitely want it to be possible to initialize random buffers in-place). However, why wouldn't it be possible to just zero the &mut [MaybeUninit<u8>], get a &mut [u8] to the same buffer, and then pass the buffer to getrandom? That should avoid copying secret data or needing a temporary buffer.

[josephlr]

In general, I wouldn't be totally opposed to adding such an API, but we would probably want to wait for rust-lang/rfcs#2930 to be implemented (tracked in rust-lang/rust#78485), before adding an API.

I don't want to invent our own API for dealing with uninitialized buffers. If we add this, we would want to use the ReadBuf type and something like:

pub fn getrandom_readbuf(dest: &mut ReadBuf<'_>) -> Result<(), Error> {
    // Our implementation dealing with [MaybeUninit<u8>]
}

Initially, this would need to be behind an unstable feature gate (like "read_buf") until the RFC is stabilized.

One advantage of this sort of API is that it might allow for partial reads from the underlying RNG to not be wasted. However, this is a very slight benefit as if the RNG succeeds once, it virtually always succeeds for all future calls.

[gilescope]

Hopefully we could see readbuf hit nightly soon.

[briansmith]

I don't want to invent our own API for dealing with uninitialized buffers. If we add this, we would want to use the ReadBuf type and something like:

ReadBuf is planned to be in std::io so it can't be used by getrandom in general, right? Perhaps we should provide feedback that it should be moved to libcore and avoid using std::io::Error.

However, I think ReadBuf is more than what is needed, because it tries to keep track of the intermediate state of a partially-filled/initialized buffer, which isn't applicable here.

[briansmith]

@briansmith I'm more sensitive to the security concern here (we definitely want it to be possible to initialize random buffers in-place). However, why wouldn't it be possible to just zero the &mut [MaybeUninit<u8>], get a &mut [u8] to the same buffer, and then pass the buffer to getrandom? That should avoid copying secret data or needing a temporary buffer.

Of course that does work. OTOH, if that's going to be a common pattern then why not encapsulate it within getrandom by adding such an API? And then we could (later) optimize the implementation to avoid the initial zeroing.

[josephlr]

ReadBuf is planned to be in std::io so it can't be used by getrandom in general, right? Perhaps we should provide feedback that it should be moved to libcore and avoid using std::io::Error.

getrandom has a "std" feature, so we could just gate getrandom_readbuf behind that feature. Looking at the API for ReadBuf, it seems that nothing in its API requires it to be in libstd, so it could be in libcore. I would encourage people to raise this point in the ReadBuf tracking issue.

OTOH, if that's going to be a common pattern then why not encapsulate it within getrandom by adding such an API?

It's mostly due a desire to keep the functionality of this crate minimal to reduce our maintenance burden, so I would actually want to know that this is a common pattern across many crates before we commit to supporting it in the long-term. However, if adding this API increases ergonomics for folks, I would support adding it. This would be true even if the only benefit is ergonomics (e.g. if there is no significant performance difference).

However, I think ReadBuf is more than what is needed, because it tries to keep track of the intermediate state of a partially-filled/initialized buffer, which isn't applicable here.

If the main reason for adding this change is ergonomics (i.e. handling common patterns), I would want getrandom to integrate well with those common patterns. It looks like ReadBuf is going to be "standard way" to express this pattern in Rust, so we would probably want to use that rather than inventing our own API.

[briansmith]

The latest revision of the PR #279 implements this.

Regarding the ReadBuf idea, I think that should be a separate proposal with its own issue. It looks like ReadBuf is dead, based on rust-lang/rust#97015, and it looks like it might be a while before all that stuff is resolved. And, it isn't clear the resolution will be no_std-compatible. A lot of things need/want a no_std-compatible interface and a std::io::*-based one is a non-starter.

[briansmith]

@bdbai This issue shouldn't be closed yet, since PR #279 hasn't been approved/merged yet, and might not be.