Hi,
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?
-Kees
--
Kees Cook
[correcting Alexei's email address and resending...]
On Tue, Apr 16, 2019 at 8:54 AM Kees Cook <[email protected]> wrote:
>
> Hi,
>
> 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?
>
> -Kees
>
> --
> Kees Cook
--
Kees Cook
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 <[email protected]> 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
On 04/16/2019 03:55 PM, Kees Cook wrote:
> [correcting Alexei's email address and resending...]
>
> On Tue, Apr 16, 2019 at 8:54 AM Kees Cook <[email protected]> wrote:
>>
>> Hi,
>>
>> 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
Correct, we split that off from the main one.
>> never reseeded like regular prandom_u32(). (See __prandom_timer().) Is
>> this intentional, or should reseeding be happening?
The prng is not to be used in any security-critical context. That said,
we optionally could perform reseeding from time to time, though that
should probably be done generically, so that all "external"
prandom_u32_state() users could register themselves upon init whether
they opt-in for periodic reseeding or not.
Thanks,
Daniel
On Tue, Apr 16, 2019 at 10:13 AM Daniel Borkmann <[email protected]> wrote:
>
> On 04/16/2019 03:55 PM, Kees Cook wrote:
> > [correcting Alexei's email address and resending...]
> >
> > On Tue, Apr 16, 2019 at 8:54 AM Kees Cook <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> 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
>
> Correct, we split that off from the main one.
>
> >> never reseeded like regular prandom_u32(). (See __prandom_timer().) Is
> >> this intentional, or should reseeding be happening?
>
> The prng is not to be used in any security-critical context. That said,
> we optionally could perform reseeding from time to time, though that
> should probably be done generically, so that all "external"
> prandom_u32_state() users could register themselves upon init whether
> they opt-in for periodic reseeding or not.
Yeah, it seemed like it should be built into the API. I'll add this to
the TODO list. ;)
Thanks for double-checking.
--
Kees Cook