From ae6c192125d57dc78bc1a1c21620e7ed02c7ec3a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Jul 2021 14:00:03 +0200 Subject: fixup! glib-aux: put more effort into seeding GRand fallback for nm_utils_random_bytes() --- src/libnm-glib-aux/nm-random-utils.c | 94 +++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 40 deletions(-) diff --git a/src/libnm-glib-aux/nm-random-utils.c b/src/libnm-glib-aux/nm-random-utils.c index d8e716c90e..f622b28411 100644 --- a/src/libnm-glib-aux/nm-random-utils.c +++ b/src/libnm-glib-aux/nm-random-utils.c @@ -21,52 +21,49 @@ /*****************************************************************************/ -static GRand * -_rand_create_global(void) +#define SEED_ARRAY_SIZE (16 + 2 + 4 + 2 + 3) + +static guint32 +_pid_hash(pid_t id) +{ + if (sizeof(pid_t) > sizeof(guint32)) + return (((guint64) id) >> 32) ^ ((guint64) id); + return id; +} + +static void +_rand_init_seed(guint32 seed_array[static SEED_ARRAY_SIZE], GRand *rand) { - GRand * rand1; - guint32 seed_array[16 + 2 + 4 + 2 + 2]; int seed_idx; const guint8 *p_at_random; guint64 now_nsec; - /* We only call this once, and only in case where getrandom()/urandom - * already failed. Let's try hard to get a good seed. - * - * We also don't want to use glib's singleton GRand instance, because - * our code should not consume random numbers from that instance. */ - - rand1 = g_rand_new(); - - /* Get some seed material by using GRand. g_rand_new() is itself seeded - * from /dev/urandom (good, but we are already in a scenario where that - * failed) or fallback to the time/pid (not so good). */ + /* Get some seed material from the provided GRand. */ for (seed_idx = 0; seed_idx < 16; seed_idx++) - seed_array[seed_idx] = g_rand_int(rand1); + seed_array[seed_idx] = g_rand_int(rand); /* Add an address from the heap. */ - seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand1))) >> 32; - seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand1))); - - g_rand_free(rand1); + seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand))) >> 32; + seed_array[seed_idx++] = ((guint64) ((uintptr_t) ((gpointer) rand))); /* Add the per-process, random number. */ p_at_random = ((gpointer) getauxval(AT_RANDOM)); if (p_at_random) { - G_STATIC_ASSERT(sizeof(guint32) == 4); memcpy(&seed_array[seed_idx], p_at_random, 16); - seed_idx += 4; - } + } else + memset(&seed_array[seed_idx], 0, 16); + G_STATIC_ASSERT_EXPR(sizeof(guint32) == 4); + seed_idx += 4; /* Add the current timestamp, the pid and ppid. */ now_nsec = nm_utils_clock_gettime_nsec(CLOCK_BOOTTIME); seed_array[seed_idx++] = ((guint64) now_nsec) >> 32; seed_array[seed_idx++] = ((guint64) now_nsec); - seed_array[seed_idx++] = getpid(); - seed_array[seed_idx++] = getppid(); + seed_array[seed_idx++] = _pid_hash(getpid()); + seed_array[seed_idx++] = _pid_hash(getppid()); + seed_array[seed_idx++] = _pid_hash(gettid()); - nm_assert(seed_idx <= G_N_ELEMENTS(seed_array)); - return g_rand_new_with_seed_array(seed_array, seed_idx); + nm_assert(seed_idx == SEED_ARRAY_SIZE); } static GRand * @@ -74,27 +71,44 @@ _rand_create_thread_local(void) { G_LOCK_DEFINE_STATIC(global_rand); static GRand *global_rand = NULL; - guint32 seed_buffer[16]; - gsize i; + guint32 seed_array[SEED_ARRAY_SIZE]; - /* For our global_rand instance we use a seed derived from getauxval(AT_RANDOM) - * or the PID, which don't change per process. So that is not a good source - * to initialize multiple GRand instances (for each thread). + /* We use thread-local instances of GRand to create a series of + * "random" numbers. We use thread-local instances, so that we don't + * require additional locking except the first time. * - * Instead, we use the random numbers generated by global_rand to - * seed the thread local instances. */ + * We trust that once seeded, a GRand gives us a good enough stream of + * random numbers. If that wouldn't be the case, then maybe GRand should + * be fixed. + * Also, we tell our callers that the numbers from GRand are not good. + * But that isn't gonna help, because callers have no other way to get + * better random numbers, so usually the just ignore the failure and make + * the best of it. + * + * That means, the remaining problem is to seed the instance well. + * Note that we are already in a situation where getrandom() failed + * to give us good random numbers. So we can not do much to get reliably + * good entropy for the seed. */ G_LOCK(global_rand); - if (G_UNLIKELY(!global_rand)) - global_rand = _rand_create_global(); - - for (i = 0; i < G_N_ELEMENTS(seed_buffer); i++) - seed_buffer[i] = g_rand_int(global_rand); + if (G_UNLIKELY(!global_rand)) { + GRand *rand1; + + /* g_rand_new() reads /dev/urandom, but we already noticed that + * /dev/urandom fails to give us good randomness (which is why + * we hit this code path). So this may not be as good as we wish, + * but let's add it to the mix. */ + rand1 = g_rand_new(); + _rand_init_seed(seed_array, rand1); + global_rand = g_rand_new_with_seed_array(seed_array, SEED_ARRAY_SIZE); + g_rand_free(rand1); + } + _rand_init_seed(seed_array, global_rand); G_UNLOCK(global_rand); - return g_rand_new_with_seed_array(seed_buffer, G_N_ELEMENTS(seed_buffer)); + return g_rand_new_with_seed_array(seed_array, SEED_ARRAY_SIZE); } /** -- cgit v1.2.1