dev_random: initial import of a high-level hwnrg API #5153

miri64 · 2016-03-23T15:23:56Z

A shot at fixing #5150 (name up to discussion).

This modules requires the board to define a randomness source DEV_RANDOM_SRC in its board.h (DEV_RANDOM_SRC_PERIPH_HWRNG, i.e. periph_hwrng is the default). Since this module is purely macro-based it is easy to override for other DEV_RANDOM_SRC cases (e.g. the functionality in #4989). I added the ADC as another example.

Depending on #4989.

kYc0o · 2016-07-22T14:54:00Z

This new feature requires more discussion, as other related PRs about RNG. Postponed.

miri64 · 2016-09-01T17:14:03Z

I'd like to have some input. Especially e.g. from @kaspar030.

miri64 · 2016-09-01T18:22:26Z

Added @mtausig's #4989 as a dependency.

kaspar030 · 2016-09-01T18:43:07Z

Why did you choose to use function macros?

miri64 · 2016-09-01T18:52:42Z

To be perfectly honest... I'm not sure anymore :-/. I guess to make absolutely clear that these just wrap around some kind of hardware access. But I'm fine with changing it to inline functions, if that is better suited.

miri64 · 2016-09-01T19:38:02Z

Moved to a inline function based API.

miri64 · 2016-09-01T19:54:47Z

Added config for hwnrg boards, too. For some reason I did not find them earlier, when I did the at86rf2xx boards. I need to research the behavior of adc_init() before I include the rest of the boards that havn't a source already.

miri64 · 2016-09-02T09:09:46Z

Rebased and squashed.

miri64 · 2016-09-02T09:10:12Z

(#4989 was merged)

miri64 · 2016-09-26T13:58:23Z

Prepared for review tomorrow ;-)

mtausig · 2016-09-26T14:01:10Z

Will there be some Makefile flag, to make a project depend onto the availability of a random source? I mean somethin like we have now in FEATURE_REQUIRED = periph_hwrng.

miri64 · 2016-10-04T16:33:31Z

Will there be some Makefile flag, to make a project depend onto the availability of a random source? I mean somethin like we have now in FEATURE_REQUIRED = periph_hwrng.

@mtausig the randomness source would be chosen by the board's configuration as I already prepared it for the boards it applies for. We can make this configurable, but I don't see the use case right now.

mtausig · 2016-10-04T16:40:43Z

On Die, 2016-10-04 at 09:33 -0700, Martine Lenders wrote:

Will there be some Makefile flag, to make a project depend onto the
availability of a random source? I mean somethin like we have now in
FEATURE_REQUIRED = periph_hwrng.
@mtausig the randomness source would be chosen by the board's configuration as
I already prepared it for the boards it applies for. We can make this
configurable, but I don't see the use case right now.

You have misunderstood me. I do not want to make the random source configurable,
I would just like to know if it is there respectively require that it is there
upon building my application.
Just the same thing that happens now, if I add FEATURE_REQUIRED = periph_hwrng
to the Makefile.

miri64 · 2016-10-04T18:18:27Z

You have misunderstood me. I do not want to make the random source configurable,
I would just like to know if it is there respectively require that it is there
upon building my application.
Just the same thing that happens now, if I add FEATURE_REQUIRED = periph_hwrng
to the Makefile.

I both misunderstood and was confused about the context, because I actually thought we talked about periph_hwnrg ^^". Yes, having something like that might be a good idea (though the board normally defaults to "/dev/zero" in case really no randomness source is available)!

mtausig · 2016-10-05T07:49:51Z

(though the board normally defaults to "/dev/zero" in case really no randomness source is available)!

I've seen that. But for security relevant code, I'd rather not use random numbers at all, before using /dev/zero as an entropy source.

miri64 · 2016-10-17T12:54:25Z

Fixes #2264

d00616 · 2016-10-17T18:59:11Z

sys/include/dev_random.h

+    hwrng_read(buf, (unsigned int)num);
+#elif DEV_RANDOM_SRC == DEV_RANDOM_SRC_PERIPH_ADC
+    for (unsigned i = 0; i < num * 8; i++) {
+        bf_set(buf, adc_sample(DEV_RANDOM_ADC, ADC_RES_16BIT) & 0x1);


Have you tested this code? In my understanding it sets the second bit of the first byte of the buffer. More than one byte isn't written to buffer and when my buffer content is 0xff i get back 0xff.

Please test your results http://www.fdk.com/cyber-e/pdf/HM-RAE103.pdf there is a command "rngtest" available in many Linux distribution.

I have tested reading a floating pin on a nRF52 and nRF51. On a Floating nRF52 pin the FIPS test was successful. On a nRF51 i have a 100% fail of FIPS 140-2. For inspiration take a look at https://github.com/infomaniac50/Random/blob/master/Random.cpp

Instead of bf_set I think its better use XOR here. So it doesn't matter what value are initially stored. You can do multiple ADC reads for a single bit to compensate bad randomness.

No I wasn't able to test, yet. I don't have the hardware for that :-/

d00616 · 2016-10-17T19:03:15Z

sys/include/dev_random.h

+#elif   DEV_RANDOM_SRC == DEV_RANDOM_SRC_AT86RF2XX
+    at86rf2xx_get_random(dev_random_at86rf2xx, buf, num);
+#else
+    memset(buf, 0, num);


This is very dangerous. What is, when someone using this random function without reviewing all code line by line on a platform without HWRNG and ADC for AES key generation in a commercial product?

Whats about letting compilation fail?

Yes, I wanted to change this.

d00616 · 2016-10-17T19:43:32Z

When using ADC as source, this code needs a check if the data is random. Otherwise if someone has access to a Board he can force generating 0 as random data by connecting the RNG pin to GND. This can used to easily attack cryptographic operations.

kaspar030 · 2016-10-17T19:46:24Z

@miri64 I think we need to get back to the drawing board. This PR's solution leaves some important things open.

d00616 · 2016-10-17T20:00:18Z

I think getting random data should use more then one source when no HWRNG is available or it is not trust-able.
One source can ADC with correction but this source can manipulated.
Another source can HAVEGE: HArdware Volatile Entropy Gathering and Expansion. There are some implementations. https://tls.mbed.org/api/havege_8h.html
There are other sources like timing of things or interrupts, radio....

miri64 · 2016-10-18T07:24:55Z

@miri64 I think we need to get back to the drawing board. This PR's solution leaves some important things open.

Well, at least we are getting there... ;-)

miri64 · 2016-10-19T16:07:29Z

I won't find time to fix the issues in this PR for the release.

miri64 · 2016-11-29T21:34:46Z

Improved on ADC implementation and also removed NULL randomness source. Problem now is that the build will fail, if there is no device supplied.

miri64 · 2017-01-13T17:16:01Z

Okay, after thinking about it (and watching a lot of talks about randomness at 33C3), I agree with @kaspar030: back to the drawing board.

miri64 added Type: new feature The issue requests / The PR implemements a new feature for RIOT Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Mar 23, 2016

miri64 assigned kaspar030 Mar 23, 2016

miri64 added this to the Release 2016.07 milestone Mar 23, 2016

miri64 mentioned this pull request Mar 23, 2016

prng: how to seed the random number generator? #5150

Open

cgundogan mentioned this pull request Apr 14, 2016

sys/auto_init: cpuid as seed for random_init #5321

Closed

kYc0o modified the milestones: Release 2016.10, Release 2016.07 Jul 22, 2016

miri64 force-pushed the dev_random/api/initial branch 2 times, most recently from 7e26747 to 983a3d2 Compare September 1, 2016 18:20

miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 1, 2016

miri64 force-pushed the dev_random/api/initial branch 3 times, most recently from 22e1ab5 to fcb7e85 Compare September 1, 2016 18:41

miri64 force-pushed the dev_random/api/initial branch from 47b924f to ed98b73 Compare September 2, 2016 09:09

miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 2, 2016

miri64 and others added 3 commits September 26, 2016 15:58

dev_random: initial import of a high-level hwnrg API

afdce88

auto_init: initialize PRNG using dev_random

ba618b7

boards: provide config for supporting boards

92e2c50

miri64 force-pushed the dev_random/api/initial branch from ed98b73 to 92e2c50 Compare September 26, 2016 13:58

miri64 added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Sep 26, 2016

miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 27, 2016

miri64 mentioned this pull request Oct 17, 2016

Random number generator for platforms without hardware RNG #2264

Closed

miri64 mentioned this pull request Oct 17, 2016

Replace pkg/micro-ecc random calls #2407

Closed

d00616 reviewed Oct 17, 2016

View reviewed changes

miri64 modified the milestones: Release 2016.10, Release 2017.01 Oct 19, 2016

fixup! dev_random: initial import of a high-level hwnrg API

49dcd64

miri64 closed this Jan 13, 2017

miri64 deleted the dev_random/api/initial branch January 17, 2017 08:49

miri64 mentioned this pull request Jun 22, 2020

sys/entropy_source: add new module for entropy sources #14324

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev_random: initial import of a high-level hwnrg API #5153

dev_random: initial import of a high-level hwnrg API #5153

miri64 commented Mar 23, 2016 •

edited

Loading

kYc0o commented Jul 22, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 1, 2016

kaspar030 commented Sep 1, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 2, 2016

miri64 commented Sep 2, 2016

miri64 commented Sep 26, 2016

mtausig commented Sep 26, 2016

miri64 commented Oct 4, 2016

mtausig commented Oct 4, 2016

miri64 commented Oct 4, 2016

mtausig commented Oct 5, 2016

miri64 commented Oct 17, 2016

d00616 Oct 17, 2016

d00616 Oct 17, 2016

miri64 Oct 18, 2016

miri64 Nov 29, 2016

d00616 Oct 17, 2016

miri64 Oct 18, 2016

d00616 commented Oct 17, 2016

kaspar030 commented Oct 17, 2016

d00616 commented Oct 17, 2016

miri64 commented Oct 18, 2016

miri64 commented Oct 19, 2016

miri64 commented Nov 29, 2016

miri64 commented Jan 13, 2017

dev_random: initial import of a high-level hwnrg API #5153

dev_random: initial import of a high-level hwnrg API #5153

Conversation

miri64 commented Mar 23, 2016 • edited Loading

kYc0o commented Jul 22, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 1, 2016

kaspar030 commented Sep 1, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 1, 2016

miri64 commented Sep 2, 2016

miri64 commented Sep 2, 2016

miri64 commented Sep 26, 2016

mtausig commented Sep 26, 2016

miri64 commented Oct 4, 2016

mtausig commented Oct 4, 2016

miri64 commented Oct 4, 2016

mtausig commented Oct 5, 2016

miri64 commented Oct 17, 2016

d00616 Oct 17, 2016

Choose a reason for hiding this comment

d00616 Oct 17, 2016

Choose a reason for hiding this comment

miri64 Oct 18, 2016

Choose a reason for hiding this comment

miri64 Nov 29, 2016

Choose a reason for hiding this comment

d00616 Oct 17, 2016

Choose a reason for hiding this comment

miri64 Oct 18, 2016

Choose a reason for hiding this comment

d00616 commented Oct 17, 2016

kaspar030 commented Oct 17, 2016

d00616 commented Oct 17, 2016

miri64 commented Oct 18, 2016

miri64 commented Oct 19, 2016

miri64 commented Nov 29, 2016

miri64 commented Jan 13, 2017

miri64 commented Mar 23, 2016 •

edited

Loading