From e80fc43f2a12db7feb7b7ad0807db3a0cf244bae Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Aug 2022 12:10:41 +0200 Subject: [PATCH 1/3] glib-aux: add assertions to nm_utils_fd_wait_for_event() --- src/libnm-glib-aux/nm-shared-utils.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index e178b4b14d..85ea3a1116 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -3383,6 +3383,8 @@ nm_utils_fd_wait_for_event(int fd, int event, gint64 timeout_nsec) struct timespec ts, *pts; int r; + nm_assert(fd >= 0); + if (timeout_nsec < 0) pts = NULL; else { @@ -3396,6 +3398,13 @@ nm_utils_fd_wait_for_event(int fd, int event, gint64 timeout_nsec) return -NM_ERRNO_NATIVE(errno); if (r == 0) return 0; + + nm_assert(r == 1); + nm_assert(pollfd.revents > 0); + + if (pollfd.revents & POLLNVAL) + return nm_assert_unreachable_val(-EBADF); + return pollfd.revents; } From d20343c9d0967e5dee8802f6e869f0b20d1b33ce Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 Aug 2022 20:11:23 +0200 Subject: [PATCH 2/3] glib-aux: rework random number utils Heavily inspired by systemd ([1]). We now also have nm_random_get_bytes{,_full}() and nm_random_get_crypto_bytes(), like systemd's random_bytes() and crypto_random_bytes(), respectively. Differences: - instead of systemd's random_bytes(), our nm_random_get_bytes_full() also estimates whether the output is of high quality. The caller may find that interesting. Due to that, we will first try to call getrandom(GRND_NONBLOCK) before getrandom(GRND_INSECURE). That is reversed from systemd's random_bytes(), because we want to find out whether we can get good random numbers. In most cases, kernel should have entropy already, and it makes no difference. Otherwise, heavily rework the code. It should be easy to understand and correct. There is also a major bugfix here. Previously, if getrandom() failed with ENOSYS and we fell back to /dev/urandom, we would assume that we have high quality random numbers. That assumption is not warranted. Now instead poll on /dev/random to find out. [1] https://github.com/systemd/systemd/blob/a268e7f4021072e120a03b42660fad21e465c44e/src/basic/random-util.c#L81 --- src/core/devices/nm-device.c | 4 +- src/core/devices/wifi/nm-iwd-manager.c | 2 +- src/core/nm-core-utils.c | 6 +- src/libnm-glib-aux/nm-hash-utils.c | 2 +- src/libnm-glib-aux/nm-random-utils.c | 349 ++++++++++++------ src/libnm-glib-aux/nm-random-utils.h | 10 +- src/libnm-glib-aux/nm-uuid.c | 4 +- .../tests/test-shared-general.c | 2 +- 8 files changed, 258 insertions(+), 121 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 6cc823b2ee..97dfe9ba97 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -1290,7 +1290,7 @@ out_fail: duid, duid_error); - nm_utils_random_bytes(&uuid, sizeof(uuid)); + nm_random_get_bytes(&uuid, sizeof(uuid)); duid_out = nm_utils_generate_duid_uuid(&uuid); } @@ -2073,7 +2073,7 @@ out_fail: fail_reason); client_id_buf = g_malloc(1 + 15); client_id_buf[0] = 0; - nm_utils_random_bytes(&client_id_buf[1], 15); + nm_random_get_bytes(&client_id_buf[1], 15); result = g_bytes_new_take(client_id_buf, 1 + 15); out_good: diff --git a/src/core/devices/wifi/nm-iwd-manager.c b/src/core/devices/wifi/nm-iwd-manager.c index 27222aaea8..2e0d51e5d8 100644 --- a/src/core/devices/wifi/nm-iwd-manager.c +++ b/src/core/devices/wifi/nm-iwd-manager.c @@ -306,7 +306,7 @@ iwd_agent_export(GDBusConnection *connection, gpointer user_data, char **agent_p unsigned int rnd; guint id; - nm_utils_random_bytes(&rnd, sizeof(rnd)); + nm_random_get_bytes(&rnd, sizeof(rnd)); nm_sprintf_buf(path, "/agent/%u", rnd); diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index 66f8204502..fb85d75a8b 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -2814,7 +2814,7 @@ _host_id_read(guint8 **out_host_id, gsize *out_host_id_len) int base64_save = 0; gsize len; - success = nm_utils_random_bytes(rnd_buf, sizeof(rnd_buf)); + nm_random_get_bytes_full(rnd_buf, sizeof(rnd_buf), &success); /* Our key is really binary data. But since we anyway generate a random seed * (with 32 random bytes), don't write it in binary, but instead create @@ -3315,7 +3315,7 @@ nm_utils_stable_id_random(void) { char buf[15]; - nm_utils_random_bytes(buf, sizeof(buf)); + nm_random_get_bytes(buf, sizeof(buf)); return g_base64_encode((guchar *) buf, sizeof(buf)); } @@ -3686,7 +3686,7 @@ nm_utils_hw_addr_gen_random_eth(const char *current_mac_address, { struct ether_addr bin_addr; - nm_utils_random_bytes(&bin_addr, ETH_ALEN); + nm_random_get_bytes(&bin_addr, ETH_ALEN); _hw_addr_eth_complete(&bin_addr, current_mac_address, generate_mac_address_mask); return nm_utils_hwaddr_ntoa(&bin_addr, ETH_ALEN); } diff --git a/src/libnm-glib-aux/nm-hash-utils.c b/src/libnm-glib-aux/nm-hash-utils.c index 68b3399800..941aba019a 100644 --- a/src/libnm-glib-aux/nm-hash-utils.c +++ b/src/libnm-glib-aux/nm-hash-utils.c @@ -45,7 +45,7 @@ again: guint8 _extra_entropy[3 * HASH_KEY_SIZE]; } t_arr; - nm_utils_random_bytes(&t_arr, sizeof(t_arr)); + nm_random_get_bytes(&t_arr, sizeof(t_arr)); /* We only initialize one random hash key. So we can spend some effort * of getting this right. For one, we collect more random bytes than diff --git a/src/libnm-glib-aux/nm-random-utils.c b/src/libnm-glib-aux/nm-random-utils.c index bbe83aa5a0..93eee7c4d3 100644 --- a/src/libnm-glib-aux/nm-random-utils.c +++ b/src/libnm-glib-aux/nm-random-utils.c @@ -10,6 +10,7 @@ #include #include #include +#include #if USE_SYS_RANDOM_H #include @@ -34,15 +35,118 @@ #define GRND_INSECURE 0x04 #endif -#if !HAVE_GETRANDOM && defined(SYS_getrandom) -static int +#if !HAVE_GETRANDOM +static ssize_t getrandom(void *buf, size_t buflen, unsigned flags) { +#if defined(SYS_getrandom) return syscall(SYS_getrandom, buf, buflen, flags); -} -#undef HAVE_GETRANDOM -#define HAVE_GETRANDOM 1 +#else + errno = ENOSYS; + return -1; #endif +} +#endif + +/*****************************************************************************/ + +static ssize_t +_getrandom(void *buf, size_t buflen, unsigned flags) +{ + static int have_getrandom = TRUE; + ssize_t l; + int errsv; + + nm_assert(buflen > 0); + + /* This calls getrandom() and either returns the positive + * success or an negative errno. ENOSYS means getrandom() + * call is not supported. That result is cached and we don't retry. */ + + if (!have_getrandom) + return -ENOSYS; + + l = getrandom(buf, buflen, flags); + if (l > 0) + return l; + if (l == 0) + return -EIO; + errsv = errno; + if (errsv == ENOSYS) + have_getrandom = FALSE; + return -errsv; +} + +static ssize_t +_getrandom_insecure(void *buf, size_t buflen) +{ + static int have_grnd_insecure = TRUE; + ssize_t l; + + /* GRND_INSECURE was added recently. We catch EINVAL + * if kernel does not support the flag (and cache it). */ + + if (!have_grnd_insecure) + return -EINVAL; + + l = _getrandom(buf, buflen, GRND_INSECURE); + + if (l == -EINVAL) + have_grnd_insecure = FALSE; + + return l; +} + +static ssize_t +_getrandom_best_effort(void *buf, size_t buflen) +{ + ssize_t l; + + /* To get best-effort bytes, we would use GRND_INSECURE (and we try that + * first). However, not all kernel versions support that, so we fallback + * to GRND_NONBLOCK. + * + * Granted, this is called from a fallback path where we have no entropy + * already, it's unlikely that GRND_NONBLOCK would succeed. Still... */ + l = _getrandom_insecure(buf, buflen); + if (l != -EINVAL) + return l; + + return _getrandom(buf, buflen, GRND_NONBLOCK); +} + +static int +_random_check_entropy(gboolean block) +{ + static gboolean seen_high_quality = FALSE; + nm_auto_close int fd = -1; + int r; + + /* We come here because getrandom() gave ENOSYS. We will fallback to /dev/urandom, + * but the caller wants to know whether we have high quality numbers. Poll + * /dev/random to find out. */ + + if (seen_high_quality) { + /* We cache the positive result. Once kernel has entropy, we will get + * good random numbers. */ + return 1; + } + + fd = open("/dev/random", O_RDONLY | O_CLOEXEC | O_NOCTTY); + if (fd < 0) + return -errno; + + r = nm_utils_fd_wait_for_event(fd, POLLIN, block ? -1 : 0); + + if (r <= 0) { + nm_assert(r < 0 || !block); + return r; + } + + nm_assert(r == 1); + seen_high_quality = TRUE; + return 1; +} /*****************************************************************************/ @@ -72,11 +176,8 @@ typedef struct _nm_packed { guint8 u8[NM_UTILS_CHECKSUM_LENGTH_SHA256 / 2]; guint32 u32[((NM_UTILS_CHECKSUM_LENGTH_SHA256 / 2) + 3) / 4]; } rand_vals; -#if HAVE_GETRANDOM guint8 rand_vals_getrandom[16]; -#endif - gint16 rand_vals_timestamp; - GRand *rand; + gint64 rand_vals_timestamp; } BadRandState; static void @@ -110,18 +211,7 @@ _bad_random_init_seed(BadRandSeed *seed) memcpy(&seed->auxval, p_at_random, 16); } -#if HAVE_GETRANDOM - { - ssize_t r; - - /* This is likely to fail, because we already failed a moment earlier. Still, give - * it a try. */ - r = getrandom(seed->getrandom_buf, - sizeof(seed->getrandom_buf), - GRND_INSECURE | GRND_NONBLOCK); - (void) r; - } -#endif + _getrandom_best_effort(seed->getrandom_buf, sizeof(seed->getrandom_buf)); seed->now_bootime = nm_utils_clock_gettime_nsec(CLOCK_BOOTTIME); seed->now_real = g_get_real_time(); @@ -172,18 +262,7 @@ _bad_random_bytes(guint8 *buf, gsize n) nm_utils_checksum_get_digest(sum, gl_state.sha_digest.full); } -#if HAVE_GETRANDOM - { - ssize_t r; - - /* This is likely to fail, because we already failed a moment earlier. Still, give - * it a try. */ - r = getrandom(gl_state.rand_vals_getrandom, - sizeof(gl_state.rand_vals_getrandom), - GRND_INSECURE | GRND_NONBLOCK); - (void) r; - } -#endif + _getrandom_best_effort(gl_state.rand_vals_getrandom, sizeof(gl_state.rand_vals_getrandom)); gl_state.rand_vals_timestamp = nm_utils_clock_gettime_nsec(CLOCK_BOOTTIME); @@ -216,105 +295,155 @@ _bad_random_bytes(guint8 *buf, gsize n) } } +/*****************************************************************************/ + /** - * nm_utils_random_bytes: + * nm_random_get_bytes_full: * @p: the buffer to fill * @n: the number of bytes to write to @p. + * @out_high_quality: (allow-none) (out): whether the returned + * random bytes are of high quality. * - * Uses getrandom() or reads /dev/urandom to fill the buffer - * with random data. If all fails, as last fallback it uses - * GRand to fill the buffer with pseudo random numbers. - * The function always succeeds in writing some random numbers - * to the buffer. The return value of FALSE indicates that the - * obtained bytes are probably not of good randomness. - * - * Returns: whether the written bytes are good. If you - * don't require good randomness, you can ignore the return - * value. - * - * Note that if calling getrandom() fails because there is not enough - * entropy (at early boot), the function will read /dev/urandom. - * Which of course, still has low entropy, and cause kernel to log - * a warning. + * - will never block + * - will always produce some numbers, but they may not + * be of high quality. + * - Whether they are of high quality, you can know via @out_high_quality. + * - will always try hard to produce high quality numbers, and on success + * they are as good as nm_random_get_crypto_bytes(). */ -gboolean -nm_utils_random_bytes(void *p, size_t n) +void +nm_random_get_bytes_full(void *p, size_t n, gboolean *out_high_quality) { int fd; int r; - gboolean has_high_quality = TRUE; - guint8 *buf = p; + gboolean has_high_quality; + ssize_t l; - if (n == 0) - return TRUE; + if (n == 0) { + NM_SET_OUT(out_high_quality, TRUE); + return; + } - g_return_val_if_fail(p, FALSE); + g_return_if_fail(p); -#if HAVE_GETRANDOM - { - static gboolean have_syscall = TRUE; +again_getrandom: + l = _getrandom(p, n, GRND_NONBLOCK); + if (l > 0) { + if ((size_t) l == n) { + NM_SET_OUT(out_high_quality, TRUE); + return; + } + p = ((uint8_t *) p) + l; + n -= l; + goto again_getrandom; + } - if (have_syscall) { - ssize_t r2; - int errsv; + /* getrandom() failed. Fallback to read /dev/urandom. */ - r2 = getrandom(buf, n, GRND_NONBLOCK); - if (r2 >= 0) { - if ((size_t) r2 == n) - return TRUE; - - /* no or partial read. There is not enough entropy. - * Fill the rest reading with the fallback code and remember - * that some bits are not high quality. */ - nm_assert((size_t) r2 < n); - buf += r2; - n -= r2; - - /* At this point, we don't want to read /dev/urandom, because - * the entropy pool is low (early boot?), and asking for more - * entropy causes kernel messages to be logged. - * - * Note that we fall back to _bad_random_bytes(), which (among others) seeds - * itself with g_rand_new(). That also will read /dev/urandom, but as - * we do that only once, we don't care. But in general, we are here in - * a situation where we want to avoid reading /dev/urandom too much. */ - goto out_bad_random; - } - errsv = errno; - if (errsv == ENOSYS) { - /* no support for getrandom(). We don't know whether - * we /dev/urandom will give us good quality. Assume yes. */ - have_syscall = FALSE; - } else if (errsv == EAGAIN) { - /* No entropy. We avoid reading /dev/urandom. */ - goto out_bad_random; - } else { - /* Unknown error, likely no entropy. We'll read /dev/urandom below, but we don't - * have high-quality randomness. */ - has_high_quality = FALSE; + if (l == -ENOSYS) { + /* no support for getrandom(). */ + if (out_high_quality) { + /* The caller wants to know whether we have high quality. Poll /dev/random + * to find out. */ + has_high_quality = (_random_check_entropy(FALSE) > 0); + } else { + /* The value doesn't matter in this case. It will be unused. */ + has_high_quality = FALSE; + } + } else { + /* Any other failure of getrandom() means we don't have high quality. */ + has_high_quality = FALSE; + if (l == -EAGAIN) { + /* getrandom(GRND_NONBLOCK) failed because lack of entropy. Retry with GRND_INSECURE. */ + for (;;) { + l = _getrandom_insecure(p, n); + if (l > 0) { + if ((size_t) l == n) { + NM_SET_OUT(out_high_quality, FALSE); + return; + } + p = ((uint8_t *) p) + l; + n -= l; + continue; + } + /* Any error. Fallback to /dev/urandom. */ + break; } } } -#endif -fd_open: +again_open: fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOCTTY); if (fd < 0) { if (errno == EINTR) - goto fd_open; - goto out_bad_random; + goto again_open; + } else { + r = nm_utils_fd_read_loop_exact(fd, p, n, TRUE); + nm_close(fd); + if (r >= 0) { + NM_SET_OUT(out_high_quality, has_high_quality); + return; + } } - r = nm_utils_fd_read_loop_exact(fd, buf, n, TRUE); - nm_close(fd); - if (r >= 0) - return has_high_quality; -out_bad_random: /* we failed to fill the bytes reading from /dev/urandom. - * Fill the bits using our pseudo random numbers. - * - * We don't have good quality. + * Fill the bits using our fallback approach (which obviously + * cannot give high quality random). */ - _bad_random_bytes(buf, n); - return FALSE; + _bad_random_bytes(p, n); + NM_SET_OUT(out_high_quality, FALSE); +} + +/*****************************************************************************/ + +/** + * nm_random_get_crypto_bytes: + * @p: the buffer to fill + * @n: the number of bytes to fill + * + * - can fail (in which case a negative number is returned + * and the output buffer is undefined). + * - will block trying to get high quality random numbers. + */ +int +nm_random_get_crypto_bytes(void *p, size_t n) +{ + nm_auto_close int fd = -1; + ssize_t l; + int r; + + if (n == 0) + return 0; + + nm_assert(p); + +again_getrandom: + l = _getrandom(p, n, 0); + if (l > 0) { + if ((size_t) l == n) + return 0; + p = (uint8_t *) p + l; + n -= l; + goto again_getrandom; + } + + if (l != -ENOSYS) { + /* We got a failure, but getrandom seems to be working in principle. We + * won't get good numbers. Fail. */ + return l; + } + + /* getrandom() failed with ENOSYS. Fallback to reading /dev/urandom. */ + + r = _random_check_entropy(TRUE); + if (r < 0) + return r; + if (r == 0) + return nm_assert_unreachable_val(-EIO); + + fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOCTTY); + if (fd < 0) + return -errno; + + return nm_utils_fd_read_loop_exact(fd, p, n, FALSE); } diff --git a/src/libnm-glib-aux/nm-random-utils.h b/src/libnm-glib-aux/nm-random-utils.h index d0eae1033b..ab8aee1b03 100644 --- a/src/libnm-glib-aux/nm-random-utils.h +++ b/src/libnm-glib-aux/nm-random-utils.h @@ -6,6 +6,14 @@ #ifndef __NM_RANDOM_UTILS_H__ #define __NM_RANDOM_UTILS_H__ -gboolean nm_utils_random_bytes(void *p, size_t n); +void nm_random_get_bytes_full(void *p, size_t n, gboolean *out_high_quality); + +static inline void +nm_random_get_bytes(void *p, size_t n) +{ + nm_random_get_bytes_full(p, n, NULL); +} + +int nm_random_get_crypto_bytes(void *p, size_t n); #endif /* __NM_RANDOM_UTILS_H__ */ diff --git a/src/libnm-glib-aux/nm-uuid.c b/src/libnm-glib-aux/nm-uuid.c index 5e7d5a7c3f..53e8b78cd7 100644 --- a/src/libnm-glib-aux/nm-uuid.c +++ b/src/libnm-glib-aux/nm-uuid.c @@ -115,12 +115,12 @@ nm_uuid_generate_random(NMUuid *out_uuid) /* See also, systemd's id128_make_v4_uuid() */ - /* nm_utils_random_bytes() is supposed to try hard to give good + /* nm_random_get_bytes() is supposed to try hard to give good * randomness. If it fails, it still makes an effort to fill * random data into the buffer. There is not much we can do about * that case, except making sure that it does not happen in the * first place. */ - nm_utils_random_bytes(out_uuid, sizeof(*out_uuid)); + nm_random_get_bytes(out_uuid, sizeof(*out_uuid)); /* Set the four most significant bits (bits 12 through 15) of the * time_hi_and_version field to the 4-bit version number from diff --git a/src/libnm-glib-aux/tests/test-shared-general.c b/src/libnm-glib-aux/tests/test-shared-general.c index a4817e1973..7d36d64a8c 100644 --- a/src/libnm-glib-aux/tests/test-shared-general.c +++ b/src/libnm-glib-aux/tests/test-shared-general.c @@ -94,7 +94,7 @@ test_nmhash(void) { int rnd; - nm_utils_random_bytes(&rnd, sizeof(rnd)); + nm_random_get_bytes(&rnd, sizeof(rnd)); g_assert(nm_hash_val(555, 4) != 0); } From 67a5cf76750f7e164a05f52db33c5832828e9d9b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Aug 2022 12:24:49 +0200 Subject: [PATCH 3/3] core: block to get good random bytes for "/var/lib/NetworkManager/secret_key" _host_id_read() is the only place where we really care to have good random numbers, because that is the secret key that we persist to disk. Previously, we tried only nm_random_get_bytes_full(), which is a best effort to get strong random numbers. If it fails to generate those, it would simply remember the generated key in memory and proceed, but not persist it to disk. nm_random_get_bytes_full() does not block waiting for good numbers. Change that. Now, first call nm_random_get_crypto_bytes(), which would block and try hard to get good random numbers. Only if that fails, fallback to nm_random_get_bytes_full() as before. The difference is of course only in early boot, when we might not yet have entropy. In that case, I think it's better for NetworkManager to block. --- src/core/nm-core-utils.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index fb85d75a8b..480e9b28f0 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -2814,7 +2814,10 @@ _host_id_read(guint8 **out_host_id, gsize *out_host_id_len) int base64_save = 0; gsize len; - nm_random_get_bytes_full(rnd_buf, sizeof(rnd_buf), &success); + if (nm_random_get_crypto_bytes(rnd_buf, sizeof(rnd_buf)) < 0) + nm_random_get_bytes_full(rnd_buf, sizeof(rnd_buf), &success); + else + success = TRUE; /* Our key is really binary data. But since we anyway generate a random seed * (with 32 random bytes), don't write it in binary, but instead create