On 27 Aug 2026, at 17:23, Aaron Conole via dev wrote:
> random_uint32() is a xorshift32 PRNG (see lib/random.c): it holds many
> properties including uniformity and reversibility, so recovering the
> internal state from any single observed output lets an attacker
> compute every other output of that (per-thread) stream, forward and
> backward. Using it for anything security-relevant is documented in
> random.c itself as inappropriate.
>
> Most NAT tuple selection does not call random_uint32() directly.
> Instead, nat_get_unique_tuple() picks both the NAT address
> (get_addr_in_range()) and, in the common case, the NAT port
> (set_sport_range()/set_dport_range()) via nat_range_hash(), a
> deterministic hash of the flow tuple mixed with a single 32-bit value:
> ct->hash_basis. That basis was previously drawn once from
> random_uint32() at conntrack_init() time. Because it is a single,
> long-lived value on which every NAT address and default-port choice
> depends, an attacker able to recover the xorshift state feeding that
> one random_uint32() call--e.g. by observing any other output drawn
> from the same per-thread PRNG stream elsewhere in the process--could
> predict every NAT tuple ovs-vswitchd will assign. In effect, it was
> derived from a shared, reversible state.
>
> Two narrower paths draw directly from random_uint32() per new
> connection rather than through the hash: the `nat(...,random)` port
> path, and the retry offset used when the hash-selected port collides
> with an existing connection under range congestion. These are
> directly observable per-connection outputs.
>
> Fix this by drawing ct->hash_basis, and the two per-connection port
> paths above, from a cryptographic source: OpenSSL's PRNG when
> compiled against OpenSSL 1.1.0+ (checking RAND_status() to catch the
> case where it has not been seeded properly despite RAND_bytes()
> succeeding, the same check already used in lib/stream-ssl.c), falling
> back to the system entropy pool.
>
> If neither source can provide randomness, do not silently downgrade
> to the non-cryptographic PRNG. nat_random_uint32() instead reports
> failure, and its callers propagate that as NAT tuple exhaustion:
> nat_get_unique_l4() and nat_get_unique_tuple() both return false, so
> the connection attempt is failed the same way as any other allocation
> exhaustion (see the nat_res_exhaustion path in conn_update_state()),
> and each occurrence increments the new conntrack_entropy_failed
> coverage counter so operators can see it happening. There is also a
> new point for ovs-vswitchd failure - at the conntrack initialization
> if sufficient entropy cannot be pulled for ct->hash_basis.
>
> Signed-off-by: Aaron Conole <[email protected]>
Hi Aaron,
Thanks for the patch. See some comments below. As Ilya mentioned offline,
it might be good to get some performance numbers with this change to see
the impact of the RAND_bytes() usage.
//Eelco
> ---
> lib/conntrack.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index f84cdd216a..d7fa6bd746 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -22,6 +22,10 @@
> #include <netinet/icmp6.h>
> #include <string.h>
>
> +#ifdef HAVE_OPENSSL
> +#include <openssl/rand.h>
> +#endif
> +
> #include "conntrack.h"
> #include "conntrack-private.h"
> #include "conntrack-tp.h"
> @@ -30,6 +34,7 @@
> #include "csum.h"
> #include "ct-dpif.h"
> #include "dp-packet.h"
> +#include "entropy.h"
> #include "flow.h"
> #include "netdev.h"
> #include "odp-netlink.h"
> @@ -47,6 +52,45 @@
>
> VLOG_DEFINE_THIS_MODULE(conntrack);
>
> +/* Counts calls to nat_random_uint32() that could not obtain any
> + * cryptographic randomness at all (neither OpenSSL nor the system
> + * entropy pool). Each occurrence corresponds to one failed NAT tuple
> + * allocation attempt. */
> +COVERAGE_DEFINE(conntrack_entropy_failed);
> +
> +/* NAT tuple selection (both the address/port hash basis and the
> + * fully-random port paths) must not be predictable from an observed
> + * random_uint32() output, so it draws from a cryptographic source here
> + * instead of the general-purpose xorshift32 PRNG in random.c.
> + *
> + * When compiled against OpenSSL 1.1.0+, this uses OpenSSL's PRNG. If
> + * that is unavailable, or RAND_status() reports that it is not properly
> + * seeded (see the same check in lib/stream-ssl.c), it falls back to the
> + * system entropy pool. If neither source can provide randomness, this
> + * returns false rather than falling back to the non-cryptographic PRNG:
> + * callers must fail the NAT tuple allocation (and, transitively, the
> + * connection attempt) instead of silently downgrading its quality. */
> +static bool
> +nat_random_uint32(uint32_t *r)
> +{
> +#ifdef HAVE_OPENSSL
> + if (RAND_bytes((uint8_t *) r, sizeof *r) == 1 && RAND_status()) {
No idea how expensive this is, and/or if it takes systemcalls.
This is used in the fast path, so maybe we should do some benchmarking?
> + return true;
> + }
> +
> + static struct vlog_rate_limit rl1 = VLOG_RATE_LIMIT_INIT(1, 5);
> + VLOG_WARN_RL(&rl1, "RAND_bytes unreliable, falling back to system "
> + "entropy pool for NAT tuple selection");
> +#endif
> +
> + if (!get_entropy(r, sizeof *r)) {
The fallback is expensive; it does open/read/close() on /dev/urandom.
> + return true;
> + }
> +
> + COVERAGE_INC(conntrack_entropy_failed);
> + return false;
> +}
> +
> COVERAGE_DEFINE(conntrack_full);
> COVERAGE_DEFINE(conntrack_l3csum_checked);
> COVERAGE_DEFINE(conntrack_l3csum_err);
> @@ -251,8 +295,16 @@ conntrack_init(void)
>
> /* This value can be used during init (e.g. timeout_policy_init()),
> * set it first to ensure it is available.
> - */
> - ct->hash_basis = random_uint32();
> + *
> + * It is also the basis that nat_range_hash() mixes into every NAT
> + * address and (non-random) NAT port choice, so it must come from
> + * nat_random_uint32() rather than the predictable general-purpose
> + * PRNG--otherwise all NAT tuples derived from it are only as
The -- looks odd here, maybe an AI text cut/paste. Guess a '; ' would be better
here.
> + * unpredictable as that single 32-bit xorshift output. */
Also, to a non-native speaker, the 'unpredictable' reads odd. Maybe something
like;
* PRNG; otherwise, all NAT tuples derived from it would be
* predictable from that PRNG output.
> + if (!nat_random_uint32(&ct->hash_basis)) {
> + VLOG_FATAL("conntrack: unable to obtain cryptographic randomness "
> + "to initialize the NAT hash basis");
Are we ok with not having a backup?
> + }
>
> ovs_rwlock_init(&ct->resources_lock);
> ovs_rwlock_wrlock(&ct->resources_lock);
> @@ -2556,8 +2608,19 @@ another_round:
> }
>
> if (attempts < range && attempts >= 16) {
> + uint32_t r;
> +
> + if (!nat_random_uint32(&r)) {
> + /* CPRNG wasn't available, return false in this case. It is
> + * possible that the entropy pool is only temporarily unaviable,
> + * but bailing on this connection attempt should be okay since
> + * since we don't want to waste cpu cycles for an event that may
> + * take a while. */
> + return false;
> + }
> +
> attempts /= 2;
> - curr = min + (random_uint32() % range);
> + curr = min + (r % range);
> goto another_round;
> }
>
> @@ -2613,7 +2676,12 @@ nat_get_unique_tuple(struct conntrack *ct, struct conn
> *conn,
> hash = nat_range_hash(fwd_key, basis, nat_info);
>
> if (nat_info->nat_flags & NAT_RANGE_RANDOM) {
> - port_off = random_uint32();
> + if (!nat_random_uint32(&port_off)) {
> + /* The entropy failure here will reflect that we're resource
> + * exhausted. It is a bit confusing because we're out of
> + * entropy rather than out of actual NAT range. */
> + return false;
> + }
> } else if (basis) {
> port_off = hash;
> } else {
> --
> 2.55.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
