randombit
commented
7 years ago Sounds good to me. Re performance, it should not be so much an issue because currently botan_rng_init is always called with NULL param, which means it defaults to reading /dev/urandom, and Botan uses a single persistent handle to the system RNG among all such RNG objects. However if rnp wanted to use a DRBG, the seeding overhead would be quite high due to the repeated initializations so there sharing a single object would be a big win.
Seems better to have the RNG be part of the rnp_t structure, since a global would cause problems for applications that use rnp from multiple threads.
Another possible advantage to using a DRBG instead of the system PRNG - it is possible then to derive private keys deterministicly from a passphrase (by using the passphrase as the DRBG seed, and running the key generation algo). This can be useful for brainwallet-type scenarios where a user wants to be able to recreate a private key later on without storing it anywhere. Actually at one point I tried to hack this functionality into gpg but the code did not make it easy to do.
ronaldtse
kriskwiatkowski
Description
RNP performs call to
botan_rng_initfunction in many places of the code in order to initialize source of random numbers. This isn't really needed - it impacts code clarity, causes memory leaks when RNG object is not destroyed correctly and may impact performance.I propose
rnpinitializes RNG only once and makes it available via some sort of handle - global, part of arnp_tstructure orpgp_randomfunction (solution must be thread safe).If rnp uses some sort of DRBG (like HMAC_DRBG) then read from device like
/dev/urandomcan be limited to one and done only when DRBG is seeded (also reseeding must be taken into account).