colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

all: use nm_random_*() instead of g_random_*()

Browse Source 
g_random_*() is based on GRand, which is not a CSPRNG. Instead, rely on
kernel to give us good random numbers, which is what nm_random_*() does.

Note that nm_random_*() calls getrandom() (or reads /dev/urandom), which
most likely is slower than GRand. It doesn't matter for our uses though.

It is cumbersome to review all uses of g_rand_*() whether their usage of
a non-cryptographically secure generator is appropriate. Instead, just
always use an appropriate function, thereby avoiding this question. Even
glib documentation refers to reading "/dev/urandom" as alternative. Which
is what nm_random_*() does. These days, it seems unnecessary to not use
the best random generator available, unless it's not fast enough or you
need a stable/seedable stream of random numbers.

In particular in nmcli, we used g_random_int_range() to generate
passwords. That is not appropriate. Sure, it's *only* for the hotspot,
but still.
This commit is contained in:
Thomas Haller
2023-01-27 12:52:20 +01:00
parent fb1d2da979
commit 6e96d71731
4 changed files with 22 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
src/core/ndisc/nm-ndisc.c
View File

@@ -10,6 +10,7 @@
#include <arpa/inet.h>
#include <stdlib.h>
#include "libnm-glib-aux/nm-random-utils.h"
#include "libnm-platform/nm-platform-utils.h"
#include "libnm-platform/nm-platform.h"
#include "libnm-platform/nmp-netns.h"
@@ -858,7 +859,7 @@ solicit_retransmit_time_jitter(gint32 solicit_retransmit_time_msec)
ten_percent = NM_MAX(1, solicit_retransmit_time_msec / 10);
return solicit_retransmit_time_msec - ten_percent
+ ((gint32) (g_random_int() % (2u * ((guint32) ten_percent))));
+ ((gint32) (nm_random_u32() % (2u * ((guint32) ten_percent))));
}
static gboolean
@@ -936,7 +937,7 @@ solicit_timer_start(NMNDisc *ndisc)
* a suitable delay in 2021. Wait only up to 250 msec instead. */
delay_msec =
g_random_int() % ((guint32) (NM_NDISC_RFC4861_MAX_RTR_SOLICITATION_DELAY * 1000 / 4));
nm_random_u32() % ((guint32) (NM_NDISC_RFC4861_MAX_RTR_SOLICITATION_DELAY * 1000 / 4));
_LOGD("solicit: schedule sending first solicitation (of %d) in %.3f seconds",
priv->config.router_solicitations,
@@ -974,8 +975,9 @@ announce_router(NMNDisc *ndisc)
/* Schedule next initial announcement retransmit. */
priv->send_ra_id =
g_timeout_add_seconds(g_random_int_range(NM_NDISC_ROUTER_ADVERT_DELAY,
NM_NDISC_ROUTER_ADVERT_INITIAL_INTERVAL),
g_timeout_add_seconds(nm_random_u64_range_full(NM_NDISC_ROUTER_ADVERT_DELAY,
NM_NDISC_ROUTER_ADVERT_INITIAL_INTERVAL,
FALSE),
(GSourceFunc) announce_router,
ndisc);
} else {
@@ -1009,10 +1011,9 @@ announce_router_initial(NMNDisc *ndisc)
/* Schedule the initial send rather early. Clamp the delay by minimal
* delay and not the initial advert internal so that we start fast. */
if (G_LIKELY(!priv->send_ra_id)) {
priv->send_ra_id =
g_timeout_add_seconds(g_random_int_range(0, NM_NDISC_ROUTER_ADVERT_DELAY),
(GSourceFunc) announce_router,
ndisc);
priv->send_ra_id = g_timeout_add_seconds(nm_random_u64_range(NM_NDISC_ROUTER_ADVERT_DELAY),
(GSourceFunc) announce_router,
ndisc);
}
}
@@ -1028,7 +1029,7 @@ announce_router_solicited(NMNDisc *ndisc)
nm_clear_g_source(&priv->send_ra_id);
if (!priv->send_ra_id) {
priv->send_ra_id = g_timeout_add(g_random_int_range(0, NM_NDISC_ROUTER_ADVERT_DELAY_MS),
priv->send_ra_id = g_timeout_add(nm_random_u64_range(NM_NDISC_ROUTER_ADVERT_DELAY_MS),
(GSourceFunc) announce_router,
ndisc);
}

4
src/core/nm-core-utils.c
View File

@@ -3673,9 +3673,7 @@ _hw_addr_eth_complete(struct ether_addr *addr,
nm_assert((ouis == NULL) ^ (ouis_len != 0));
if (ouis) {
/* g_random_int() is good enough here. It uses a static GRand instance
* that is seeded from /dev/urandom. */
oui = ouis[g_random_int() % ouis_len];
oui = ouis[nm_random_u64_range(ouis_len)];
g_free(ouis);
} else {
if (!nm_utils_hwaddr_aton(current_mac_address, &oui, ETH_ALEN))

4
src/libnm-client-aux-extern/nm-libnm-aux.c
View File

@@ -4,6 +4,8 @@
#include "nm-libnm-aux.h"
#include "libnm-glib-aux/nm-random-utils.h"
/*****************************************************************************/
NMClient *
@@ -101,7 +103,7 @@ nmc_client_new_waitsync(GCancellable *cancellable,
* code no longer uses that, we hardly test those code paths. But they should
* work just the same. Randomly use instead the sync initialization in a debug
* build... */
if ((g_random_int() % 2) == 0) {
if (nm_random_bool()) {
gboolean success;
va_start(ap, first_property_name);

13
src/nmcli/devices.c
View File

@@ -17,6 +17,7 @@
#include <linux/if_ether.h>
#include "libnm-glib-aux/nm-secret-utils.h"
#include "libnm-glib-aux/nm-random-utils.h"
#include "common.h"
#include "connections.h"
#include "libnmc-base/nm-client-utils.h"
@@ -4098,10 +4099,11 @@ generate_wpa_key(char *key, size_t len)
/* generate a 8-chars ASCII WPA key */
for (i = 0; i < WPA_PASSKEY_SIZE; i++) {
int c;
c = g_random_int_range(33, 126);
/* too many non alphanumeric characters are hard to remember for humans */
while (!g_ascii_isalnum(c))
c = g_random_int_range(33, 126);
do {
c = nm_random_u64_range_full(33, 126, TRUE);
/* too many non alphanumeric characters are hard to remember for humans */
} while (g_ascii_isalnum(c));
key[i] = (char) c;
}
@@ -4120,7 +4122,8 @@ generate_wep_key(char *key, size_t len)
/* generate a 10-digit hex WEP key */
for (i = 0; i < 10; i++) {
int digit;
digit = g_random_int_range(0, 16);
digit = nm_random_u64_range_full(0, 16, TRUE);
key[i] = hexdigits[digit];
}
key[10] = '\0';