sys/random: default to musl LCG instead of TinyMT #17188

benpicco · 2021-11-11T19:07:59Z

Contribution description

In A Guideline on Pseudorandom Number Generation (PRNG) in the IoT the paper concludes with

The Knuth LCG is the most efficient general purpose generator that provides decent statistical quality.
It is simple and lean enough to run on very constrained devices.

So let's select prng_musl_lcg to be the default PRNG instead of prng_tinymt32.

Testing procedure

This saves a good chunk of memory on e.g. samr21-xpro:

prng_tinymt32

   text	   data	    bss	    dec	    hex	filename
  26452	    136	   2824	  29412	   72e4	tests/rng/bin/samr21-xpro/tests_rng.elf

prng_musl_lcg

   text	   data	    bss	    dec	    hex	filename
  26208	    136	   2808	  29152	   71e0	tests/rng/bin/samr21-xpro/tests_rng.elf

Issues/PRs references

In [0] the paper concludes with > The Knuth LCG is the most efficient general purpose generator that > provides decent statistical quality. > It is simple and lean enough to run on very constrained devices. So let's select `prng_musl_lcg` to be the default PRNG instead of `prng_tinymt32`. This gives a good chunk of memory on e.g. `samr21-xpro`: prng_tinymt32 ------------- text data bss dec hex filename 26452 136 2824 29412 72e4 tests/rng/bin/samr21-xpro/tests_rng.elf prng_musl_lcg ------------- text data bss dec hex filename 26208 136 2808 29152 71e0 tests/rng/bin/samr21-xpro/tests_rng.elf [0] https://sci-hub.se/10.1145/3453159

kaspar030 · 2021-11-12T08:24:18Z

@PeterKietzmann do you remember why we didn't do this after you published the paper?

PeterKietzmann · 2021-11-12T15:14:35Z

@PeterKietzmann do you remember why we didn't do this after you published the paper?

No particular reason. I want to restructure the random subsystem and started from the bottom #14324, currently waiting for #15671. Once I made it to the PRNG level, the plan was to set the default generators.

I'm fine with changing now.

benpicco · 2021-11-12T15:17:15Z

Looks like all applications that already have been converted to Kconfig and use random select

CONFIG_MODULE_PRNG_TINYMT32=y

Is this really needed? Can't we have a default PRNG with Kconfig?

leandrolanzieri · 2021-11-12T16:36:07Z

Looks like all applications that already have been converted to Kconfig and use random select
CONFIG_MODULE_PRNG_TINYMT32=y
Is this really needed? Can't we have a default PRNG with Kconfig?

