You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We could reexport (some of) the traits from rand_core:
👍 Widely used traits, seems mostly stable.
👍 Maintained by Rust team itself.
👎 External dependency.
👎 rand_core is not stable yet so it would hold back embedded-hal 1.0.
❓ Too much complexity?
❓ Reexport everything or only a subset?
❓ Remove embedded_hal::rng?
We encourage direct dependency on rand_core for HAL/HWRNG impls like we do with e.g. embedded-dma:
👍 Flexibility for HAL impls to choose rand_core version.
👎 Fragmentation in rand_core versions across ecosystem.
❓ Too much complexity for HAL/HWRNG impls?
❓ Remove embedded_hal::rng?
Keep embedded_hal::rng as-is:
👍 No dependencies.
👎 Separate RNG trait to what the Rust community is already using detriments interoperability.
👍 Much simpler than rand_core traits.
❓ Too simple?
I am unaware about how many implementations of embedded_hal::rng::Read there are. As of 0.2.x it is marked as unproven.
Does somebody know if it is being used?
I think I like option 2 the most for now, including removing embedded_hal::rng if there are no users. Then in a future release maybe do option 1.
Opinions from hardware RNG implementers would be helpful here.
hmm, all interesting options! it's definitely neat to have consistent rand everywhere. i am also perhaps a little cautious about the weird breakages we might see trying to match libraries with different rand_core versions (like, e-h versions is already a constraint, e-h and rand versions with the hal is going to make things even trickier, and it looks like they don't semver trick the rand-core crate...
in related things there's also getrandom which is great for, making rand just work but not so good for binding in peripherals, and i ran into a bunch of problems actually using the rng that ended in rand-facade, it'd be neat to discuss / document how to effectively use rngs as part of any improvements.
No strong opinions; I did also implement this unproven trait in our LPC55 HAL, just to not reinvent the wheel. Practically, all I need is a method to fill byte buffers with entropy, with the ability to ignore errors.
Might it be possible to nudge rand_core towards a v1? I do fear a bit of a mess and tangle for what's essentially filling a byte buffer. Personally, despite "doing cryptography" I've avoided the rand ecosystem so far.
In my opinion though RngCore makes the mistake of not making up its mind whether fill_bytes/try_fill_bytes can fail or not (the documentation's example of "reading a file of entropy" seems both contrived and misguided). By definition, a CSPRNG is supposed to transform a good entropic seed (the goodness of which is untestable) into random bits "forever", whereas a hardware TRNG typically can perform a self-test on startup (so the fallibility would be in the constructor - which if anything would be a separate trait NewRng) and then doesn't really have a reason to suddenly start failing. I understand that the RngCore trait wants to "offer both", but what's a driver supposed to use or rely on?
I'd tend to prefer an infallible method for these reasons, but certainly someone is going to come up with some valid corner case on embedded Linux or something 😅.
In any case, if the consensus is for a fallible method, then #229 seems applicable, I don't think NonZeroU32 without additional interpretation is a particularly useful error type.
cc @tarcieri as it would be great if the eventual trait integrated nicely with RustCrypto.
Outside the embedded space, rand_core::OsRng provides an infallible wrapper for the fallible getrandom crate. That approach seems to work well enough.
We've adopted rand_core pretty ubiquitously in the https://github.com/RustCrypto crates. I also wish it were 1.0 and it's been something of a complication in other 1.0 crates (e.g. curve25519-dalek), but we've managed to live with that so far.
In yesterdays meeting we mostly hovered around going option 2 with the possible path to go option 1 in the future if things get too wild.
Personally I think if there's a perfectly fine universal solution like rand_core it doesn't really make sense to re-export it unless we manage to create a good chunk of value-add on top of it. It certainly makes sense to nudge the maintainers into stabilizing it for good to get rid of the moving target.
This is basically waiting for someone to PR the removal of embedded_hal::rng traits and adding some documentation/pointers on how to use rand_core instead.
The std feature of rand_core is activated by all sorts of commonly used dev-dependencies (e.g. criterion, proptest), and until features=dev_dep (rust-lang/cargo#7916) is stable it's a giant headache.
Activity
eldruin commentedon Sep 16, 2020
Good question.
Here the alternatives:
rand_core
:rand_core
is not stable yet so it would hold backembedded-hal
1.0.embedded_hal::rng
?rand_core
for HAL/HWRNG impls like we do with e.g.embedded-dma
:rand_core
version.rand_core
versions across ecosystem.embedded_hal::rng
?embedded_hal::rng
as-is:rand_core
traits.I am unaware about how many implementations of
embedded_hal::rng::Read
there are. As of0.2.x
it is marked asunproven
.Does somebody know if it is being used?
I think I like option 2 the most for now, including removing
embedded_hal::rng
if there are no users. Then in a future release maybe do option 1.Opinions from hardware RNG implementers would be helpful here.
therealprof commentedon Sep 16, 2020
Funny enough I had the exact same trait definition and implementation in my
nrf51-hal
which seems to have gotten dropped by the consolidation of all nRF HALs. This one is implementingrand_core
now: https://github.com/nrf-rs/nrf-hal/blob/master/nrf-hal-common/src/rng.rsRng
API #38eldruin commentedon Sep 16, 2020
stm32l4xx-hal does implement
embedded_hal::rng::Read
. See herecc @nickray
ryankurte commentedon Sep 17, 2020
hmm, all interesting options! it's definitely neat to have consistent rand everywhere. i am also perhaps a little cautious about the weird breakages we might see trying to match libraries with different
rand_core
versions (like,e-h
versions is already a constraint,e-h
andrand
versions with thehal
is going to make things even trickier, and it looks like they don't semver trick the rand-core crate...in related things there's also
getrandom
which is great for, making rand just work but not so good for binding in peripherals, and i ran into a bunch of problems actually using the rng that ended in rand-facade, it'd be neat to discuss / document how to effectively use rngs as part of any improvements.nickray commentedon Sep 18, 2020
No strong opinions; I did also implement this unproven trait in our LPC55 HAL, just to not reinvent the wheel. Practically, all I need is a method to fill byte buffers with entropy, with the ability to ignore errors.
Might it be possible to nudge
rand_core
towards a v1? I do fear a bit of a mess and tangle for what's essentially filling a byte buffer. Personally, despite "doing cryptography" I've avoided therand
ecosystem so far.In my opinion though
RngCore
makes the mistake of not making up its mind whetherfill_bytes
/try_fill_bytes
can fail or not (the documentation's example of "reading a file of entropy" seems both contrived and misguided). By definition, a CSPRNG is supposed to transform a good entropic seed (the goodness of which is untestable) into random bits "forever", whereas a hardware TRNG typically can perform a self-test on startup (so the fallibility would be in the constructor - which if anything would be a separate traitNewRng
) and then doesn't really have a reason to suddenly start failing. I understand that theRngCore
trait wants to "offer both", but what's a driver supposed to use or rely on?I'd tend to prefer an infallible method for these reasons, but certainly someone is going to come up with some valid corner case on embedded Linux or something 😅.
In any case, if the consensus is for a fallible method, then #229 seems applicable, I don't think
NonZeroU32
without additional interpretation is a particularly useful error type.cc @tarcieri as it would be great if the eventual trait integrated nicely with RustCrypto.
tarcieri commentedon Sep 21, 2020
Outside the embedded space,
rand_core::OsRng
provides an infallible wrapper for the falliblegetrandom
crate. That approach seems to work well enough.We've adopted
rand_core
pretty ubiquitously in the https://github.com/RustCrypto crates. I also wish it were 1.0 and it's been something of a complication in other 1.0 crates (e.g.curve25519-dalek
), but we've managed to live with that so far.therealprof commentedon Sep 23, 2020
In yesterdays meeting we mostly hovered around going option 2 with the possible path to go option 1 in the future if things get too wild.
Personally I think if there's a perfectly fine universal solution like
rand_core
it doesn't really make sense to re-export it unless we manage to create a good chunk of value-add on top of it. It certainly makes sense to nudge the maintainers into stabilizing it for good to get rid of the moving target.therealprof commentedon Oct 6, 2020
This is basically waiting for someone to PR the removal of
embedded_hal::rng
traits and adding some documentation/pointers on how to userand_core
instead.tarcieri commentedon Oct 6, 2020
I think you'll be a lot better off not re-exporting it, as it's one of the most common triggers of this bug:
rust-lang/cargo#7916
The
std
feature ofrand_core
is activated by all sorts of commonly useddev-dependencies
(e.g.criterion
,proptest
), and untilfeatures=dev_dep
(rust-lang/cargo#7916) is stable it's a giant headache.11 remaining items