Ilya Maximets <[email protected]> writes: > On 10/30/26 1:26 PM, Aaron Conole wrote: >> Aaron Conole <[email protected]> writes: >> >>> Ilya Maximets <[email protected]> writes: >>> >>>> On 10/28/26 8:14 PM, Aaron Conole wrote: >>>>> Until now, utilities wishing access to a quality source of >>>>> cryptographically secure random numbers would have to either >>>>> forego security and just use the xorshift, or use one of a >>>>> few very slow entropy options like get_entropy_or_die or >>>>> OpenSSL's RAND_bytes() calls. Both of these are quality >>>>> tested mechanisms, but the drawback is a very long time to >>>>> execute (measured 10m iterations of rand_bytes taking >>>>> roughly 5.14s to complete). >>>>> >>>>> This change adds a ChaCha20 based CSPRNG, which is known >>>>> to be a high quality CSPRNG. The benefits are very fast >>>>> execution time (the same 10m iterations only take 0.059s >>>>> vs. the PRNG taking 0.018s). >>>>> >>>>> The following references can help with understanding the >>>>> implementation: >>>>> * RFC 8439, "ChaCha20 and Poly1305 for IETF Protocols" >>>>> * libevent/arc4random.c, "Arc4 random number generator for OpenBSD." >>>>> * glibc/stdlib/arc4random.c, "Psuedo random number generator" >>>>> * Ben Tasker, "Writing (and backdooring) a ChaCha20 based CSPRNG" >>>>> >>>>> Assisted-by: LLM (claude, opencode, sonnet, opus, qwen3.8) >>>>> Signed-off-by: Aaron Conole <[email protected]> >>>>> --- >>>>> v2->v3: >>>>> * Update the verbiage around the implementations origins >>>>> * Still exists the possiblity to abort on reseed >>>>> * Fixed the CHACHA_K_INPUT comment >>>>> * Fixed the chacha_sigma constant definition + comment >>>>> * Documented maximum length in csprng_random_bytes >>>>> * Introduce ovs_memzero that will clear memory without eliding >>>>> the operations >>>>> * Zero out the chacha20_block >>>> >>>> [...] >>>> >>>>> +struct csprng { >>>>> + uint32_t state[16]; /* ChaCha20 state: >>>>> + * constant|key|counter|nonce. */ >>>>> + uint8_t buf[CSPRNG_BUFSZ]; /* Buffered keystream. */ >>>>> + size_t pos; /* Next unused byte in 'buf'. */ >>>>> + size_t served; /* Bytes served since last reseed. */ >>>>> + int initialized; >>>>> +}; >>>>> + >>>>> +/* ChaCha20 constant: "expand 32-byte k". Must be at least 16 >>>>> bytes + NUL. */ >>>>> +static const uint8_t chacha_sigma[] = "expand 32-byte k"; >>>>> + >>>>> +/* Current random state. */ >>>>> +DEFINE_STATIC_PER_THREAD_DATA(struct csprng, cs_state, { >>>>> .initialized = 0 }); >>>> >>>> LLMs complain that this state is preserved on fork. It's probably not a >>>> big >>>> deal since there should be no cases where this code would be used >>>> before forking >>>> today, but maybe a bit of a trap. Maybe we should make xfork_at() >>>> call a special >>>> function that clears the state if it was initialized before? >>> >>> It's a weird complaint. Honestly, there isn't much that we can do if >>> someone does a fork() unless we explicitly are only calling forks from >>> our library. But a code that can somehow do a fork() from a called >>> library will still completely bypass any preventing measures we do here. > > Code that forks in a called library is unlikely to use our random.c. > >>> >>> The real way to do it safely is to use xmalloc() for the per-thread >>> allocation and madvise(region, MADV_WIPEONFORK), I think. Then even if >>> a library we link do does a fork(), the pages are guaranteed to be >>> wiped. >> >> Okay - coffee kicked in. madvise() MADV_WIPEONFORK is only linux from >> my little bit of checking. FreeBSD (and I think NetBSD maybe?) supports >> using something like minherit(.., MAP_INHERIT_ZERO) instead. >> >> So we can probably provide something like: >> >> xmalloc_fork_zero() >> >> which either sets the minherit on the range, or sets madvise() to >> wipeonfork and maybe that could be the "more portable" approach. >> >>>> It's not the only place where we can fork, but it may be better to >>>> convert other >>>> place into xfork if needed in the future. >>>> >>>> WDYT? >> >> There is also a pthread_atfork() function that takes a callback, and we >> could use that to clear the state in the child through the child >> function handler (IIUC). That should run right after the fork() >> completes. We could use that instead. Do you have a preference? > We also need to keep in mind Windows that is supported on our older > branches. I'd suggest we do not overengineer this and just assume > that anyone who uses our implementation of random is also using our > xfork. This is also not a publicly exported library, so we can make > such assumptions. May add a comment about this at the top of the > cs_random_bytes() along with the size restriction, I guess.
Windows doesn't really support fork() properly anyway, so I don't think we need to worry too much about it (at least, all of the stackoverflow, reddit, and learn.microsoft pages seem to imply it). When looking into solutions I even saw this (from mingw) https://github.com/mingw-w64/mingw-w64/blob/c47c7b4d6875104ca001f8e7300a4e2d0f789556/mingw-w64-libraries/winpthreads/include/pthread.h#L145 145: /* No fork() in windows - so ignore this */ 146: #define pthread_atfork(F1,F2,F3) 0 So I guess windows shouldn't really be a concern that way. Just to present the options as I see them: 1. Just call forbid_forking() from the initialization code for the csprng states and be done with it. Then xfork()/xfork_at() don't work at all, and we can just document it. No worries anymore, not too hard. 2. Using pthread_atfork() (or an equivalent that just provides our memory space from the exact context we have it to be cleared). Again in the initialization routine. This is a POSIX standard function, and we just need to write the callback routine to pass as an argument. This works with fork()/xfork()/xfork_at() to clear the state. Mostly portable. 3. Trying to slice up madvice/minherit type semantics based on the underlying system. They exist, and are for this kind of purpose. But they require system specific ifdefs to figure out which one to use. And rigging that is probably a bit error prone. This works with fork()/xfork()/xfork_at(), but also requires a bit higher maintenance burden. 4. Write our own callback routine type thing for xfork()/xfork_at(), convert any callers in-tree to definitely use xfork()/xfork_at(), and document it as well. This doesn't work with fork(), but gives a little bit of protection for the xfork()/xfork_at() case while making changes to the ovs-thread/utils area, and we need to maintain some kind of callback system, plus the callback that will be in random.c. Feels like a lot of work for a little gain. 5. Just write, "Don't fork() after intializing, as we don't guarantee behavior" as a comment on `cs_random_bytes()`. This seems like least complicated because it kicks the can down the road until there's a caller outside of conntrack (ex: if we add some kind of proper ip stack thingy for generating packets, we may need it there for ipid or something). WDYT? My own preference order looks like: 2, 1, 5, 4, 3 - maybe there's an option I missed though, or I'm misunderstanding your suggestions. > Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
