Visa Hankala <v...@hankala.org> wrote:
> On Thu, Jul 28, 2022 at 11:00:12AM +1000, Damien Miller wrote:
> > + rs->rs_count = REKEY_BASE;
> > + /* rekey interval should not be predictable */
> > + _rs_random_u32(&rekey_fuzz);
> > + rs->rs_count += rekey_fuzz % REKEY_BASE;
>
> The randomization looks good.
>
> However, might it cause a problem (in the future) that _rs_random_u32()
> calls _rs_stir_if_needed()? rs_count has a largish value so a recursive
> invocation of _rs_stir() should not happen, but anyway.
The question is what _rs_random_u32() will do when it calls
_rs_stir_if_needed().
There is one potential problem. lib/libcrypto/arc4random/*.h contains portable
wrappers for _rs_forkdetect(), which actually do things. memset(rs, 0,
sizeof(*rs))
will trash the rs state. Let's imagine a "fork" has happened same time that
bytes
run out.
_rs_stir()
...
rs->rs_count = REKEY_BASE;
_rs_random_u32 -> _rs_stir_if_needed -> _rs_forkdetect
- all rs fields are zero'd with memset
- _rs_forkdetect returns
back in _rs_stir_if_needed,
- if (!rs || rs->rs_count <= len)
_rs_stir();
So it will recurse once (only once, because a 2nd fork cannot happen).
But this is fragile.
Alternatives are to get the value direct from getentropy -- with KEYSZ + IVSZ +
4
maybe? Or fetch a value for this random bias early and track it in rs, but
don't
damage it in the memset? Or split _rs_random_u32() so that a sub-function of it
may collect these 4 keystream bytes without the _rs_stir_if_needed/_rs_rekey
checks?
