[PATCH ustream-ssl] ustream-mbedtls: Use getrandom() instead of /dev/urandom

[Torsten Duwe]

Hi Hauke!

On Sun, 29 Jan 2023 17:08:38 +0100
Hauke Mehrtens <hauke at hauke-m.de> wrote:

> > drivers/char/random.c lines 1240- ...
> >   * Reading from /dev/urandom has the same functionality as calling
> >   * getrandom(2) with flags=GRND_INSECURE. Because it does not block
> >   * waiting for the RNG to be ready, it should not be used.
> > 
> > Haven't audited mbedtls, but I assume it reads urandom for "lesser"
> > entropy when needed. In any case, getrandom(out, len, GRND_INSECURE)
> > would be the proper replacement.
> > 
> > 	Torsten
> 
> Hi Torsten,
> 
> The mapage says this:
>  > By default, getrandom() draws entropy from the urandom source
>  > (i.e., the same source as the /dev/urandom device).  This
>  > behavior can be changed via the flags argument.
> https://man7.org/linux/man-pages/man2/getrandom.2.html
>
> GRND_INSECURE is also not documented in the man page.

Well, it exists in the kernel source and headers...
 
> The option was added to the Linux kernel 5.6 here:
> https://git.kernel.org/linus/75551dbf112c992bc6c99a972990b3f272247e23
> 
> The documentation says
>  > GRND_INSECURE	Return non-cryptographic random bytes
> We want to use the random bytes in mbedtls for cryptographic
> operations. I think giving no flags is the correct option here.
> 
> I think the behavior of /dev/random changed some years ago. This
> article described it a bit:  https://lwn.net/Articles/808575/

That's only the internal workings.
BTW, the mentioned quote of Andy Lutomirski applies here in some sense.
You read away the true entropy and might even block on it when pseudo-
randomness might suffice, see below.

> As far as I understood the code, giving no flags will guarantee that
> the random pool is initialized (crng_ready() returns true) and
> otherwise it is the same as using GRND_INSECURE. As we use it for
> cryptographic operations I think we should give no flags.

"cryptographic operations" is a wide area. Some randomness is required,
to varying degrees, for long-term keys, session keys, IVs and padding.

Especially for long living keys, each end every bit should be totally
unpredictable, which should correspond to read an appropriate amount
from /dev/random. IVs and padding can be generated from a pseudo-RNG
whose initial state is "uncertain enough", usually /dev/urandom.

GIT is cool.
c6e9d6f388947986 2014-Jul-17 tytso: introduce getrandom(2) system call
75551dbf112c992b 2019-Dec-23 luto: add GRND_INSECURE ...
48446f198f9adcb4 2019-Dec-23 luto: ignore GRND_RANDOM

The first commit also has a man page in the comment, which is probably
what was recorded. The second one changes the no-flags behaviour, away
from the man page text you quoted.

Someone once mentioned on LKML that drivers/char/random.c needs better
maintenance... ;)

I had a quick look at mbedtls and its usage of the provided rng
function. It generates not only padding and IVs, but also session keys.
Especially on OpenWRT these are a trade-off IMHO. OpenWRT supports a
lot of hardware that is low on entropy at boot (MIPS anyone?) Do you
want to block early ssh sessions, maybe even for minutes, or would you
rather risk eavesdropping on those early connections?

Depending on your choice for ustream, you can keep the proposed code,
but please rename the functions to "random", not "urandom". Or you want
to stick with the current urandom behaviour but then please add Luto's
GRND_INSECURE flag to achieve that.

HTH,
	Torsten