From 373684fdc09c508982eff8c7d3fee88ffa1f9627 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Oct 2017 10:45:15 +0200 Subject: [PATCH 1/6] shared: add _nm_thread_local macro Copied and adjusted from systemd. --- shared/nm-utils/nm-macros-internal.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index f511c142b8..435cef6568 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -32,6 +32,22 @@ #define _nm_const __attribute__ ((const)) #define _nm_printf(a,b) __attribute__ ((__format__ (__printf__, a, b))) +/*****************************************************************************/ + +#ifdef thread_local +#define _nm_thread_local thread_local +/* + * Don't break on glibc < 2.16 that doesn't define __STDC_NO_THREADS__ + * see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53769 + */ +#elif __STDC_VERSION__ >= 201112L && !(defined(__STDC_NO_THREADS__) || (defined(__GNU_LIBRARY__) && __GLIBC__ == 2 && __GLIBC_MINOR__ < 16)) +#define _nm_thread_local _Thread_local +#else +#define _nm_thread_local __thread +#endif + +/*****************************************************************************/ + #include "nm-glib.h" /*****************************************************************************/ From 93ea7a5905fa6be0b94275a16dba6ac5a8dfc569 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Oct 2017 11:02:25 +0200 Subject: [PATCH 2/6] shared: move nm_utils_fd_*() from src/ to shared/nm-utils/ The functions are general purpose and independent from NetworkManager core. Move them to "shared/nm-utils/" so they can be used independently. --- shared/nm-utils/nm-shared-utils.c | 97 +++++++++++++++++++++++++++++++ shared/nm-utils/nm-shared-utils.h | 12 ++++ src/nm-core-utils.c | 94 ------------------------------ src/nm-core-utils.h | 8 --- 4 files changed, 109 insertions(+), 102 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index ad727c4a77..ce45f9140a 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -25,6 +25,7 @@ #include #include +#include /*****************************************************************************/ @@ -992,3 +993,99 @@ nm_utils_str_utf8safe_escape_take (char *str, NMUtilsStrUtf8SafeFlags flags) } return str; } + +/*****************************************************************************/ + +/* taken from systemd's fd_wait_for_event(). Note that the timeout + * is here in nano-seconds, not micro-seconds. */ +int +nm_utils_fd_wait_for_event (int fd, int event, gint64 timeout_ns) +{ + struct pollfd pollfd = { + .fd = fd, + .events = event, + }; + struct timespec ts, *pts; + int r; + + if (timeout_ns < 0) + pts = NULL; + else { + ts.tv_sec = (time_t) (timeout_ns / NM_UTILS_NS_PER_SECOND); + ts.tv_nsec = (long int) (timeout_ns % NM_UTILS_NS_PER_SECOND); + pts = &ts; + } + + r = ppoll (&pollfd, 1, pts, NULL); + if (r < 0) + return -errno; + if (r == 0) + return 0; + return pollfd.revents; +} + +/* taken from systemd's loop_read() */ +ssize_t +nm_utils_fd_read_loop (int fd, void *buf, size_t nbytes, bool do_poll) +{ + uint8_t *p = buf; + ssize_t n = 0; + + g_return_val_if_fail (fd >= 0, -EINVAL); + g_return_val_if_fail (buf, -EINVAL); + + /* If called with nbytes == 0, let's call read() at least + * once, to validate the operation */ + + if (nbytes > (size_t) SSIZE_MAX) + return -EINVAL; + + do { + ssize_t k; + + k = read (fd, p, nbytes); + if (k < 0) { + if (errno == EINTR) + continue; + + if (errno == EAGAIN && do_poll) { + + /* We knowingly ignore any return value here, + * and expect that any error/EOF is reported + * via read() */ + + (void) nm_utils_fd_wait_for_event (fd, POLLIN, -1); + continue; + } + + return n > 0 ? n : -errno; + } + + if (k == 0) + return n; + + g_assert ((size_t) k <= nbytes); + + p += k; + nbytes -= k; + n += k; + } while (nbytes > 0); + + return n; +} + +/* taken from systemd's loop_read_exact() */ +int +nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll) +{ + ssize_t n; + + n = nm_utils_fd_read_loop (fd, buf, nbytes, do_poll); + if (n < 0) + return (int) n; + if ((size_t) n != nbytes) + return -EIO; + + return 0; +} + diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 32176da54d..96aab5341e 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -394,4 +394,16 @@ char *nm_utils_str_utf8safe_escape_take (char *str, NMUtilsStrUtf8SafeFlags flag /*****************************************************************************/ +#define NM_UTILS_NS_PER_SECOND ((gint64) 1000000000) +#define NM_UTILS_NS_PER_MSEC ((gint64) 1000000) +#define NM_UTILS_NS_TO_MSEC_CEIL(nsec) (((nsec) + (NM_UTILS_NS_PER_MSEC - 1)) / NM_UTILS_NS_PER_MSEC) + +/*****************************************************************************/ + +int nm_utils_fd_wait_for_event (int fd, int event, gint64 timeout_ns); +ssize_t nm_utils_fd_read_loop (int fd, void *buf, size_t nbytes, bool do_poll); +int nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll); + +/*****************************************************************************/ + #endif /* __NM_SHARED_UTILS_H__ */ diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 76cc8eb1bc..65687482de 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -2699,99 +2698,6 @@ nm_utils_machine_id_read (void) /*****************************************************************************/ -/* taken from systemd's fd_wait_for_event(). Note that the timeout - * is here in nano-seconds, not micro-seconds. */ -int -nm_utils_fd_wait_for_event (int fd, int event, gint64 timeout_ns) -{ - struct pollfd pollfd = { - .fd = fd, - .events = event, - }; - struct timespec ts, *pts; - int r; - - if (timeout_ns < 0) - pts = NULL; - else { - ts.tv_sec = (time_t) (timeout_ns / NM_UTILS_NS_PER_SECOND); - ts.tv_nsec = (long int) (timeout_ns % NM_UTILS_NS_PER_SECOND); - pts = &ts; - } - - r = ppoll (&pollfd, 1, pts, NULL); - if (r < 0) - return -errno; - if (r == 0) - return 0; - return pollfd.revents; -} - -/* taken from systemd's loop_read() */ -ssize_t -nm_utils_fd_read_loop (int fd, void *buf, size_t nbytes, bool do_poll) -{ - uint8_t *p = buf; - ssize_t n = 0; - - g_return_val_if_fail (fd >= 0, -EINVAL); - g_return_val_if_fail (buf, -EINVAL); - - /* If called with nbytes == 0, let's call read() at least - * once, to validate the operation */ - - if (nbytes > (size_t) SSIZE_MAX) - return -EINVAL; - - do { - ssize_t k; - - k = read (fd, p, nbytes); - if (k < 0) { - if (errno == EINTR) - continue; - - if (errno == EAGAIN && do_poll) { - - /* We knowingly ignore any return value here, - * and expect that any error/EOF is reported - * via read() */ - - (void) nm_utils_fd_wait_for_event (fd, POLLIN, -1); - continue; - } - - return n > 0 ? n : -errno; - } - - if (k == 0) - return n; - - g_assert ((size_t) k <= nbytes); - - p += k; - nbytes -= k; - n += k; - } while (nbytes > 0); - - return n; -} - -/* taken from systemd's loop_read_exact() */ -int -nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll) -{ - ssize_t n; - - n = nm_utils_fd_read_loop (fd, buf, nbytes, do_poll); - if (n < 0) - return (int) n; - if ((size_t) n != nbytes) - return -EIO; - - return 0; -} - _nm_printf (3, 4) static int _get_contents_error (GError **error, int errsv, const char *format, ...) diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index 067220b54e..4dff0176c2 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -240,10 +240,6 @@ int nm_utils_cmp_connection_by_autoconnect_priority (NMConnection *a, NMConnecti void nm_utils_log_connection_diff (NMConnection *connection, NMConnection *diff_base, guint32 level, guint64 domain, const char *name, const char *prefix); -#define NM_UTILS_NS_PER_SECOND ((gint64) 1000000000) -#define NM_UTILS_NS_PER_MSEC ((gint64) 1000000) -#define NM_UTILS_NS_TO_MSEC_CEIL(nsec) (((nsec) + (NM_UTILS_NS_PER_MSEC - 1)) / NM_UTILS_NS_PER_MSEC) - gint64 nm_utils_get_monotonic_timestamp_ns (void); gint64 nm_utils_get_monotonic_timestamp_us (void); gint64 nm_utils_get_monotonic_timestamp_ms (void); @@ -257,10 +253,6 @@ const char *nm_utils_ip4_property_path (const char *ifname, const char *property gboolean nm_utils_is_specific_hostname (const char *name); -int nm_utils_fd_wait_for_event (int fd, int event, gint64 timeout_ns); -ssize_t nm_utils_fd_read_loop (int fd, void *buf, size_t nbytes, bool do_poll); -int nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll); - int nm_utils_fd_get_contents (int fd, gsize max_length, char **contents, From b01a453ca29850cb70e519c2e254226a05c44ed2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 12 Oct 2017 15:43:21 +0200 Subject: [PATCH 3/6] core: add nm_utils_random_bytes() and use getrandom() Add a new function nm_utils_random_bytes(). This function now preferably uses getrandom() syscall if it is available. As fallback, it always tries to fill the buffer from /dev/urandom. If it cannot, as last fallback it uses GRand, which cannot fail. Hence, the function always sets some (pseudo) random bytes. It also returns FALSE if the obtained bytes are possibly not good randomness. --- shared/nm-utils/nm-shared-utils.c | 137 ++++++++++++++++++++++++++++++ shared/nm-utils/nm-shared-utils.h | 4 + src/nm-core-utils.c | 36 +------- src/nm-core-utils.h | 2 - 4 files changed, 145 insertions(+), 34 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index ce45f9140a..d2c057c48c 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -26,6 +26,13 @@ #include #include #include +#include + +#if USE_SYS_RANDOM_H +#include +#else +#include +#endif /*****************************************************************************/ @@ -1089,3 +1096,133 @@ nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll) return 0; } +/*****************************************************************************/ + +/** + * nm_utils_random_bytes: + * @p: the buffer to fill + * @n: the number of bytes to write to @p. + * + * 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 + * entroy (at early boot), the function will read /dev/urandom. + * Which of course, still has low entropy, and cause kernel to log + * a warning. + */ +gboolean +nm_utils_random_bytes (void *p, size_t n) +{ + int fd; + int r; + gboolean has_high_quality = TRUE; + gboolean urandom_success; + guint8 *buf = p; + gboolean avoid_urandom = FALSE; + + g_return_val_if_fail (p, FALSE); + g_return_val_if_fail (n > 0, FALSE); + +#if HAVE_GETRANDOM + { + static gboolean have_syscall = TRUE; + + if (have_syscall) { + r = getrandom (buf, n, GRND_NONBLOCK); + if (r > 0) { + if ((size_t) r == n) + return TRUE; + + /* no or partial read. There is not enough entropy. + * Fill the rest reading from urandom, and remember that + * some bits are not hight quality. */ + nm_assert (r < n); + buf += r; + n -= r; + has_high_quality = FALSE; + + /* 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. + * + * We use our fallback via GRand. Note that g_rand_new() also + * tries to seed itself with data from /dev/urandom, but since + * we reuse the instance, it shouldn't matter. */ + avoid_urandom = TRUE; + } else { + if (errno == ENOSYS) { + /* no support for getrandom(). We don't know whether + * we urandom will give us good quality. Assume yes. */ + have_syscall = FALSE; + } else { + /* unknown error. We'll read urandom below, but we don't have + * high-quality randomness. */ + has_high_quality = FALSE; + } + } + } + } +#endif + + urandom_success = FALSE; + if (!avoid_urandom) { +fd_open: + fd = open ("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOCTTY); + if (fd < 0) { + r = errno; + if (r == EINTR) + goto fd_open; + } else { + r = nm_utils_fd_read_loop_exact (fd, buf, n, TRUE); + close (fd); + if (r >= 0) + urandom_success = TRUE; + } + } + + if (!urandom_success) { + static _nm_thread_local GRand *rand = NULL; + gsize i; + int j; + + /* we failed to fill the bytes reading from urandom. + * Fill the bits using GRand pseudo random numbers. + * + * We don't have good quality. + */ + has_high_quality = FALSE; + + if (G_UNLIKELY (!rand)) + rand = g_rand_new (); + + nm_assert (n > 0); + i = 0; + for (;;) { + const union { + guint32 v32; + guint8 v8[4]; + } v = { + .v32 = g_rand_int (rand), + }; + + for (j = 0; j < 4; ) { + buf[i++] = v.v8[j++]; + if (i >= n) + goto done; + } + } +done: + ; + } + + return has_high_quality; +} diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 96aab5341e..e7cfec0817 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -406,4 +406,8 @@ int nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll) /*****************************************************************************/ +gboolean nm_utils_random_bytes (void *p, size_t n); + +/*****************************************************************************/ + #endif /* __NM_SHARED_UTILS_H__ */ diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 65687482de..42bd47c1ba 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2938,30 +2938,6 @@ nm_utils_file_get_contents (int dirfd, /*****************************************************************************/ -/* taken from systemd's dev_urandom(). */ -int -nm_utils_read_urandom (void *p, size_t nbytes) -{ - int fd = -1; - int r; - -again: - fd = open ("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOCTTY); - if (fd < 0) { - r = errno; - if (r == EINTR) - goto again; - return r == ENOENT ? -ENOSYS : -r; - } - - r = nm_utils_fd_read_loop_exact (fd, p, nbytes, TRUE); - close (fd); - - return r; -} - -/*****************************************************************************/ - guint8 * nm_utils_secret_key_read (gsize *out_key_len, GError **error) { @@ -2980,7 +2956,6 @@ nm_utils_secret_key_read (gsize *out_key_len, GError **error) key_len = 0; } } else { - int r; mode_t key_mask; /* RFC7217 mandates the key SHOULD be at least 128 bits. @@ -2988,10 +2963,9 @@ nm_utils_secret_key_read (gsize *out_key_len, GError **error) key_len = 32; secret_key = g_malloc (key_len); - r = nm_utils_read_urandom (secret_key, key_len); - if (r < 0) { + if (!nm_utils_random_bytes (secret_key, key_len)) { g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, - "Can't read /dev/urandom: %s", strerror (-r)); + "Can't get random data to generate secret key"); key_len = 0; goto out; } @@ -3245,8 +3219,7 @@ nm_utils_stable_id_random (void) { char buf[15]; - if (nm_utils_read_urandom (buf, sizeof (buf)) < 0) - g_return_val_if_reached (nm_utils_uuid_generate ()); + nm_utils_random_bytes (buf, sizeof (buf)); return g_base64_encode ((guchar *) buf, sizeof (buf)); } @@ -3630,8 +3603,7 @@ nm_utils_hw_addr_gen_random_eth (const char *current_mac_address, { struct ether_addr bin_addr; - if (nm_utils_read_urandom (&bin_addr, ETH_ALEN) < 0) - return NULL; + nm_utils_random_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/nm-core-utils.h b/src/nm-core-utils.h index 4dff0176c2..9d35e2f62d 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -272,8 +272,6 @@ gboolean nm_utils_file_set_contents (const gchar *filename, mode_t mode, GError **error); -int nm_utils_read_urandom (void *p, size_t n); - char *nm_utils_machine_id_read (void); gboolean nm_utils_machine_id_parse (const char *id_str, /*uuid_t*/ guchar *out_uuid); From fb6fecc036f466f569a7d05ed0d499164e032f15 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 12 Oct 2017 16:05:40 +0200 Subject: [PATCH 4/6] dhcp: use nm_utils_random_bytes() for generating random DUID --- src/dhcp/nm-dhcp-client.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index b9d4ce72fe..59845c8c7a 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -481,8 +481,6 @@ generate_duid_from_machine_id (void) gsize sumlen = sizeof (buffer); const guint16 duid_type = g_htons (4); uuid_t uuid; - GRand *generator; - guint i; gs_free char *machine_id_s = NULL; gs_free char *str = NULL; @@ -498,10 +496,7 @@ generate_duid_from_machine_id (void) "or " LOCALSTATEDIR "/lib/dbus/machine-id to generate " "DHCPv6 DUID; creating non-persistent random DUID."); - generator = g_rand_new (); - for (i = 0; i < sizeof (buffer) / sizeof (guint32); i++) - ((guint32 *) buffer)[i] = g_rand_int (generator); - g_rand_free (generator); + nm_utils_random_bytes (buffer, sizeof (buffer)); } /* Generate a DHCP Unique Identifier for DHCPv6 using the From 4a2798434ef162b31a129cb6a857c950ec992f3e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Oct 2017 11:51:18 +0200 Subject: [PATCH 5/6] core: introduce NM_HASH_INIT() to initialize hash seed Introduce a NM_HASH_INIT() function. It makes the places where we initialize a hash with a certain seed visually clear. Also, move them from "shared/nm-utils/nm-shared-utils.h" to "shared/nm-utils/nm-macros-internal.h". We might want to have NM_HASH_INIT() non-inline (hence, define it in the source file). --- libnm-core/nm-utils.c | 2 +- shared/nm-utils/nm-dedup-multi.c | 2 +- shared/nm-utils/nm-macros-internal.h | 15 --------- shared/nm-utils/nm-shared-utils.h | 28 +++++++++++++++++ src/devices/nm-device.c | 2 +- src/devices/nm-lldp-listener.c | 2 +- src/nm-core-utils.c | 2 +- src/nm-session-monitor.c | 6 ++-- src/platform/nm-platform.c | 30 +++++++++--------- src/platform/nmp-object.c | 46 ++++++++++++++++------------ 10 files changed, 78 insertions(+), 57 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 4a46681c47..5b8af9b205 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -4006,7 +4006,7 @@ _nm_utils_strstrdictkey_hash (gconstpointer a) { const NMUtilsStrStrDictKey *k = a; const signed char *p; - guint32 h = 5381; + guint32 h = NM_HASH_INIT (76642997u); if (k) { if (((int) k->type) & ~STRSTRDICTKEY_ALL_SET) diff --git a/shared/nm-utils/nm-dedup-multi.c b/shared/nm-utils/nm-dedup-multi.c index a1b6da114d..e7616074b0 100644 --- a/shared/nm-utils/nm-dedup-multi.c +++ b/shared/nm-utils/nm-dedup-multi.c @@ -182,7 +182,7 @@ _dict_idx_entries_hash (const NMDedupMultiEntry *entry) nm_assert (obj); h = idx_type->klass->idx_obj_partition_hash (idx_type, obj); } else - h = 1914869417; + h = NM_HASH_INIT (1914869417u); if (!lookup_head) h = NM_HASH_COMBINE (h, idx_type->klass->idx_obj_id_hash (idx_type, obj)); diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 435cef6568..6b70693258 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -426,21 +426,6 @@ _NM_IN_STRSET_streq (const char *x, const char *s) /*****************************************************************************/ -static inline guint -NM_HASH_COMBINE (guint h, guint val) -{ - /* see g_str_hash() for reasons */ - return (h << 5) + h + val; -} - -static inline guint -NM_HASH_COMBINE_UINT64 (guint h, guint64 val) -{ - return NM_HASH_COMBINE (h, (((guint) val) & 0xFFFFFFFFu) + ((guint) (val >> 32))); -} - -/*****************************************************************************/ - /* NM_CACHED_QUARK() returns the GQuark for @string, but caches * it in a static variable to speed up future lookups. * diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index e7cfec0817..57f1619245 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -378,6 +378,34 @@ GParamSpec *nm_g_object_class_find_property_from_gtype (GType gtype, /*****************************************************************************/ +static inline guint +NM_HASH_INIT (guint seed) +{ + return seed; +} + +static inline guint +NM_HASH_COMBINE (guint h, guint val) +{ + /* see g_str_hash() for reasons */ + return (h << 5) + h + val; +} + +static inline guint +NM_HASH_COMBINE_UINT64 (guint h, guint64 val) +{ + return NM_HASH_COMBINE (h, (((guint) val) & 0xFFFFFFFFu) + ((guint) (val >> 32))); +} + +static inline guint +NM_HASH_POINTER (gconstpointer ptr) +{ + /* same as g_direct_hash(), but inline. */ + return GPOINTER_TO_UINT (ptr); +} + +/*****************************************************************************/ + typedef enum { NM_UTILS_STR_UTF8_SAFE_FLAG_NONE = 0, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL = 0x0001, diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 3000adb32f..c8ec439e5a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -2858,7 +2858,7 @@ typedef struct { static guint _v4_has_shadowed_routes_detect_hash (const IP4RPFilterData *d) { - guint h = 0; + guint h = NM_HASH_INIT (1105201169u); h = NM_HASH_COMBINE (h, d->network); h = NM_HASH_COMBINE (h, d->plen); diff --git a/src/devices/nm-lldp-listener.c b/src/devices/nm-lldp-listener.c index 8f12fd8595..28e7fd6c77 100644 --- a/src/devices/nm-lldp-listener.c +++ b/src/devices/nm-lldp-listener.c @@ -274,7 +274,7 @@ static guint lldp_neighbor_id_hash (gconstpointer ptr) { const LldpNeighbor *neigh = ptr; - guint hash = 23423423u; + guint hash = NM_HASH_INIT (23423423u); hash = NM_HASH_COMBINE (hash, neigh->chassis_id ? g_str_hash (neigh->chassis_id) : 12321u); hash = NM_HASH_COMBINE (hash, neigh->port_id ? g_str_hash (neigh->port_id) : 34342343u); diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 42bd47c1ba..48a4aa25db 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -188,7 +188,7 @@ nm_utils_exp10 (gint16 ex) guint nm_utils_in6_addr_hash (const struct in6_addr *addr) { - guint hash = (guint) 0x897da53981a13ULL; + guint hash = NM_HASH_INIT (3675559913u); int i; for (i = 0; i < sizeof (*addr); i++) diff --git a/src/nm-session-monitor.c b/src/nm-session-monitor.c index 151deec899..20781bd45d 100644 --- a/src/nm-session-monitor.c +++ b/src/nm-session-monitor.c @@ -260,9 +260,9 @@ ck_init (NMSessionMonitor *monitor) if ((monitor->ck.monitor = g_file_monitor_file (file, G_FILE_MONITOR_NONE, NULL, &error))) { monitor->ck.cache = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_free); g_signal_connect (monitor->ck.monitor, - "changed", - G_CALLBACK (ck_changed), - monitor); + "changed", + G_CALLBACK (ck_changed), + monitor); } else { _LOGE ("error monitoring " CKDB_PATH ": %s", error->message); g_clear_error (&error); diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index c04d4f85d3..eedd2d8243 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -5126,7 +5126,7 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi guint nm_platform_link_hash (const NMPlatformLink *obj) { - guint h = 99413953; + guint h = NM_HASH_INIT (99413953u); guint8 i8; h = NM_HASH_COMBINE (h, obj->ifindex); @@ -5187,7 +5187,7 @@ nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) guint nm_platform_lnk_gre_hash (const NMPlatformLnkGre *obj) { - guint h = 1887023311; + guint h = NM_HASH_INIT (1887023311u); h = NM_HASH_COMBINE (h, obj->parent_ifindex); h = NM_HASH_COMBINE (h, obj->input_flags); @@ -5222,7 +5222,7 @@ nm_platform_lnk_gre_cmp (const NMPlatformLnkGre *a, const NMPlatformLnkGre *b) guint nm_platform_lnk_infiniband_hash (const NMPlatformLnkInfiniband *obj) { - guint h = 1748638583; + guint h = NM_HASH_INIT (1748638583u); h = NM_HASH_COMBINE (h, obj->p_key); if (obj->mode) @@ -5242,7 +5242,7 @@ nm_platform_lnk_infiniband_cmp (const NMPlatformLnkInfiniband *a, const NMPlatfo guint nm_platform_lnk_ip6tnl_hash (const NMPlatformLnkIp6Tnl *obj) { - guint h = 1651660009; + guint h = NM_HASH_INIT (1651660009u); h = NM_HASH_COMBINE (h, obj->parent_ifindex); h = NM_HASH_COMBINE_IN6ADDR (h, &obj->local); @@ -5273,7 +5273,7 @@ nm_platform_lnk_ip6tnl_cmp (const NMPlatformLnkIp6Tnl *a, const NMPlatformLnkIp6 guint nm_platform_lnk_ipip_hash (const NMPlatformLnkIpIp *obj) { - guint h = 861934429; + guint h = NM_HASH_INIT (861934429u); h = NM_HASH_COMBINE (h, obj->parent_ifindex); h = NM_HASH_COMBINE (h, obj->local); @@ -5300,7 +5300,7 @@ nm_platform_lnk_ipip_cmp (const NMPlatformLnkIpIp *a, const NMPlatformLnkIpIp *b guint nm_platform_lnk_macsec_hash (const NMPlatformLnkMacsec *obj) { - guint h = 226984267; + guint h = NM_HASH_INIT (226984267u); h = NM_HASH_COMBINE (h, obj->sci); h = NM_HASH_COMBINE_UINT64 (h, obj->icv_length); @@ -5339,7 +5339,7 @@ nm_platform_lnk_macsec_cmp (const NMPlatformLnkMacsec *a, const NMPlatformLnkMac guint nm_platform_lnk_macvlan_hash (const NMPlatformLnkMacvlan *obj) { - guint h = 771014989; + guint h = NM_HASH_INIT (771014989u); h = NM_HASH_COMBINE (h, obj->mode); h = NM_HASH_COMBINE (h, obj->no_promisc); @@ -5360,7 +5360,7 @@ nm_platform_lnk_macvlan_cmp (const NMPlatformLnkMacvlan *a, const NMPlatformLnkM guint nm_platform_lnk_sit_hash (const NMPlatformLnkSit *obj) { - guint h = 1690154969; + guint h = NM_HASH_INIT (1690154969u); h = NM_HASH_COMBINE (h, obj->parent_ifindex); h = NM_HASH_COMBINE (h, obj->local); @@ -5391,7 +5391,7 @@ nm_platform_lnk_sit_cmp (const NMPlatformLnkSit *a, const NMPlatformLnkSit *b) guint nm_platform_lnk_vlan_hash (const NMPlatformLnkVlan *obj) { - guint h = 58751383; + guint h = NM_HASH_INIT (58751383u); h = NM_HASH_COMBINE (h, obj->id); h = NM_HASH_COMBINE (h, obj->flags); @@ -5410,7 +5410,7 @@ nm_platform_lnk_vlan_cmp (const NMPlatformLnkVlan *a, const NMPlatformLnkVlan *b guint nm_platform_lnk_vxlan_hash (const NMPlatformLnkVxlan *obj) { - guint h = 461041297; + guint h = NM_HASH_INIT (461041297u); h = NM_HASH_COMBINE (h, obj->parent_ifindex); h = NM_HASH_COMBINE (h, obj->id); @@ -5461,7 +5461,7 @@ nm_platform_lnk_vxlan_cmp (const NMPlatformLnkVxlan *a, const NMPlatformLnkVxlan guint nm_platform_ip4_address_hash (const NMPlatformIP4Address *obj) { - guint h = 469681301; + guint h = NM_HASH_INIT (469681301u); if (obj) { h = NM_HASH_COMBINE (h, obj->ifindex); @@ -5498,7 +5498,7 @@ nm_platform_ip4_address_cmp (const NMPlatformIP4Address *a, const NMPlatformIP4A guint nm_platform_ip6_address_hash (const NMPlatformIP6Address *obj) { - guint h = 605908909; + guint h = NM_HASH_INIT (605908909u); if (obj) { h = NM_HASH_COMBINE (h, obj->ifindex); @@ -5537,8 +5537,9 @@ nm_platform_ip6_address_cmp (const NMPlatformIP6Address *a, const NMPlatformIP6A guint nm_platform_ip4_route_hash (const NMPlatformIP4Route *obj, NMPlatformIPRouteCmpType cmp_type) { - guint h = NM_HASH_COMBINE (1228913327, cmp_type); + guint h = NM_HASH_INIT (1228913327u); + h = NM_HASH_COMBINE (h, cmp_type); if (obj) { switch (cmp_type) { case NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID: @@ -5688,8 +5689,9 @@ nm_platform_ip4_route_cmp (const NMPlatformIP4Route *a, const NMPlatformIP4Route guint nm_platform_ip6_route_hash (const NMPlatformIP6Route *obj, NMPlatformIPRouteCmpType cmp_type) { - guint h = NM_HASH_COMBINE (1053326051, cmp_type); + guint h = NM_HASH_INIT (1053326051u); + h = NM_HASH_COMBINE (h, cmp_type); if (obj) { switch (cmp_type) { case NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID: diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index e58868645e..24651a66d0 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -150,7 +150,8 @@ _idx_obj_part (const DedupMultiIdxType *idx_type, if (obj_b) return NMP_OBJECT_GET_TYPE (obj_a) == NMP_OBJECT_GET_TYPE (obj_b); if (request_hash) { - h = (guint) idx_type->cache_id_type; + h = NM_HASH_INIT (487703243u); + h = NM_HASH_COMBINE (h, idx_type->cache_id_type); h = NM_HASH_COMBINE (h, NMP_OBJECT_GET_TYPE (obj_a)); return _HASH_NON_ZERO (h); } @@ -170,7 +171,8 @@ _idx_obj_part (const DedupMultiIdxType *idx_type, } if (request_hash) { /* we request a hash from obj_a. Hash the relevant parts. */ - h = (guint) idx_type->cache_id_type; + h = NM_HASH_INIT (2126752699u); + h = NM_HASH_COMBINE (h, idx_type->cache_id_type); h = NM_HASH_COMBINE (h, g_str_hash (obj_a->link.name)); return _HASH_NON_ZERO (h); } @@ -189,7 +191,8 @@ _idx_obj_part (const DedupMultiIdxType *idx_type, && nmp_object_is_visible (obj_b); } if (request_hash) { - h = (guint) idx_type->cache_id_type; + h = NM_HASH_INIT (4278960223u); + h = NM_HASH_COMBINE (h, idx_type->cache_id_type); h = NM_HASH_COMBINE (h, NMP_OBJECT_GET_TYPE (obj_a)); return _HASH_NON_ZERO (h); } @@ -209,7 +212,8 @@ _idx_obj_part (const DedupMultiIdxType *idx_type, && nmp_object_is_visible (obj_b); } if (request_hash) { - h = (guint) idx_type->cache_id_type; + h = NM_HASH_INIT (920415631u); + h = NM_HASH_COMBINE (h, idx_type->cache_id_type); h = NM_HASH_COMBINE (h, NMP_OBJECT_GET_TYPE (obj_a)); h = NM_HASH_COMBINE (h, obj_a->object.ifindex); return _HASH_NON_ZERO (h); @@ -230,7 +234,8 @@ _idx_obj_part (const DedupMultiIdxType *idx_type, : (nm_platform_ip6_route_cmp (&obj_a->ip6_route, &obj_b->ip6_route, NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID) == 0)); } if (request_hash) { - h = (guint) idx_type->cache_id_type; + h = NM_HASH_INIT (778646573u); + h = NM_HASH_COMBINE (h, idx_type->cache_id_type); if (obj_type == NMP_OBJECT_TYPE_IP4_ROUTE) h = NM_HASH_COMBINE (h, nm_platform_ip4_route_hash (&obj_a->ip4_route, NM_PLATFORM_IP_ROUTE_CMP_TYPE_WEAK_ID)); else @@ -300,7 +305,7 @@ static guint _vlan_xgress_qos_mappings_hash (guint n_map, const NMVlanQosMapping *map) { - guint h = 1453577309; + guint h = NM_HASH_INIT (1453577309u); guint i; for (i = 0; i < n_map; i++) { @@ -756,19 +761,20 @@ nmp_object_hash (const NMPObject *obj) klass = NMP_OBJECT_GET_CLASS (obj); + h = NM_HASH_INIT (816200863u); if (klass->cmd_obj_hash) - h = klass->cmd_obj_hash (obj); + h = NM_HASH_COMBINE (h, klass->cmd_obj_hash (obj)); else if (klass->cmd_plobj_hash) - h = klass->cmd_plobj_hash (&obj->object); + h = NM_HASH_COMBINE (h, klass->cmd_plobj_hash (&obj->object)); else - return GPOINTER_TO_UINT (obj); + return NM_HASH_POINTER (obj); return NM_HASH_COMBINE (h, klass->obj_type); } static guint _vt_cmd_obj_hash_link (const NMPObject *obj) { - guint h = 1228913327; + guint h = NM_HASH_INIT (3448776161u); nm_assert (NMP_OBJECT_GET_TYPE (obj) == NMP_OBJECT_TYPE_LINK); @@ -783,7 +789,7 @@ _vt_cmd_obj_hash_link (const NMPObject *obj) static guint _vt_cmd_obj_hash_lnk_vlan (const NMPObject *obj) { - guint h = 914932607; + guint h = NM_HASH_INIT (914932607u); nm_assert (NMP_OBJECT_GET_TYPE (obj) == NMP_OBJECT_TYPE_LNK_VLAN); @@ -1077,7 +1083,7 @@ nmp_object_id_hash (const NMPObject *obj) if (!klass->cmd_plobj_id_hash) { /* The klass doesn't implement ID compare. It means, to use pointer * equality (g_direct_hash). */ - return g_direct_hash (obj); + return NM_HASH_POINTER (obj); } return klass->cmd_plobj_id_hash (&obj->object); @@ -1093,29 +1099,29 @@ _vt_cmd_plobj_id_hash_##type (const NMPlatformObject *_obj) \ return hash; \ } _vt_cmd_plobj_id_hash (link, NMPlatformLink, { - hash = (guint) 3982791431u; - hash = hash + ((guint) obj->ifindex); + hash = NM_HASH_INIT (3982791431u); + hash = NM_HASH_COMBINE (hash, ((guint) obj->ifindex)); }) _vt_cmd_plobj_id_hash (ip4_address, NMPlatformIP4Address, { - hash = (guint) 3591309853u; - hash = hash + ((guint) obj->ifindex); + hash = NM_HASH_INIT (3591309853u); + hash = NM_HASH_COMBINE (hash, ((guint) obj->ifindex)); hash = NM_HASH_COMBINE (hash, obj->plen); hash = NM_HASH_COMBINE (hash, obj->address); /* for IPv4 we must also consider the net-part of the peer-address (IFA_ADDRESS) */ hash = NM_HASH_COMBINE (hash, (obj->peer_address & _nm_utils_ip4_prefix_to_netmask (obj->plen))); }) _vt_cmd_plobj_id_hash (ip6_address, NMPlatformIP6Address, { - hash = (guint) 2907861637u; - hash = hash + ((guint) obj->ifindex); + hash = NM_HASH_INIT (2907861637u); + hash = NM_HASH_COMBINE (hash, ((guint) obj->ifindex)); /* for IPv6 addresses, the prefix length is not part of the primary identifier. */ hash = NM_HASH_COMBINE (hash, nm_utils_in6_addr_hash (&obj->address)); }) _vt_cmd_plobj_id_hash (ip4_route, NMPlatformIP4Route, { - hash = (guint) 1038302471u; + hash = NM_HASH_INIT (1038302471u); hash = NM_HASH_COMBINE (hash, nm_platform_ip4_route_hash (obj, NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID)); }) _vt_cmd_plobj_id_hash (ip6_route, NMPlatformIP6Route, { - hash = (guint) 1233384151u; + hash = NM_HASH_INIT (1233384151u); hash = NM_HASH_COMBINE (hash, nm_platform_ip6_route_hash (obj, NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID)); }) From c978b9dfe57e68d92c6dd3afac565fe05d325be0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 13 Oct 2017 11:56:06 +0200 Subject: [PATCH 6/6] core: randomize hash seed with a global seed This makes hashing non-deterministic with the aim to make it harder to exploit hash collisions. Non-deterministic also means that for unit testing we will get different values on each run. But since we shall never assign any meaning to these hash values nor rely on them being stable between restarts (or upgrades), that doesn't hurt. --- shared/nm-utils/nm-shared-utils.c | 23 +++++++++++++++++++++++ shared/nm-utils/nm-shared-utils.h | 6 +----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index d2c057c48c..ba99ce2f87 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -863,6 +863,29 @@ nm_g_object_class_find_property_from_gtype (GType gtype, /*****************************************************************************/ +guint +NM_HASH_INIT (guint seed) +{ + static volatile guint global_seed = 0; + guint g, s; + + /* we xor @seed with a random @global_seed. This is to make the hashing behavior + * less predictable and harder to exploit collisions. */ + g = global_seed; + if (G_UNLIKELY (g == 0)) { + nm_utils_random_bytes (&s, sizeof (s)); + if (s == 0) + s = 42; + g_atomic_int_compare_and_exchange ((int *) &global_seed, 0, s); + g = global_seed; + nm_assert (g); + } + + return g ^ seed; +} + +/*****************************************************************************/ + static void _str_append_escape (GString *s, char ch) { diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 57f1619245..0f9df73fb3 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -378,11 +378,7 @@ GParamSpec *nm_g_object_class_find_property_from_gtype (GType gtype, /*****************************************************************************/ -static inline guint -NM_HASH_INIT (guint seed) -{ - return seed; -} +guint NM_HASH_INIT (guint seed); static inline guint NM_HASH_COMBINE (guint h, guint val)