Example applications without rand dependency#521
Example applications without rand dependency#521finalyards wants to merge 1 commit intoembassy-rs:mainfrom
rand dependency#521Conversation
04b1458 to
fb672ae
Compare
|
/test |
|
I'd like to hear some feedback on this PR. Is it looking okay? The real thing is what it says on the title: whether TrouBLE apps (with security) are expected to depend on Some news from elsewhere:
I wonder...
...shouldn't those two tests be passing? Can I do anything about it? Is there any reason not to merge this PR? |
|
My 2c: I like the idea of removing |
Thanks for the comment. Yes,
Note how it takes 256 bits. I tried to be clever about that, but it's also completely fine to keep the current API.
In some future, I would like to advance to using
|
|
I just worry about the increased boilerplate code in user space: - let _trng_source = TrngSource::new(peripherals.RNG, peripherals.ADC1);
- let mut trng = Trng::try_new().unwrap(); // Ok when there's a TrngSource accessible
+ let seed: TrulyRandomBits = {
+ let mut buf: [u8; 32];
+ let _ = TrngSource::new(peripherals.RNG, peripherals.ADC1);
+ Trng::try_new().unwrap().read(&mut buf);
+ TrulyRandomBits(buf)
+ };What about just using the raw |
This comment was marked as outdated.
This comment was marked as outdated.
fb672ae to
bb19eb8
Compare
|
Changed the underlying implementation to use The host library itself uses ’getrandom’ for getting the seed for its encryption. This means simplification in the examples / apps side:
The point of the PR was to remove ’rand’ and ’rand_core’ dependencies from the examples/apps side, so that making the rand 0.6 -> 0.10 change (separate PR) becomes easier. I feel it also simplifies the application usage. NOTE: Would someone please run ’_sec’ examples on nRF52. I have access to ESP32-C3, -C6 and nRF54 but not to that board - and nRF54 examples don’t contain ’_sec’ examples. This would check that the getrandom hack works. Asks:
|
This comment was marked as resolved.
This comment was marked as resolved.
f26edaa to
4f5ecaf
Compare
|
I've rebased the PR on latest Since keeping up to date with an evolving I have edited the CI script accordingly. For open questions, please see |
|
I'm sorry, but removing rand/rand_core in favor of this getrandom gets a thumbs down from me:
|
I'll go through these, one at a time: 1.
|

Please consider this PR mostly as a cleanup / reorganization suggestion.
What it does
It removes the
randandrand_coredependencies fully from theexamples/side of the repo. The code used to create various random number generators that where then used - only - for seeding the actual RNG withintrouble-host. This one-time effort feels clutter to the author of the PR - more importantly it created complexity in maintaining the #474 PR, which aims at bringing inrand0.10 once it is released.What's the beef?
We do want actual, hardware provided and platform specific, true random values, to get things going. Unfortunately, within the embedded Rust domain there is no standard for providing such (hardware dependent) values. I believe after the PR, the code reflects this better. There is no need to carry
CryptoRngetc. traits around - with regard to nrf52 the old code actually used an initial RNG to generate aChaCha18(which is "crypto" grade), though that one was only used once, to seed in turn the real RNG within the library. I fail to see the point of that - it's likely just an accumulation of design choices and adhering to the compiler's requirements (it wants aCryptoRng; how can I provide one?). This is the kind of things the new "just give a seed" approach imho helps steer clear of.Does it really simplify things?
It drops dependencies (
rand,rand_core). It feels like it reduces the line count, but actually it's slightly longer:This is because of repetitive sections that create the seeds. [Edit: I can take those common things into a
src/lib.rsbut this breaks the nice approach that everything for an application is in a single file.] My gut feel is that the overall system becomes easier to reason about - and to maintain - due to randomness being locked fully withintrouble-host, as an implementation detail.Is there more?
Not really. The PR contains 2..3 places of unrelated (but useful) changes; I'm happy if I may bring them in at once.
If this gets approved, it makes the work on the real
rand-update PR way easier...