Hi Aaron, this isn't a full review at all but I noticed two little nits while going over this series.
On 10/28/26 3:14 PM, Aaron Conole via dev wrote: > This commit makes the ChaCha20 CSPRNG prefer using > OpenSSL's SIMD optimized ChaCha20 on platforms which > are compiled with such support. This represents a > major improvement on performance, without impacting > the underlying CSPRNG functionality. > > The EVP backend re-keys from the first bytes of every > refill, just as the built-in implementation does, so > that the fast key erasure - and thus the forward > secrecy - of the generator is the same for both. The > built-in ChaCha20 implementation is always built so > that it can serve as a fallback when OpenSSL is unable > to provide ChaCha20, for example when it only offers > approved algorithms in FIPS mode. > > Assisted-by: LLM (claude, opencode, sonnet, opus, qwen3.8) > Signed-off-by: Aaron Conole <[email protected]> > --- > v2->v3: > * Update the the csprng to provide local ChaCha20 when > openssl isn't available for many reasons. > * Updated the cs_random_bytes() max comment (since the OpenSSL > version will shrink the max a bit more). > * Updated the rekey to provide forward key erasure and forward > secrecy. > > lib/random.c | 217 ++++++++++++++++++++++++++++++++++++++++++--------- > lib/random.h | 2 +- > 2 files changed, 182 insertions(+), 37 deletions(-) > > diff --git a/lib/random.c b/lib/random.c > index d2f28866a2..27d57530f7 100644 > --- a/lib/random.c > +++ b/lib/random.c > @@ -18,15 +18,24 @@ > #include "random.h" > > #include <errno.h> > +#include <stdbool.h> > #include <stdlib.h> > #include <sys/time.h> > > +#if defined(HAVE_OPENSSL) && !defined(OPENSSL_NO_CHACHA) > +#include <openssl/evp.h> > +#define HAVE_CHACHA20_EVP 1 > +#endif > + > #include "byte-order.h" > #include "entropy.h" > #include "hash.h" > #include "ovs-thread.h" > #include "timeval.h" > #include "util.h" > +#include "openvswitch/vlog.h" > + > +VLOG_DEFINE_THIS_MODULE(random); > > /* This is the 32-bit PRNG recommended in G. Marsaglia, "Xorshift RNGs", > * _Journal of Statistical Software_ 8:14 (July 2003). According to the > paper, > @@ -120,6 +129,39 @@ random_next(void) > } > > > +/* Cryptographic secure pseudo-random number generator. */ > + > +/* Refill a page worth of keystream at a time to amortize the block cost. */ > +#define CSPRNG_BUFSZ 4096 > + > +#define CHACHA_KEYLEN 32 /* 256-bit key. */ > +#define CHACHA_IVLEN 16 /* ChaCha20 IV: counter and nonce. */ > + > +/* Reseed from the OS entropy pool after this many bytes served, matching > + * the arc4random policy of periodic reseeding. */ > +#define CSPRNG_RESEED (1600 * 1024) > + > +#ifdef HAVE_CHACHA20_EVP > +/* The OpenSSL EVP backend consumes an IV in addition to a key. */ > +#define CSPRNG_SECRETS (CHACHA_KEYLEN + CHACHA_IVLEN) > +#else > +#define CSPRNG_SECRETS CHACHA_KEYLEN > +#endif > + > +struct csprng { > +#ifdef HAVE_CHACHA20_EVP > + EVP_CIPHER_CTX *ctx; /* OpenSSL EVP state, once created. */ > + bool use_evp; /* Serve from OpenSSL rather than the > + * built-in implementation. */ > +#endif > + 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; > +}; > + > /* This is a 32-bit CSPRNG adapted from the ChaCha20 algorithm described in > * RFC 7539 and RFC 8439, similar to arc4random(3) and the Linux kernel's > * get_random_u32(). > @@ -130,14 +172,6 @@ random_next(void) > */ > > #define CHACHA_ROUNDS 20 > -#define CHACHA_KEYLEN 32 /* 256-bit key. */ > - > -/* Refill a page worth of keystream at a time to amortize the block cost. */ > -#define CSPRNG_BUFSZ 4096 > - > -/* Reseed from the OS entropy pool after this many bytes served, matching > - * the arc4random policy of periodic reseeding. */ > -#define CSPRNG_RESEED (1600 * 1024) > > #define ROTL32(v, n) (((v) << (n)) | ((v) >> (32 - (n)))) > > @@ -147,20 +181,13 @@ random_next(void) > a += b; d ^= a; d = ROTL32(d, 8); \ > c += d; b ^= c; b = ROTL32(b, 7) > > -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"; > > +#ifndef HAVE_CHACHA20_EVP > /* Current random state. */ > DEFINE_STATIC_PER_THREAD_DATA(struct csprng, cs_state, { .initialized = 0 }); > +#endif > > /* Produces one 64-byte ChaCha20 block from 'input' (16 words) into 'out'. */ > void > @@ -211,22 +238,8 @@ csprng_rekey(struct csprng *c, const uint8_t > key[CHACHA_KEYLEN]) > } > } > > -static void > -csprng_seed(struct csprng *c) > -{ > - uint8_t key[CHACHA_KEYLEN]; > - > - get_entropy_or_die(key, sizeof key); > - csprng_rekey(c, key); > - ovs_memzero(key, sizeof key); > - > - c->pos = sizeof c->buf; > - c->served = 0; > - c->initialized = 1; > -} > - > -/* Fills csprng with fresh keystream, reserving the first 32 bytes to re-key > - * the generator (fast key erasure / forward secrecy). */ > +/* Fills 'c' with fresh keystream, reserving the first 32 bytes to re-key the > + * generator (fast key erasure / forward secrecy). */ Nit: This can be rebased into patch 2 of the set. > static void > csprng_refill(struct csprng *c) > { > @@ -246,21 +259,153 @@ csprng_refill(struct csprng *c) > c->pos = CHACHA_KEYLEN; > } > > +#ifdef HAVE_CHACHA20_EVP > +/* This is the EVP based CSPRNG using the OpenSSL EVP framework to create a > + * long stream of bytes, and pull those. The OpenSSL EVP manages the > + * underlying bytes, all this does is manage the policy. It re-keys from the > + * beginning of every refill, as the built-in implementation above does, so > + * that both provide the same fast key erasure. > + */ > + > +static void > +csprng_state_destroy(void *ctx) > +{ > + struct csprng *csp = ctx; > + > + if (csp->ctx) { > + EVP_CIPHER_CTX_free(csp->ctx); > + csp->ctx = NULL; > + } > + > + free(csp); > +} Nit: It would be nice if the above and below functions followed the same naming convention. - Rosemarie O'Riorden > + > +static struct csprng * > +cs_state_get(void) > +{ > + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; > + static ovsthread_key_t cskey; > + struct csprng *csp; > + > + if (ovsthread_once_start(&once)) { > + ovsthread_key_create(&cskey, csprng_state_destroy); > + ovsthread_once_done(&once); > + } > + > + csp = ovsthread_getspecific(cskey); > + if (!csp) { > + csp = xzalloc(sizeof *csp); > + csp->use_evp = true; > + ovsthread_setspecific(cskey, csp); > + } > + return csp; > +} > + > +/* (Re)keys the OpenSSL EVP state from 'key' (32 bytes) and 'iv' (16 bytes). > */ > +static bool > +csprng_evp_rekey(struct csprng *c, const uint8_t key[CHACHA_KEYLEN], > + const uint8_t iv[CHACHA_IVLEN]) > +{ > + return EVP_EncryptInit_ex(c->ctx, EVP_chacha20(), NULL, key, iv) == 1; > +} > + > +/* Seeds the OpenSSL EVP state from fresh entropy, returning false if OpenSSL > + * cannot provide ChaCha20. */ > +static bool > +csprng_evp_seed(struct csprng *c, const uint8_t key[CHACHA_KEYLEN]) > +{ > + uint8_t iv[CHACHA_IVLEN]; > + bool ok; > + > + if (!c->ctx) { > + c->ctx = EVP_CIPHER_CTX_new(); > + if (!c->ctx) { > + VLOG_WARN_ONCE("cs_random: cannot create cipher context."); > + return false; > + } > + } > + > + get_entropy_or_die(iv, sizeof iv); > + ok = csprng_evp_rekey(c, key, iv); > + ovs_memzero(iv, sizeof iv); > + > + if (!ok) { > + /* OpenSSL may not provide ChaCha20 at all, for example when it only > + * offers approved algorithms in FIPS mode. Fall back to the > built-in > + * implementation rather than failing to provide randomness. */ > + VLOG_WARN_ONCE("cs_random: OpenSSL ChaCha20 unavailable, using the " > + "built-in implementation."); > + } > + return ok; > +} > + > +/* Fills 'c' with fresh keystream from OpenSSL, reserving the first bytes to > + * re-key the generator (fast key erasure / forward secrecy). */ > +static void > +csprng_evp_refill(struct csprng *c) > +{ > + static const uint8_t zero[CSPRNG_BUFSZ]; > + int outl; > + > + if (EVP_EncryptUpdate(c->ctx, c->buf, &outl, zero, sizeof c->buf) != 1 || > + outl != sizeof c->buf) { > + VLOG_FATAL("cs_random: EVP fill failure."); > + } > + > + csprng_evp_rekey(c, c->buf, c->buf + CHACHA_KEYLEN); > + ovs_memzero(c->buf, CSPRNG_SECRETS); > + c->pos = CSPRNG_SECRETS; > +} > +#endif /* HAVE_CHACHA20_EVP */ > + > +static void > +csprng_seed(struct csprng *c) > +{ > + uint8_t key[CHACHA_KEYLEN]; > + > + get_entropy_or_die(key, sizeof key); > + > +#ifdef HAVE_CHACHA20_EVP > + if (c->use_evp && !csprng_evp_seed(c, key)) { > + c->use_evp = false; > + } > + if (!c->use_evp) { > + csprng_rekey(c, key); > + } > +#else > + csprng_rekey(c, key); > +#endif > + > + ovs_memzero(key, sizeof key); > + > + c->pos = sizeof c->buf; > + c->served = 0; > + c->initialized = 1; > +} > + > void > cs_random_bytes(void *b, size_t l) > { > struct csprng *c = cs_state_get(); > > - /* c->buf isn't fully available for reading because the first 32 bytes > - * of every refill are consumed as the next key. */ > - ovs_assert(l <= sizeof c->buf - CHACHA_KEYLEN); > + /* c->buf isn't fully available for reading because the first bytes of > + * every refill are consumed as the next key and IV. */ > + ovs_assert(l <= sizeof c->buf - CSPRNG_SECRETS); > > if (!c->initialized || c->served >= CSPRNG_RESEED) { > csprng_seed(c); > } > > if (c->pos + l > sizeof c->buf) { > +#ifdef HAVE_CHACHA20_EVP > + if (c->use_evp) { > + csprng_evp_refill(c); > + } else { > + csprng_refill(c); > + } > +#else > csprng_refill(c); > +#endif > } > > memcpy(b, c->buf + c->pos, l); > diff --git a/lib/random.h b/lib/random.h > index 77792e28f1..76f165c7a8 100644 > --- a/lib/random.h > +++ b/lib/random.h > @@ -53,7 +53,7 @@ void chacha20_block(const uint32_t input[CHACHA_K_INPUT], > uint8_t out[CHACHA_BLOCK]); > > /* Fills 'b' with 'l' cryptographically secure random bytes. 'l' must not be > - * larger than 4064; callers needing more bytes should make multiple calls. > */ > + * larger than 4048; callers needing more bytes should make multiple calls. > */ > void cs_random_bytes(void *, size_t); > uint32_t cs_random_uint32(void); > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
