netdev - Re: Flaw in "random32: update the net random state on interrupt and activity"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 8 Aug 2020 11:19:01 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Willy Tarreau <w@....eu>
Cc:     George Spelvin <lkml@....org>, Netdev <netdev@...r.kernel.org>,
        Amit Klein <aksecurity@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Andrew Lutomirski <luto@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        "Theodore Ts'o" <tytso@....edu>,
        Marc Plumb <lkml.mplumb@...il.com>,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: Flaw in "random32: update the net random state on interrupt and activity"

On Sat, Aug 8, 2020 at 10:45 AM Willy Tarreau <w@....eu> wrote:
>
>
>     WIP: random32: use siphash with a counter for prandom_u32

siphash is good.

But no:

> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1277,7 +1277,6 @@ void add_interrupt_randomness(int irq, int irq_flags)
>
>         fast_mix(fast_pool);
>         add_interrupt_bench(cycles);
> -       this_cpu_add(net_rand_state.s1, fast_pool->pool[cycles & 3]);
>
>         if (unlikely(crng_init == 0)) {
>                 if ((fast_pool->count >= 64) &&
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 026ac01af9da..c9d839c2b179 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1743,13 +1743,6 @@ void update_process_times(int user_tick)
>         scheduler_tick();
>         if (IS_ENABLED(CONFIG_POSIX_TIMERS))
>                 run_posix_cpu_timers();
> -
> -       /* The current CPU might make use of net randoms without receiving IRQs
> -        * to renew them often enough. Let's update the net_rand_state from a
> -        * non-constant value that's not affine to the number of calls to make
> -        * sure it's updated when there's some activity (we don't care in idle).
> -        */
> -       this_cpu_add(net_rand_state.s1, rol32(jiffies, 24) + user_tick);
>  }

We're not going back to "don't add noise, just make a stronger
analyzable function".

I simply won't take it. See my previous email. I'm 100% fed up with
security people screwing up real security by trying to make things
overly analyzable.

If siphash is a good enough hash to make the pseudo-random state hard
to guess, then it's also a good enough hash to hide the small part of
the fast-pool we mix in.

And while security researchers may hate it because it's hard to
analyze, that's the POINT, dammit.

This "it's analyzable" makes it repeatable. And since mixing in
serious things is too slow, and people thus delay reseeding for too
long, then we reseed with random crap that doesn't hurt and that isn't
realistically analyzable.

              Linus