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

[Hauke Mehrtens]

Hi Torsten,

Sorry for the late answer, I forgot about this mail thread.

On 1/30/23 10:57, Torsten Duwe wrote:
> 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.

ustreamss uses the randomness to generate session keys (including 
ephemeral keys), IVs and padding. The long term keys are generated in a 
different application.

> 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.

crng_ready() should only return false at bootup before the system got 
enough random bits, afterwards it never returns false. Even without 
GRND_INSECURE it will never run out of entropy.

I think we should wait with creating TLS sessions till we have enough 
random data to do it securely. I tested this on a lantiq xrx200 (MIPS) 
device and it was initialized much before the LAN interface was up.

The code in ustream-mbedtls.c was probably initially written when 
/dev/random was still blocking when too much entropy was read out of the 
pool.

I will rename the function.

Hauke