Message-ID: <20200805193824.GA17981@1wt.eu>
Date:   Wed, 5 Aug 2020 21:38:24 +0200
From:   Willy Tarreau <w@....eu>
To:     Marc Plumb <lkml.mplumb@...il.com>
Cc:     tytso@....edu, netdev@...r.kernel.org, aksecurity@...il.com,
        torvalds@...ux-foundation.org, edumazet@...gle.com,
        Jason@...c4.com, luto@...nel.org, keescook@...omium.org,
        tglx@...utronix.de, peterz@...radead.org, stable@...r.kernel.org
Subject: Re: Flaw in "random32: update the net random state on interrupt and
 activity"

Hi Mark,

On Wed, Aug 05, 2020 at 09:06:40AM -0700, Marc Plumb wrote:
> Just because you or I don't have a working exploit doesn't mean that someone
> else isn't more clever.

I agree on the principle, but it can be said from many things, including
our respective inability to factor large numbers for example. But for
sure we do need to be careful, and actually picking only some limited
parts of the fast pool (which are only used to update the input pool
and are only made of low-difficulty stuff like instruction pointers,
jiffies and TSC values) is probably not going to disclose an extremely
well guarded secret.

> The fundamental question is: Why is this attack on net_rand_state problem?
> It's Working as Designed. Why is it a major enough problem to risk harming
> cryptographically important functions?

It's not *that* major an issue (in my personal opinion) but the current
net_rand_state is easy enough to guess so that an observer may reduce
the difficulty to build certain attacks (using known source ports for
example). The goal of this change (and the one in update_process_times())
is to disturb the net_rand_state a little bit so that external observations
turn from "this must be that" to "this may be this or maybe that", which
is sufficient to limit the ability to reliably guess a state and reduce
the cost of an attack.

Another approach involving the replacement of the algorithm was considered
but we were working with -stable in mind, trying to limit the backporting
difficulty (and it revealed a circular dependency nightmare that had been
sleeping there for years), and making the changes easier to check (which
is precisely what you're doing).

> Do you remember how you resisted making dev/urandom fast for large reads for
> a long time to punish stupid uses of the interface? In this case anyone who
> is using net_rand_state assuming it is a CPRNG should stop doing that. Don't
> enable stupidity in the kernel.
> 
> This whole thing is making the fundamental mistake of all amateur
> cryptographers of trying to create your own cryptographic primitive. You're
> trying to invent a secure stream cipher. Either don't try to make
> net_rand_state secure, or use a known secure primitive.

We're not trying to invent any stream cipher or whatever, just making
use of a few bits that are never exposed alone as-is to internal nor
external states, to slightly disturb another state that otherwise only
changes once a minute so that there's no more a 100% chance of guessing
a 16-bit port after seeing a few packets. I mean, I'm pretty sure that
even stealing three or four bits only from there would be quite enough
to defeat the attack given that Amit only recovers a few bits per packet.

For me the right longterm solution will be to replace the easily guessable
LFSR. But given the build breakage we got by just adding one include, I
can only guess what we'll see when trying to do more in this area :-/

Regards,
Willy

Powered by blists - more mailing lists