There are defaults, in fact you are modifying one here. The problem is that the makefile dependency resolution fails to default to use HWRNG (as you pointed in #16732), which Kconfig prioritizes. We decided to override this in application configuration and leave it 'well modelled'.

leandrolanzieri · 2021-11-12T17:11:08Z

Looks like all applications that already have been converted to Kconfig and use random select
CONFIG_MODULE_PRNG_TINYMT32=y
Is this really needed? Can't we have a default PRNG with Kconfig?

BTW you'll also need to adapt those app.config.test to match the new Makefile default:

CONFIG_MODULE_PRNG_MUSL_LCG=y

benpicco · 2021-11-12T17:46:51Z

Hm so maybe we should remove the HWRNG default with Kconfig to make it even with the make based dependency system?

PeterKietzmann · 2021-11-15T08:18:40Z

Hm so maybe we should remove the HWRNG default with Kconfig to make it even with the make based dependency system?

+1. Besides harmonizing both systems, HWRNG isn't always the best choice for direct random number generation.

leandrolanzieri · 2021-11-15T08:20:54Z

Hm so maybe we should remove the HWRNG default with Kconfig to make it even with the make based dependency system?

+1. Besides harmonizing both systems, HWRNG isn't always the best choice for direct random number generation.

Ok, if you think that's the best default sure. I would not do it just for the fact of matching the makefile default though.

benpicco · 2021-11-15T11:45:36Z

The random dependency resolution is such a mess unfortunately. The reason why I went with HWRNG by default was because that was the only way to get different randomness across reboots (important for CoAP as we otherwise always start with the same sequence number and the server discards our requests).

It's also much slimmer than TinyMT, but with LCG that is not so relevant anymore.

Ideally we'd default to HWRNG to seed the PRNG, if no HWRNG is available use PUF SRAM to at least get some random seed.
But I don't think this is possible with the current make based dependency system.

Kconfig of course will allow us to model this behavior properly.

PeterKietzmann · 2021-11-16T08:42:09Z

Seeding needs rework. #15671 is the next step towards seed generation and separation.

PeterKietzmann · 2021-11-16T08:53:28Z

tests/pkg_c25519/app.config.test

@@ -3,5 +3,5 @@ CONFIG_MODULE_RANDOM=y
 # Should be autoselecting the MODULE_PRNG_HWRNG if possible
 # Since the makefile cannot we have to override until end of migration
 # Remove when TEST_KCONFIG is complete
-CONFIG_MODULE_PRNG_TINYMT32=y
+CONFIG_MODULE_PRNG_MUSL_LCG=y


Neither Tinymt32, nor LCG are crypto RNGs. One step further, HWRNGs are not necessarily secure for direct use. Hence, we should default to a CSPRNG in crypto contexts. SHA256PRNG would be the leanest choice.

This shows the problem with our current random module: There is only a single (non-secure) default RNG. As said, I am planning to continue my work on that starting from #15671.

What I meant: crypto packages (cifra, hacl, dsa, ...) that require access to a random number generator should configure a crypto RNG (SHA256PRNG)

I agree, but let's do this as a follow-up. This PR only changes the default RNG, those apps are still using the default RNG - if they should not, that would be a different PR.

What if we say "set correct default choice"? Anyway, we can also go with two PRs.

PeterKietzmann · 2021-11-24T08:44:22Z

Here?

PeterKietzmann · 2021-11-24T08:50:48Z

tests/rng/tests/01-run.py

-    child.expect("1918982324")
-    child.expect("1550167154")
-    child.expect("3454972398")
-    child.expect("1034066532")


How about moving this to a separate test case?

PeterKietzmann · 2021-11-29T08:49:15Z

tests/rng make test passes with native and nrf52840dk (selects Musl LCG).
checked some tests/pkg_* (cifra, libcose, c25519,monocipher) with native or nrf52040dk, all passand use Musl LCG.
menuconfig shows the correct defaults (tests/pkg_c25519 in the screenshot

)

@benpicco did you create the new test values of took them from somewhere? In the latter case, is there more?

benpicco · 2021-11-29T09:48:38Z

The new test values are just the output of the new RNG

PeterKietzmann · 2021-11-29T12:14:01Z

So lets fix this occurce, merge, and fix the default choice for crypto packages afterwards

benpicco requested review from waehlisch, tcschmidt and PeterKietzmann November 11, 2021 19:07

benpicco requested review from cgundogan, jia200x and leandrolanzieri as code owners November 11, 2021 19:08

github-actions bot added Area: build system Area: Build system Area: Kconfig Area: Kconfig integration Area: sys Area: System labels Nov 11, 2021

benpicco requested review from kaspar030 and maribu November 11, 2021 19:08

maribu approved these changes Nov 11, 2021

View reviewed changes

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 11, 2021

benpicco requested review from aabadie, fjmolinas, MichelRottleuthner and smlng as code owners November 12, 2021 17:45

github-actions bot added the Area: tests Area: tests and testing framework label Nov 12, 2021

maribu mentioned this pull request Nov 15, 2021

tests/pthread_barrier: fix test script #17196

Merged

PeterKietzmann reviewed Nov 16, 2021

View reviewed changes

fjmolinas added this to the Release 2022.01 milestone Nov 18, 2021

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 18, 2021

benpicco force-pushed the sys/random-musl_lcg-default branch from e1a606a to 8a80b93 Compare November 18, 2021 21:16

benpicco requested a review from miri64 as a code owner November 18, 2021 21:16

PeterKietzmann reviewed Nov 24, 2021

View reviewed changes

PeterKietzmann added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Nov 29, 2021

tests: default to musl LCG

87e0032

benpicco force-pushed the sys/random-musl_lcg-default branch from 8a80b93 to 87e0032 Compare November 29, 2021 12:46

benpicco enabled auto-merge November 29, 2021 13:26

kaspar030 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 30, 2021

leandrolanzieri mentioned this pull request Nov 30, 2021

cpu/esp{32,8266}: model kconfig #17232

Merged

kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 30, 2021

benpicco merged commit 2079642 into RIOT-OS:master Nov 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys/random: default to musl LCG instead of TinyMT #17188

sys/random: default to musl LCG instead of TinyMT #17188

benpicco commented Nov 11, 2021 •

edited

kaspar030 commented Nov 12, 2021

PeterKietzmann commented Nov 12, 2021 •

edited

benpicco commented Nov 12, 2021

leandrolanzieri commented Nov 12, 2021

leandrolanzieri commented Nov 12, 2021

benpicco commented Nov 12, 2021

PeterKietzmann commented Nov 15, 2021

leandrolanzieri commented Nov 15, 2021

benpicco commented Nov 15, 2021

PeterKietzmann commented Nov 16, 2021

PeterKietzmann Nov 16, 2021

PeterKietzmann Nov 22, 2021

benpicco Nov 23, 2021

PeterKietzmann Nov 24, 2021

PeterKietzmann commented Nov 24, 2021

PeterKietzmann Nov 24, 2021

PeterKietzmann commented Nov 29, 2021

benpicco commented Nov 29, 2021

PeterKietzmann commented Nov 29, 2021

sys/random: default to musl LCG instead of TinyMT #17188

sys/random: default to musl LCG instead of TinyMT #17188

Conversation

benpicco commented Nov 11, 2021 • edited

Contribution description

Testing procedure

prng_tinymt32

prng_musl_lcg

Issues/PRs references

kaspar030 commented Nov 12, 2021

PeterKietzmann commented Nov 12, 2021 • edited

benpicco commented Nov 12, 2021

leandrolanzieri commented Nov 12, 2021

leandrolanzieri commented Nov 12, 2021

benpicco commented Nov 12, 2021

PeterKietzmann commented Nov 15, 2021

leandrolanzieri commented Nov 15, 2021

benpicco commented Nov 15, 2021

PeterKietzmann commented Nov 16, 2021

PeterKietzmann Nov 16, 2021

Choose a reason for hiding this comment

PeterKietzmann Nov 22, 2021

Choose a reason for hiding this comment

benpicco Nov 23, 2021

Choose a reason for hiding this comment

PeterKietzmann Nov 24, 2021

Choose a reason for hiding this comment

PeterKietzmann commented Nov 24, 2021

PeterKietzmann Nov 24, 2021

Choose a reason for hiding this comment

PeterKietzmann commented Nov 29, 2021

benpicco commented Nov 29, 2021

PeterKietzmann commented Nov 29, 2021

benpicco commented Nov 11, 2021 •

edited

PeterKietzmann commented Nov 12, 2021 •

edited