Subject: bug#41456: fix cases where insecure randomness could be used

From: Pádraig Brady <[email-masked]>
Date: Fri, 22 May 2020 15:17:29 +0100

On 22/05/2020 08:19, Taylor Hornby wrote:
> I reported a potential security bug on GitHub:
> https://github.com/coreutils/coreutils/pull/32. To save you a click, I'll
> copy-paste it here (for context this is on a PR with a fix):
> 
> Comment #1:
> 
> Apologies for submitting on GitHub, it's so much more convenient. I will
> understand if no one sees this because I didn't follow the guidelines.
> 
> Justification:
> 
>     - The existing code is dangerous because it can silently fail to seed
>     the random number generator securely, either when fopen() fails or when
>     read() returns fewer bytes than requested, which can happen if the call
>     is interrupted by an interrupt. This is important for utilities like
>     shred where cryptographic-quality randomness is important.
>     - I removed the bytes_bound stuff because it didn't seem necessary
>     anywhere it was used, and if get_nonce is ever called with bytes_bound <
>     bufsize, then part of ISAAC's initial state will contain
>     timestamps/PIDs, so it will not be uniformly random. Usually, stream
>     ciphers like ISAAC require their initial state to be uniformly random,
>     otherwise there will be statistical biases in the early output.
> 
> I have not tested all the utilities this affects.
> 
> (Full disclosure is appropriate in this case because any damage has already
> been done, fixing the problem in secret would not stop any attacks, but
> disclosing might encourage users to stop using the dangerous code and
> upgrade.)
> 
> Comment #2:
> 
> This is a more serious issue on Solars, which apparently has a blocking
> /dev/random <https://icmconference.org/wp-content/uploads/G11b-Fenwick.pdf>
> and NAME_OF_NONCE_DEVICE defaults to /dev/random (see gc-random.m4), or
> when NAME_OF_NONCE_DEVICE is overriden to /dev/random with a configure flag
> on a Linux system.
> 
> I ran some experiments on a Debian 9 box, and read() from /dev/random
> frequently returns very few bytes, sometimes as few as just 6 bytes. This
> means, ironically, if someone built the code with /dev/random thinking it
> would be more secure, it's actually less secure, because read() will return
> fewer bytes and then very little of the ISAAC seed will be random and most
> of it will be timestamp/PID/uninitialized memory.

Testing on a Solaris 10 box indicates that /dev/random doesn't give short reads.
All other systems default to /dev/urandom.
coreutils doesn't need cryptographic randomness, so the read from
/dev/urandom should be seen as optional, and present to improve randomness when available.
I'm not sure your concerns are justified in the coreutils context.

cheers,
Pádraig