Add getrandom::value function based on zerocopy::FromBytes

[newpavlov]

This PR is an alternative to #293 and closes #281.

It introduces function with the folllowing signature:

#[cfg(feature = "zerocopy")]
#[inline]
pub fn value<T: zerocopy::FromBytes + Sized>() -> Result<T, Error> { ... }

It allows to generate a random value of any type which implements the FromBytes trait, including nested arrays.

The main disadvantage is an additional dependency, but since it's optional and rand started to unconditionally pull zerocopy starting from the rand_core level, I think it's a reasonable cost. Enabling the zerocopy feature bumps MSRV to 1.60. Notably, zerocopy has a conservative MSRV policy, so it shouldn't cause any MSRV breakage in future.

cc @briansmith @joshlf

[josephlr]

I really like this. It seems low-cost, and I think we could add it in a non-breaking release of getrandom (as it's an optional feature).

[joshlf]

Sounds cool! Out of curiosity, do you have preferences on how to handle different versions of zerocopy? Ie, if/when we release a 0.8, will you wait for a breaking version of getrandom to upgrade your dependency, will you provide multiple versions of the feature (ie, zerocopy_0_7, zerocopy_0_8, etc), or something else? We're currently working to better support use cases like these (google/zerocopy#619), so knowing what your preferences are will help our planning.

[newpavlov]

No, I did not plan to. Updating to a breaking release of zerocopy would need a breaking release of getrandom.

If you think that zerocopy will have breaking releases more frequently than getrandom, then it may be better to reverse things a bit and add an optional getrandom support to zerocopy, not vice versa.

[joshlf]

We have short-term plans to release two breaking changes (0.8 and 0.9), but then things should be stable for the foreseeable future after that. Maybe it'd make sense to just wait for 0.9 and then land this? Both should be released on the order of a few weeks to a few months.

[briansmith]

Personally, I do not think it is worth adding the dependency. First, the vast majority of uses should be in seeding a userspace PRNG, where I believe the current APIs are good enough, even without this or even #293, because doing "construct an all-zeros array and then overwrite it" has more than good enough performance characteristics for that. And, in general, "construct an all-zeros array and then overwrite it" is almost always going to be good enough because the syscall overhead is going to dwarf the zeroing, and in the extremely rare cases where that isn't true, we have the _uninit version now.

You can see that between the time you submitted this PR and now, everything that depended on zerocopy had to deal with google/zerocopy#716 due to their security monitoring (dependeabot, etc.) going off, and because it was yanked. I think that's likely to happen again. It would be very frustrating to me to be affected by it.

If the user wants to use zerocopy then it is easy for them to do so, since zerocopy does (or should) make it easy.

[newpavlov]

If the user wants to use zerocopy then it is easy for them to do so, since zerocopy does (or should) make it easy.

The goal is to eliminate the need for users to use unsafe and slightly improve ergonomics. As you correctly note, the main use-case of getrandom is generation of seeds. let seed: u64 = getrandom::value(); is short and good at conveying the intent compared to the small dance around zeroized arrays and from_ne_bytes.

Your concerns about zerocopy itself are valid, but note that I do not plan to make the zerocopy feature enabled by default (even if forget about the MSRV mismatch), so users would need to explicitly opt-in for it.

As I mentioned earlier, a potential alternative to this PR would be to add this functionality to the zerocopy crate.

[briansmith]

The goal is to eliminate the need for users to use unsafe and slightly improve ergonomics. As you correctly note, the main use-case of getrandom is generation of seeds. let seed: u64 = getrandom::value(); is short and good at conveying the intent compared to the small dance around zeroized arrays and from_ne_bytes.

As a user, I would prefer the "small dance" over adding a dependency to the dependency tree.

I still think that PR #293 had the best compromise to get to a "no unsafe" usage. The usage would be (as documented in that PR) like let random_u32s: [u32; 4] = getrandom::array()?.map(u32::from_ne_bytes); which is concise and clear and efficient. Since seeds for a CSPRNG are relatively small even if some copying would be involved, it would have virtually no performance impact.

In contrast, I feel like the usability improvement between this PR and PR #293 is very small which makes taking on a dependency not worthwhile.

[newpavlov]

I am going to close this PR in favor of the zerocopy PR which hopefully will land, if not, we may revive the getrandom::array PR.