Re: BPF RNG

[Theodore Ts'o]

On Tue, Apr 16, 2019 at 08:55:59AM -0500, Kees Cook wrote:
> [correcting Alexei's email address and resending...]
> On Tue, Apr 16, 2019 at 8:54 AM Kees Cook <keescook@xxxxxxxxxx> wrote:
> >
> > In looking at prandom_u32() users, I noticed that BPF uses its own
> > state variable with bpf_user_rnd_u32(). It appears that this state is
> > never reseeded like regular prandom_u32(). (See __prandom_timer().) Is
> > this intentional, or should reseeding be happening?

It looks like this isn't the only place where prandom's machinery is
being used w/o reseeding.  See also net/netfilter/nft_meta.c and
net/netfilter/nft_numgen.c.

That being said, guarantees we give for prandom are extremely weak.
The only reason why we reseed for net_rand_state was they wanted
something sorta-kinda secure, but they weren't willing to pay the cost
of calling get_random_u64() or get_random_u32().

So I don't think we should be encouraging people to think that
reseeding should actually be happening, at least with some explicit
explanation of documentation somewhere about why it's needed, and what
security guarantees people should be expecting.  (e.g., for the
networking subsystem, the claim is that it's trying to make certain
DOS attackers harder, and it shouldn't be mistaken for real security).

In fact, why we're supporting multiple prandom per-cpu state families
makes me worried we're going in the wrong direction.  I suspect the
reason why people are settng up separate prandom states is because
they don't want to leak knowledge about the prandom internal state to
an external attacker.  That should only matter for a small set of
network stack users; so maybe we should just have two prandom states.
One for the vast majority of users where there should be no security
concerns around using prandom (because if there are, 99% of the time
they should be get_random_u32), and then for those people who want
get_random_fast_but_maybe_insecure_u64() --- and maybe for those
architectures where there is some instruction like RDRAND which is
really super-fast, it would use RDRAND for the
get_random_fast_but_maybe_insecure_u64().

						- Ted