Re: [PATCH 0/7] Add arc4random support

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Thu, Apr 14, 2022 at 1:39 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 14/04/2022 04:36, Yann Droneaud wrote:
> > Hi,
> >
> > Le 13/04/2022 à 22:23, Adhemerval Zanella via Libc-alpha a écrit :
> >
> >> This patch adds the arc4random, arc4random_buf, and arc4random_uniform
> >> along with optimized versions for x86_64, aarch64, and powerpc64.
> >>
> >> The generic implementation is based on scalar Chacha20, with a global
> >> cache and locking.  It uses getrandom or /dev/urandom as fallback to
> >> get the initial entropy, and reseeds the internal state on every 16MB
> >> of consumed entropy.
> >>
> >> It maintains an internal buffer which consumes at maximum one page on
> >> most systems (assuming 4k pages).  The internal buffer optimizes the
> >> cipher encrypt calls, by amortize arc4random calls (where both
> >> function call and locks cost are the dominating factor).
> >>
> >> Fork detection is done by checking if MADV_WIPEONFORK supported.  If not
> >> the fork callback will reset the state on the fork call.  It does not
> >> handle direct clone calls, nor vfork or _Fork (arc4random is not
> >> async-signal-safe due the internal lock usage, althought the
> >> implementation does try to handle fork cases).
> >>
> >> The generic ChaCha20 implementation is based on the RFC8439 [1], which
> >> a simple memcpy with xor implementation.
> >
> > The xor (with 0) is a waste of CPU cycles as the ChaCha20 keystream is the PRNG output.
>
> I don't have a strong feeling about, although it seems that any other
> ChaCha20 implementation I have checked does it (libgcrypt, Linux,
> BSD).  The BSD also does it for arc4random, although most if not
> all come from OpenBSD and they are usually paranoid with security
> hardening.
>
> I am no security expert, so I will keep it as is for generic interface
> (also the arch optimization also does it, so I think it might be a
> good idea to keep the implementation with similar semantic).

Does the arc4random usecase require the xor zeroing though? Think
it would be a mistake to gurantee it as it seems like a pretty reasonable
thing to want to optimize out if we need better performance.