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

[Hauke Mehrtens]

On 1/29/23 15:13, Torsten Duwe wrote:
> On Sat, 28 Jan 2023 19:41:13 +0100
> Hauke Mehrtens <hauke at hauke-m.de> wrote:
> 
>> Instead of keeping a file descriptor open just use the getrandom syscall
>> to get random data. This is supported by the musl, glibc and Linux for
>> some time now.
>>
>> This also improves the error handling in case this function returns not
>> as many bytes as expected.
>>
>> Signed-off-by: Hauke Mehrtens <hauke at hauke-m.de>
>> ---
>>   ustream-mbedtls.c | 23 +++++------------------
>>   1 file changed, 5 insertions(+), 18 deletions(-)
>>
>> diff --git a/ustream-mbedtls.c b/ustream-mbedtls.c
>> index e79e37b..51ba2fa 100644
>> --- a/ustream-mbedtls.c
>> +++ b/ustream-mbedtls.c
>> @@ -17,6 +17,7 @@
>>    */
>>   
>>   #include <sys/types.h>
>> +#include <sys/random.h>
>>   #include <fcntl.h>
>>   #include <unistd.h>
>>   #include <stdlib.h>
>> @@ -25,8 +26,6 @@
>>   #include "ustream-ssl.h"
>>   #include "ustream-internal.h"
>>   
>> -static int urandom_fd = -1;
>> -
>>   static int s_ustream_read(void *ctx, unsigned char *buf, size_t len)
>>   {
>>   	struct ustream *s = ctx;
>> @@ -66,21 +65,12 @@ __hidden void ustream_set_io(struct ustream_ssl_ctx *ctx, void *ssl, struct ustr
>>   	mbedtls_ssl_set_bio(ssl, conn, s_ustream_write, s_ustream_read, NULL);
>>   }
>>   
>> -static bool urandom_init(void)
>> -{
>> -	if (urandom_fd > -1)
>> -		return true;
>> -
>> -	urandom_fd = open("/dev/urandom", O_RDONLY);
>> -	if (urandom_fd < 0)
>> -		return false;
>> -
>> -	return true;
>> -}
>> -
>>   static int _urandom(void *ctx, unsigned char *out, size_t len)
>>   {
>> -	if (read(urandom_fd, out, len) < 0)
>> +	ssize_t ret;
>> +
>> +	ret = getrandom(out, len, 0);
>> +	if (ret < 0 || (size_t)ret != len)
>>   		return MBEDTLS_ERR_ENTROPY_SOURCE_FAILED;
> [...]
> 
> 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.

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/

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.

Hauke