[0/4] random: change usage of arch_get_random_long()

Message

Sandy Harris Feb. 10, 2022, 2:28 p.m. UTC

This series of patches is not strictly necessary, but it is a
significant improvement.

The current code has a sequence in several places that calls one or
more of arch_get_random_long() or related functions, checks the return
value(s) and on failure falls back to random_get_entropy(). These
patches provide get_source_long(), which is intended to replace all
such sequences.

This is better in several ways. It never wastes effort by calling
arch_get_random_long() et al. when the relevant config variables are
not set. If config variables for a hardware rng or the latent entropy
plugin are set, then it uses those instead. It does not deliver raw
output from any of these sources, but masks it by mixing with stored
random data. In the fallback case it gives much more random output

In the cases where a good source is available, this adds a little
overhead, but not much. It also saves some by not trying
arch_get-random_long() unnecessarily.

If no better source is available, get_source_long() falls back to
get_xtea_long(), an internal-use-only pseudorandom generator based on
the xtea block cipher. In general, that is considerably more expensive
than random_get_entropy(), but also provably much stronger.

With no good source, there is still a problem at boot; xtea cannot
become secure until it is properly keyed. It does become safe
eventually, and in the meanwhile it is certainly no worse than
random_get_entropy().

Comments

Greg KH Feb. 10, 2022, 2:57 p.m. UTC | #1

On Thu, Feb 10, 2022 at 10:28:26PM +0800, Sandy Harris wrote:
> This series of patches is not strictly necessary, but it is a
> significant improvement.
> 
> The current code has a sequence in several places that calls one or
> more of arch_get_random_long() or related functions, checks the return
> value(s) and on failure falls back to random_get_entropy(). These
> patches provide get_source_long(), which is intended to replace all
> such sequences.
> 
> This is better in several ways. It never wastes effort by calling
> arch_get_random_long() et al. when the relevant config variables are
> not set. If config variables for a hardware rng or the latent entropy
> plugin are set, then it uses those instead. It does not deliver raw
> output from any of these sources, but masks it by mixing with stored
> random data. In the fallback case it gives much more random output
> 
> In the cases where a good source is available, this adds a little
> overhead, but not much. It also saves some by not trying
> arch_get-random_long() unnecessarily.
> 
> If no better source is available, get_source_long() falls back to
> get_xtea_long(), an internal-use-only pseudorandom generator based on
> the xtea block cipher. In general, that is considerably more expensive
> than random_get_entropy(), but also provably much stronger.
> 
> With no good source, there is still a problem at boot; xtea cannot
> become secure until it is properly keyed. It does become safe
> eventually, and in the meanwhile it is certainly no worse than
> random_get_entropy().

This patch series is not actually threaded at all, and is totally
whitespace damaged.

How did you send it?  Try using 'git send-email' next time to correctly
thread them and not corrupt them so that they are unusable :(

thanks,

greg k-h

Jason A. Donenfeld Feb. 10, 2022, 3:10 p.m. UTC | #2

Hard NACK on this patchset.

Issues:
- The superficial ones that Greg's mailbot is now emailing you about:
improper threading, missing sign off, bad whitespace, poor coding
style, etc
- We don't need XTEA for this; there are probably better choices these days.
- Distinction between "get_hw_long()" and arch_get_random_long()
doesn't exist, since get_random_bytes_arch() just calls
arch_get_random_long().
- Even if we did want this, replacing arch_get_random_seed_long(), a
function meant to supply a *fresh* value from a hardware source, with
something more deterministic changes the intention.
- Likewise the cycle counter is supposed to be at least a little bit
entropic, some of the time, maybe, in the best of circumstances,
perhaps... whereas expanding an XTEA key that's seeded from that same
cycle counter (in the absence of other things) at one point in time
seems clearly worse.
- Probably others, but after 10 minutes of reading this it seemed like
it wasn't worth it to go further.