#21 Improve rng init portability

[Julien Vermillard]

Today this code can become quite messy when porting to something else than contiki/linux:

dtls_context_t
dtls_new_context(void app_data) {
dtls_context_t *c;
dtls_tick_t now;

ifndef WITH_CONTIKI

FILE *urandom = fopen("/dev/urandom", "r");
unsigned char buf[sizeof(unsigned long)];

endif / WITH_CONTIKI /

dtls_ticks(&now);

ifdef WITH_CONTIKI

/ FIXME: need something better to init PRNG here /
dtls_prng_init(now);

else / WITH_CONTIKI /

if (!urandom) {
dtls_emerg("cannot initialize PRNG\n");
return NULL;
}

if (fread(buf, 1, sizeof(buf), urandom) != sizeof(buf)) {
dtls_emerg("cannot initialize PRNG\n");
return NULL;
}

fclose(urandom);
dtls_prng_init((unsigned long)*buf);

endif / WITH_CONTIKI /

IMO dtls_new_context(void *app_data) should take an extra argument "unsigned long seed" and leave to the user the responsibility to provide a random seed.

No more #ifdef :)

[Julien Vermillard]

proposed fix:
https://github.com/jvermillard/tinydtls/commit/17e481d9e361fb272e39daec4b83bc9c0c6be4ba

[Olaf Bergmann]

I agree that the code in dtls_new_context() is everything but optimal (several issues of which #ifdef seems to be one of the most harmless). I also intend to agree that the PRNG might better be seeded from the application code rather than the library.

Said this, I do not think that adding a parameter to dtls_new_context() is the right approach, especially because dtls_prng_init() is a no-op on some platforms (see L79-L81 in prng.h). In that case, passing a seed parameter to dtls_new_context() could lead to erroneous and hence dangerous assumptions about the security level that can be achieved by this.

If you still think that the code should be removed from dtls_new_context() (as said, I see very good reasons for this apart from not wanting to dig through pre-processor conditionals), I prefer making the call to dtls_prng_init() explicit.

[Julien Vermillard]

hi olaf, thanks for the details, I'll dig a little more in the prng code

[Julien Vermillard]

I think all this platform specific code should not be part of the lib.
Maybe the user should provide a callback or a macro for providing random numbers?

[Olaf Bergmann]

I also thought about a callback initially but came to the conclusion that this is too cumbersome to use. A macro is exactly the intention of prng.h.

[Julien Vermillard]

but it's in the lib? I'm confused, I'm supposed to add a bunch of #ifdef for each platform I'm going to port it?
I'm working on two (a Golang binding and a mbed one).
For example for the golang binding if I want it to run on Windows I need to remove the fopen("/dev/urandom") code.

IMO it would be easier if all this platform specfic code/macro are moved to the client/server examples, or we will see more and more fork (I think the RIOT OS guys already have one).

[Olaf Bergmann]

The code you are referring to is the PRNG seeding. This is indeed something that should be done from outside. But the actual use of the (platform-specific) PRNG should be done in the lib. This includes passing the seed value to the PRNG.