From 0a4bbb18fe9fb80acf038177ae581b7aef2c7883 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 12 Dec 2018 12:11:53 +0100 Subject: [PATCH 1/5] shared: expose siphash24() related functionality in nm-hash-utils.h CSiphash is a first class citizen, it's fine to use everwhere where we need it. NMHash wraps CSiphash and provides three things: 1) Convenience macros that make hashing nicer to use. 2) it uses a randomly generated, per-run hash seed, that can be combined with a guint static seed. 3) it's a general API for hashing data. It nowhere promises that it actually uses siphash24, although currently it does everywhere. NMHash is not (officially) siphash24. Add API nm_hash_siphash42_init() and nm_hash_siphash42() to "nm-hash-utils.h", that exposes (2) for use with regular CSiphash. You of course no longer get the convenice macros (1) but you get plain siphash24 (which NMHash does not give (3)). While at it, also add a nm_hash_complete_u64(). Usually, for hasing we want guint types. But we don't need to hide the fact, that the underlying value is first uint64. Expose it. (cherry picked from commit db791db4e1ff7a850b4ac49d24d159e2ccbe005c) --- shared/nm-utils/nm-hash-utils.c | 6 ++-- shared/nm-utils/nm-hash-utils.h | 62 +++++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/shared/nm-utils/nm-hash-utils.c b/shared/nm-utils/nm-hash-utils.c index 9f164a119e..80387c7143 100644 --- a/shared/nm-utils/nm-hash-utils.c +++ b/shared/nm-utils/nm-hash-utils.c @@ -122,17 +122,17 @@ nm_hash_static (guint static_seed) } void -nm_hash_init (NMHashState *state, guint static_seed) +nm_hash_siphash42_init (CSipHash *h, guint static_seed) { const guint8 *g; guint seed[HASH_KEY_SIZE_GUINT]; - nm_assert (state); + nm_assert (h); g = _get_hash_key (); memcpy (seed, g, HASH_KEY_SIZE); seed[0] ^= static_seed; - c_siphash_init (&state->_state, (const guint8 *) seed); + c_siphash_init (h, (const guint8 *) seed); } guint diff --git a/shared/nm-utils/nm-hash-utils.h b/shared/nm-utils/nm-hash-utils.h index b797fb75af..cf71a7e9f2 100644 --- a/shared/nm-utils/nm-hash-utils.h +++ b/shared/nm-utils/nm-hash-utils.h @@ -25,6 +25,39 @@ #include "c-siphash/src/c-siphash.h" #include "nm-macros-internal.h" +/*****************************************************************************/ + +void nm_hash_siphash42_init (CSipHash *h, guint static_seed); + +/* Siphash24 of binary buffer @arr and @len, using the randomized seed from + * other NMHash functions. + * + * Note, that this is guaranteed to use siphash42 under the hood (contrary to + * all other NMHash API, which leave this undefined). That matters at the point, + * where the caller needs to be sure that a reasonably strong hasing algorithm + * is used. (Yes, NMHash is all about siphash24, but otherwise that is not promised + * anywhere). + * + * Another difference is, that this returns guint64 (not guint like other NMHash functions). + * + * Another difference is, that this may also return zero (not like nm_hash_complete()). + * + * Then, why not use c_siphash_hash() directly? Because this also uses the randomized, + * per-run hash-seed like nm_hash_init(). So, you get siphash24 with a random + * seed (which is cached for the current run of the program). + */ +static inline guint64 +nm_hash_siphash42 (guint static_seed, const void *ptr, gsize n) +{ + CSipHash h; + + nm_hash_siphash42_init (&h, static_seed); + c_siphash_append (&h, ptr, n); + return c_siphash_finalize (&h); +} + +/*****************************************************************************/ + struct _NMHashState { CSipHash _state; }; @@ -33,16 +66,33 @@ typedef struct _NMHashState NMHashState; guint nm_hash_static (guint static_seed); -void nm_hash_init (NMHashState *state, guint static_seed); +static inline void +nm_hash_init (NMHashState *state, guint static_seed) +{ + nm_assert (state); + + nm_hash_siphash42_init (&state->_state, static_seed); +} + +static inline guint64 +nm_hash_complete_u64 (NMHashState *state) +{ + nm_assert (state); + + /* this returns the native u64 hash value. Note that this differs + * from nm_hash_complete() in two ways: + * + * - the type, guint64 vs. guint. + * - nm_hash_complete() never returns zero. */ + return c_siphash_finalize (&state->_state); +} static inline guint nm_hash_complete (NMHashState *state) { guint64 h; - nm_assert (state); - - h = c_siphash_finalize (&state->_state); + h = nm_hash_complete_u64 (state); /* we don't ever want to return a zero hash. * @@ -218,8 +268,8 @@ guint nm_str_hash (gconstpointer str); ({ \ NMHashState _h; \ \ - nm_hash_init (&_h, static_seed); \ - nm_hash_update_val (&_h, val); \ + nm_hash_init (&_h, (static_seed)); \ + nm_hash_update_val (&_h, (val)); \ nm_hash_complete (&_h); \ }) From cdb7f6f6d2927aac501efba22557778301834d0e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 12 Dec 2018 10:10:38 +0100 Subject: [PATCH 2/5] core/trivial: rename secret-key to host-key Now that the secret-key is hashed with the machine-id, the name is no longer best. Sure, part of the key are persisted in /var/lib/NetworkManager/secret_key file, which the user is well advised to keep secret. But what nm_utils_secret_key_get() returns is first and foremost a binary key that is per-host and used for hashing a per-host component. It's really the "host-id". Compare that to what we also have, the "machine-id" and the "boot-id". Rename. (cherry picked from commit 6ffcd263177d01a528a89709609f06550ecded9b) --- src/devices/nm-device.c | 24 ++++---- src/nm-core-utils.c | 133 ++++++++++++++++++++++------------------ src/nm-core-utils.h | 14 ++--- 3 files changed, 93 insertions(+), 78 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index df1b28d39b..3df95b5329 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -7552,8 +7552,8 @@ dhcp4_get_client_id (NMDevice *self, NMUtilsStableType stable_type; const char *stable_id; guint32 salted_header; - const guint8 *secret_key; - gsize secret_key_len; + const guint8 *host_id; + gsize host_id_len; stable_id = _get_stable_id (self, connection, &stable_type); if (!stable_id) @@ -7561,12 +7561,12 @@ dhcp4_get_client_id (NMDevice *self, salted_header = htonl (2011610591 + stable_type); - nm_utils_secret_key_get (&secret_key, &secret_key_len); + nm_utils_host_id_get (&host_id, &host_id_len); sum = g_checksum_new (G_CHECKSUM_SHA1); g_checksum_update (sum, (const guchar *) &salted_header, sizeof (salted_header)); g_checksum_update (sum, (const guchar *) stable_id, strlen (stable_id) + 1); - g_checksum_update (sum, (const guchar *) secret_key, secret_key_len); + g_checksum_update (sum, (const guchar *) host_id, host_id_len); nm_utils_checksum_get_digest (sum, digest); client_id_buf = g_malloc (1 + 15); @@ -8341,7 +8341,7 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole } else { gint64 time; - time = nm_utils_secret_key_get_timestamp (); + time = nm_utils_host_id_get_timestamp (); if (!time) { duid_error = "cannot retrieve the secret key timestamp"; goto out_fail; @@ -8358,8 +8358,8 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole NMUtilsStableType stable_type; const char *stable_id = NULL; guint32 salted_header; - const guint8 *secret_key; - gsize secret_key_len; + const guint8 *host_id; + gsize host_id_len; union { guint8 sha256[NM_UTILS_CHECKSUM_LENGTH_SHA256]; guint8 hwaddr[ETH_ALEN]; @@ -8376,12 +8376,12 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole salted_header = htonl (670531087 + stable_type); - nm_utils_secret_key_get (&secret_key, &secret_key_len); + nm_utils_host_id_get (&host_id, &host_id_len); sum = g_checksum_new (G_CHECKSUM_SHA256); g_checksum_update (sum, (const guchar *) &salted_header, sizeof (salted_header)); g_checksum_update (sum, (const guchar *) stable_id, -1); - g_checksum_update (sum, (const guchar *) secret_key, secret_key_len); + g_checksum_update (sum, (const guchar *) host_id, host_id_len); nm_utils_checksum_get_digest (sum, digest.sha256); G_STATIC_ASSERT_EXPR (sizeof (digest) == sizeof (digest.sha256)); @@ -8393,11 +8393,11 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole #define EPOCH_DATETIME_THREE_YEARS (356 * 24 * 3600 * 3) - /* We want a variable time between the secret_key timestamp and three years + /* We want a variable time between the host_id timestamp and three years * before. Let's compute the time (in seconds) from 0 to 3 years; then we'll - * subtract it from the secret_key timestamp. + * subtract it from the host_id timestamp. */ - time = nm_utils_secret_key_get_timestamp (); + time = nm_utils_host_id_get_timestamp (); if (!time) { duid_error = "cannot retrieve the secret key timestamp"; goto out_fail; diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index c3d2e3c2a5..4ac68c2dad 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2657,7 +2657,7 @@ again: return NULL; } - if (nm_utils_secret_key_get (&seed_bin, &seed_len)) { + if (nm_utils_host_id_get (&seed_bin, &seed_len)) { /* we have no valid machine-id. Generate a fake one by hashing * the secret-key. This key is commonly persisted, so it should be * stable accross reboots (despite having a broken system without @@ -2728,9 +2728,9 @@ nm_utils_machine_id_is_fake (void) #define SECRET_KEY_FILE NMSTATEDIR"/secret_key" static const guint8 * -_secret_key_hash_v2 (const guint8 *seed_arr, - gsize seed_len, - guint8 *out_digest /* 32 bytes (NM_UTILS_CHECKSUM_LENGTH_SHA256) */) +_host_id_hash_v2 (const guint8 *seed_arr, + gsize seed_len, + guint8 *out_digest /* 32 bytes (NM_UTILS_CHECKSUM_LENGTH_SHA256) */) { nm_auto_free_checksum GChecksum *sum = g_checksum_new (G_CHECKSUM_SHA256); const UuidData *machine_id_data; @@ -2758,8 +2758,8 @@ _secret_key_hash_v2 (const guint8 *seed_arr, } static gboolean -_secret_key_read (guint8 **out_key, - gsize *out_key_len) +_host_id_read (guint8 **out_host_id, + gsize *out_host_id_len) { #define SECRET_KEY_LEN 32u guint8 sha256_digest[NM_UTILS_CHECKSUM_LENGTH_SHA256]; @@ -2797,7 +2797,7 @@ _secret_key_read (guint8 **out_key, * except that it seems simpler not to distinguish between the v2 prefix and the content. * It's all just part of the seed. */ - secret_arr = _secret_key_hash_v2 (file_content.bin, file_content.len, sha256_digest); + secret_arr = _host_id_hash_v2 (file_content.bin, file_content.len, sha256_digest); secret_len = NM_UTILS_CHECKSUM_LENGTH_SHA256; success = TRUE; goto out; @@ -2842,7 +2842,7 @@ _secret_key_read (guint8 **out_key, &base64_save); nm_assert (len <= sizeof (new_content)); - secret_arr = _secret_key_hash_v2 (new_content, len, sha256_digest); + secret_arr = _host_id_hash_v2 (new_content, len, sha256_digest); secret_len = NM_UTILS_CHECKSUM_LENGTH_SHA256; if (!success) @@ -2866,53 +2866,68 @@ _secret_key_read (guint8 **out_key, } out: - *out_key_len = secret_len; - *out_key = nm_memdup (secret_arr, secret_len); + *out_host_id_len = secret_len; + *out_host_id = nm_memdup (secret_arr, secret_len); return success; } typedef struct { - guint8 *secret_key; - gsize key_len; + guint8 *host_id; + gsize host_id_len; bool is_good:1; -} SecretKeyData; +} HostIdData; +/** + * nm_utils_host_id_get: + * @out_host_id: (out) (transfer none): the binary host key + * @out_host_id_len: the length of the host key. + * + * This returns a per-host key that depends on /var/lib/NetworkManage/secret_key + * and (depending on the version) on /etc/machine-id. If /var/lib/NetworkManage/secret_key + * does not exist, it will be generated and persisted for next boot. + * + * Returns: %TRUE, if the host key is "good". Note that this function + * will always succeed to return a host-key, and that this key + * won't change during the run of the program (no matter what). + * A %FALSE return possibly means, that the secret_key is not persisted + * to disk, and/or that it was generated with bad randomness. + */ gboolean -nm_utils_secret_key_get (const guint8 **out_secret_key, - gsize *out_key_len) +nm_utils_host_id_get (const guint8 **out_host_id, + gsize *out_host_id_len) { - static const SecretKeyData *volatile secret_key_static; - const SecretKeyData *secret_key; + static const HostIdData *volatile host_id_static; + const HostIdData *host_id; again: - secret_key = g_atomic_pointer_get (&secret_key_static); - if (G_UNLIKELY (!secret_key)) { - static SecretKeyData secret_key_data; + host_id = g_atomic_pointer_get (&host_id_static); + if (G_UNLIKELY (!host_id)) { + static HostIdData host_id_data; static gsize init_value = 0; if (!g_once_init_enter (&init_value)) goto again; - secret_key_data.is_good = _secret_key_read (&secret_key_data.secret_key, - &secret_key_data.key_len); - secret_key = &secret_key_data; - g_atomic_pointer_set (&secret_key_static, secret_key); + host_id_data.is_good = _host_id_read (&host_id_data.host_id, + &host_id_data.host_id_len); + host_id = &host_id_data; + g_atomic_pointer_set (&host_id_static, host_id); g_once_init_leave (&init_value, 1); } - *out_secret_key = secret_key->secret_key; - *out_key_len = secret_key->key_len; - return secret_key->is_good; + *out_host_id = host_id->host_id; + *out_host_id_len = host_id->host_id_len; + return host_id->is_good; } gint64 -nm_utils_secret_key_get_timestamp (void) +nm_utils_host_id_get_timestamp (void) { struct stat stat_buf; - const guint8 *key; - gsize key_len; + const guint8 *host_id; + gsize host_id_len; - if (!nm_utils_secret_key_get (&key, &key_len)) + if (!nm_utils_host_id_get (&host_id, &host_id_len)) return 0; if (stat (SECRET_KEY_FILE, &stat_buf) != 0) @@ -3379,20 +3394,20 @@ _set_stable_privacy (NMUtilsStableType stable_type, const char *ifname, const char *network_id, guint32 dad_counter, - const guint8 *secret_key, - gsize key_len, + const guint8 *host_id, + gsize host_id_len, GError **error) { nm_auto_free_checksum GChecksum *sum = NULL; guint8 digest[NM_UTILS_CHECKSUM_LENGTH_SHA256]; guint32 tmp[2]; - nm_assert (key_len); + nm_assert (host_id_len); nm_assert (network_id); sum = g_checksum_new (G_CHECKSUM_SHA256); - key_len = MIN (key_len, G_MAXUINT32); + host_id_len = MIN (host_id_len, G_MAXUINT32); if (stable_type != NM_UTILS_STABLE_TYPE_UUID) { guint8 stable_type_uint8; @@ -3405,7 +3420,7 @@ _set_stable_privacy (NMUtilsStableType stable_type, * * That is no real problem and it is still impossible to * force a collision here, because of how the remaining - * fields are hashed. That is, as we also hash @key_len + * fields are hashed. That is, as we also hash @host_id_len * and the terminating '\0' of @network_id, it is unambigiously * possible to revert the process and deduce the @stable_type. */ @@ -3416,9 +3431,9 @@ _set_stable_privacy (NMUtilsStableType stable_type, g_checksum_update (sum, (const guchar *) ifname, strlen (ifname) + 1); g_checksum_update (sum, (const guchar *) network_id, strlen (network_id) + 1); tmp[0] = htonl (dad_counter); - tmp[1] = htonl (key_len); + tmp[1] = htonl (host_id_len); g_checksum_update (sum, (const guchar *) tmp, sizeof (tmp)); - g_checksum_update (sum, (const guchar *) secret_key, key_len); + g_checksum_update (sum, (const guchar *) host_id, host_id_len); nm_utils_checksum_get_digest (sum, digest); while (_is_reserved_ipv6_iid (digest)) { @@ -3439,11 +3454,11 @@ nm_utils_ipv6_addr_set_stable_privacy_impl (NMUtilsStableType stable_type, const char *ifname, const char *network_id, guint32 dad_counter, - guint8 *secret_key, - gsize key_len, + guint8 *host_id, + gsize host_id_len, GError **error) { - return _set_stable_privacy (stable_type, addr, ifname, network_id, dad_counter, secret_key, key_len, error); + return _set_stable_privacy (stable_type, addr, ifname, network_id, dad_counter, host_id, host_id_len, error); } #define RFC7217_IDGEN_RETRIES 3 @@ -3463,8 +3478,8 @@ nm_utils_ipv6_addr_set_stable_privacy (NMUtilsStableType stable_type, guint32 dad_counter, GError **error) { - const guint8 *secret_key; - gsize key_len; + const guint8 *host_id; + gsize host_id_len; g_return_val_if_fail (network_id, FALSE); @@ -3474,10 +3489,10 @@ nm_utils_ipv6_addr_set_stable_privacy (NMUtilsStableType stable_type, return FALSE; } - nm_utils_secret_key_get (&secret_key, &key_len); + nm_utils_host_id_get (&host_id, &host_id_len); return _set_stable_privacy (stable_type, addr, ifname, network_id, dad_counter, - secret_key, key_len, error); + host_id, host_id_len, error); } /*****************************************************************************/ @@ -3549,8 +3564,8 @@ nm_utils_hw_addr_gen_random_eth (const char *current_mac_address, static char * _hw_addr_gen_stable_eth (NMUtilsStableType stable_type, const char *stable_id, - const guint8 *secret_key, - gsize key_len, + const guint8 *host_id, + gsize host_id_len, const char *ifname, const char *current_mac_address, const char *generate_mac_address_mask) @@ -3562,19 +3577,19 @@ _hw_addr_gen_stable_eth (NMUtilsStableType stable_type, guint8 stable_type_uint8; nm_assert (stable_id); - nm_assert (secret_key); + nm_assert (host_id); sum = g_checksum_new (G_CHECKSUM_SHA256); - key_len = MIN (key_len, G_MAXUINT32); + host_id_len = MIN (host_id_len, G_MAXUINT32); nm_assert (stable_type < (NMUtilsStableType) 255); stable_type_uint8 = stable_type; g_checksum_update (sum, (const guchar *) &stable_type_uint8, sizeof (stable_type_uint8)); - tmp = htonl ((guint32) key_len); + tmp = htonl ((guint32) host_id_len); g_checksum_update (sum, (const guchar *) &tmp, sizeof (tmp)); - g_checksum_update (sum, (const guchar *) secret_key, key_len); + g_checksum_update (sum, (const guchar *) host_id, host_id_len); g_checksum_update (sum, (const guchar *) (ifname ?: ""), ifname ? (strlen (ifname) + 1) : 1); g_checksum_update (sum, (const guchar *) stable_id, strlen (stable_id) + 1); @@ -3588,13 +3603,13 @@ _hw_addr_gen_stable_eth (NMUtilsStableType stable_type, char * nm_utils_hw_addr_gen_stable_eth_impl (NMUtilsStableType stable_type, const char *stable_id, - const guint8 *secret_key, - gsize key_len, + const guint8 *host_id, + gsize host_id_len, const char *ifname, const char *current_mac_address, const char *generate_mac_address_mask) { - return _hw_addr_gen_stable_eth (stable_type, stable_id, secret_key, key_len, ifname, current_mac_address, generate_mac_address_mask); + return _hw_addr_gen_stable_eth (stable_type, stable_id, host_id, host_id_len, ifname, current_mac_address, generate_mac_address_mask); } char * @@ -3604,17 +3619,17 @@ nm_utils_hw_addr_gen_stable_eth (NMUtilsStableType stable_type, const char *current_mac_address, const char *generate_mac_address_mask) { - const guint8 *secret_key; - gsize key_len; + const guint8 *host_id; + gsize host_id_len; g_return_val_if_fail (stable_id, NULL); - nm_utils_secret_key_get (&secret_key, &key_len); + nm_utils_host_id_get (&host_id, &host_id_len); return _hw_addr_gen_stable_eth (stable_type, stable_id, - secret_key, - key_len, + host_id, + host_id_len, ifname, current_mac_address, generate_mac_address_mask); diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index ab9dade210..c93e69bbd2 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -289,9 +289,9 @@ gboolean nm_utils_machine_id_is_fake (void); const char *nm_utils_get_boot_id_str (void); const struct _NMUuid *nm_utils_get_boot_id_bin (void); -gboolean nm_utils_secret_key_get (const guint8 **out_secret_key, - gsize *out_key_len); -gint64 nm_utils_secret_key_get_timestamp (void); +gboolean nm_utils_host_id_get (const guint8 **out_host_id, + gsize *out_host_id_len); +gint64 nm_utils_host_id_get_timestamp (void); /* IPv6 Interface Identifier helpers */ @@ -359,8 +359,8 @@ gboolean nm_utils_ipv6_addr_set_stable_privacy_impl (NMUtilsStableType stable_ty const char *ifname, const char *network_id, guint32 dad_counter, - guint8 *secret_key, - gsize key_len, + guint8 *host_id, + gsize host_id_len, GError **error); gboolean nm_utils_ipv6_addr_set_stable_privacy (NMUtilsStableType id_type, @@ -374,8 +374,8 @@ char *nm_utils_hw_addr_gen_random_eth (const char *current_mac_address, const char *generate_mac_address_mask); char *nm_utils_hw_addr_gen_stable_eth_impl (NMUtilsStableType stable_type, const char *stable_id, - const guint8 *secret_key, - gsize key_len, + const guint8 *host_id, + gsize host_id_len, const char *ifname, const char *current_mac_address, const char *generate_mac_address_mask); From 4482c4d4af091baa16f10f5ac42919095e079559 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 12 Dec 2018 10:14:51 +0100 Subject: [PATCH 3/5] core/trivial: rename nm_utils_get_boot_id_*() Rename to nm_utils_boot_id_*(), it matches nm_utils_machine_id_*() and nm_utils_host_id_get(). (cherry picked from commit d693e03a74bee600ba14ee8dad666ff6fe658ca2) --- src/devices/nm-device.c | 2 +- src/nm-core-utils.c | 6 +++--- src/nm-core-utils.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 3df95b5329..ba76ed007b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1289,7 +1289,7 @@ _get_stable_id (NMDevice *self, stable_type = nm_utils_stable_id_parse (stable_id, nm_device_get_ip_iface (self), !hwaddr_is_fake ? hwaddr : NULL, - nm_utils_get_boot_id_str (), + nm_utils_boot_id_str (), uuid, &generated); diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 4ac68c2dad..18d79bd1d0 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2669,7 +2669,7 @@ again: * to read/write the secret-key to disk. Fallback to boot-id. The boot-id * itself may be fake and randomly generated ad-hoc, but that is as best * as it gets. */ - seed_bin = (const guint8 *) nm_utils_get_boot_id_bin (); + seed_bin = (const guint8 *) nm_utils_boot_id_bin (); seed_len = sizeof (NMUuid); fake_type = "boot-id"; hash_seed = "7ff0c8f5-5399-4901-ab63-61bf594abe8b"; @@ -2975,13 +2975,13 @@ again: } const char * -nm_utils_get_boot_id_str (void) +nm_utils_boot_id_str (void) { return _boot_id_get ()->str; } const NMUuid * -nm_utils_get_boot_id_bin (void) +nm_utils_boot_id_bin (void) { return &_boot_id_get ()->bin; } diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index c93e69bbd2..51a695f9fa 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -286,8 +286,8 @@ const char *nm_utils_machine_id_str (void); const struct _NMUuid *nm_utils_machine_id_bin (void); gboolean nm_utils_machine_id_is_fake (void); -const char *nm_utils_get_boot_id_str (void); -const struct _NMUuid *nm_utils_get_boot_id_bin (void); +const char *nm_utils_boot_id_str (void); +const struct _NMUuid *nm_utils_boot_id_bin (void); gboolean nm_utils_host_id_get (const guint8 **out_host_id, gsize *out_host_id_len); From 164d796cf87e1f99ac2a2581f0f7096e6e230408 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 12 Dec 2018 11:03:02 +0100 Subject: [PATCH 4/5] core: split initializing host-id singleton out of nm_utils_host_id_get() (cherry picked from commit e9887d4816805d939e91215ad137b94ffa3c2c5d) --- src/nm-core-utils.c | 49 ++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 18d79bd1d0..b4997db219 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2877,24 +2877,8 @@ typedef struct { bool is_good:1; } HostIdData; -/** - * nm_utils_host_id_get: - * @out_host_id: (out) (transfer none): the binary host key - * @out_host_id_len: the length of the host key. - * - * This returns a per-host key that depends on /var/lib/NetworkManage/secret_key - * and (depending on the version) on /etc/machine-id. If /var/lib/NetworkManage/secret_key - * does not exist, it will be generated and persisted for next boot. - * - * Returns: %TRUE, if the host key is "good". Note that this function - * will always succeed to return a host-key, and that this key - * won't change during the run of the program (no matter what). - * A %FALSE return possibly means, that the secret_key is not persisted - * to disk, and/or that it was generated with bad randomness. - */ -gboolean -nm_utils_host_id_get (const guint8 **out_host_id, - gsize *out_host_id_len) +static const HostIdData * +_host_id_get (void) { static const HostIdData *volatile host_id_static; const HostIdData *host_id; @@ -2915,6 +2899,31 @@ again: g_once_init_leave (&init_value, 1); } + return host_id; +} + +/** + * nm_utils_host_id_get: + * @out_host_id: (out) (transfer none): the binary host key + * @out_host_id_len: the length of the host key. + * + * This returns a per-host key that depends on /var/lib/NetworkManage/secret_key + * and (depending on the version) on /etc/machine-id. If /var/lib/NetworkManage/secret_key + * does not exist, it will be generated and persisted for next boot. + * + * Returns: %TRUE, if the host key is "good". Note that this function + * will always succeed to return a host-key, and that this key + * won't change during the run of the program (no matter what). + * A %FALSE return possibly means, that the secret_key is not persisted + * to disk, and/or that it was generated with bad randomness. + */ +gboolean +nm_utils_host_id_get (const guint8 **out_host_id, + gsize *out_host_id_len) +{ + const HostIdData *host_id; + + host_id = _host_id_get (); *out_host_id = host_id->host_id; *out_host_id_len = host_id->host_id_len; return host_id->is_good; @@ -2924,10 +2933,8 @@ gint64 nm_utils_host_id_get_timestamp (void) { struct stat stat_buf; - const guint8 *host_id; - gsize host_id_len; - if (!nm_utils_host_id_get (&host_id, &host_id_len)) + if (!_host_id_get ()->is_good) return 0; if (stat (SECRET_KEY_FILE, &stat_buf) != 0) From 62722347c58fc0dfc8805a41813781e5a4ecacdf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 12 Dec 2018 11:04:12 +0100 Subject: [PATCH 5/5] core: never fail reading host-id timestamp and never change it The timestamp of the host-id is the timestamp of the secret_key file. Under normal circumstances, reading the timestamp should never fail, and reading it multiple times should always yield the same result. If we unexpectedly fail to read the timestamp from the file we want: - log a warning, so that the user can find out what's wrong. But do so only once. - we don't want to handle errors or fail operation due to a missing timestamp. Remember, it's not supposed to ever fail, and if it does, just log a warning and proceed with a fake timestamp instead. In that case something is wrong, but using a non-stable, fake timestamp is the least of the problems here. We already have a stable identifier (the host-id) which we can use to generate a fake timestamp. Use it. In case the user would replace the secret_key file, we also don't want that accessing nm_utils_host_id_get_timestamp*() yields different results. It's not implemented (nor necessary) to support reloading a different timestamp. Hence, nm_utils_host_id_get_timestamp() should memoize the value and ensure that it never changes. (cherry picked from commit a68d027ba4537a6e30272e39a9f7927bcdca0eee) --- src/devices/nm-device.c | 18 +++-------- src/nm-core-utils.c | 66 ++++++++++++++++++++++++++++++++++------- src/nm-core-utils.h | 2 +- 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index ba76ed007b..996b8afff9 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8339,15 +8339,8 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole if (nm_streq (duid, "ll")) { duid_out = generate_duid_ll (g_bytes_get_data (hwaddr, NULL)); } else { - gint64 time; - - time = nm_utils_host_id_get_timestamp (); - if (!time) { - duid_error = "cannot retrieve the secret key timestamp"; - goto out_fail; - } - - duid_out = generate_duid_llt (g_bytes_get_data (hwaddr, NULL), time); + duid_out = generate_duid_llt (g_bytes_get_data (hwaddr, NULL), + nm_utils_host_id_get_timestamp_ns () / NM_UTILS_NS_PER_SECOND); } goto out_good; @@ -8397,11 +8390,8 @@ dhcp6_get_duid (NMDevice *self, NMConnection *connection, GBytes *hwaddr, gboole * before. Let's compute the time (in seconds) from 0 to 3 years; then we'll * subtract it from the host_id timestamp. */ - time = nm_utils_host_id_get_timestamp (); - if (!time) { - duid_error = "cannot retrieve the secret key timestamp"; - goto out_fail; - } + time = nm_utils_host_id_get_timestamp_ns () / NM_UTILS_NS_PER_SECOND; + /* don't use too old timestamps. They cannot be expressed in DUID-LLT and * would all be truncated to zero. */ time = NM_MAX (time, EPOCH_DATETIME_200001010000 + EPOCH_DATETIME_THREE_YEARS); diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index b4997db219..d5e6346de6 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2727,6 +2727,49 @@ nm_utils_machine_id_is_fake (void) #define SECRET_KEY_V2_PREFIX "nm-v2:" #define SECRET_KEY_FILE NMSTATEDIR"/secret_key" +static gboolean +_host_id_read_timestamp (gboolean use_secret_key_file, + const guint8 *host_id, + gsize host_id_len, + gint64 *out_timestamp_ns) +{ + struct stat st; + gint64 now; + guint64 v; + + if ( use_secret_key_file + && stat (SECRET_KEY_FILE, &st) == 0) { + /* don't check for overflow or timestamps in the future. We get whatever + * (bogus) date is on the file. */ + *out_timestamp_ns = (st.st_mtim.tv_sec * NM_UTILS_NS_PER_SECOND) + st.st_mtim.tv_nsec; + return TRUE; + } + + /* generate a fake timestamp based on the host-id. + * + * This really should never happen under normal circumstances. We already + * are in a code path, where the system has a problem (unable to get good randomness + * and/or can't access the secret_key). In such a scenario, a fake timestamp is the + * least of our problems. + * + * At least, generate something sensible so we don't have to worry about the + * timestamp. It is wrong to worry about using a fake timestamp (which is tied to + * the secret_key) if we are unable to access the secret_key file in the first place. + * + * Pick a random timestamp from the past two years. Yes, this timestamp + * is not stable accross restarts, but apparently neither is the host-id + * nor the secret_key itself. */ + +#define EPOCH_TWO_YEARS (G_GINT64_CONSTANT (2 * 365 * 24 * 3600) * NM_UTILS_NS_PER_SECOND) + + v = nm_hash_siphash42 (1156657133u, host_id, host_id_len); + + now = time (NULL); + *out_timestamp_ns = NM_MAX ((gint64) 1, + (now * NM_UTILS_NS_PER_SECOND) - ((gint64) (v % ((guint64) (EPOCH_TWO_YEARS))))); + return FALSE; +} + static const guint8 * _host_id_hash_v2 (const guint8 *seed_arr, gsize seed_len, @@ -2874,7 +2917,9 @@ out: typedef struct { guint8 *host_id; gsize host_id_len; + gint64 timestamp_ns; bool is_good:1; + bool timestamp_is_good:1; } HostIdData; static const HostIdData * @@ -2894,6 +2939,15 @@ again: host_id_data.is_good = _host_id_read (&host_id_data.host_id, &host_id_data.host_id_len); + + host_id_data.timestamp_is_good = _host_id_read_timestamp (host_id_data.is_good, + host_id_data.host_id, + host_id_data.host_id_len, + &host_id_data.timestamp_ns); + if ( !host_id_data.timestamp_is_good + && host_id_data.is_good) + nm_log_warn (LOGD_CORE, "secret-key: failure reading host timestamp (use fake one)"); + host_id = &host_id_data; g_atomic_pointer_set (&host_id_static, host_id); g_once_init_leave (&init_value, 1); @@ -2930,17 +2984,9 @@ nm_utils_host_id_get (const guint8 **out_host_id, } gint64 -nm_utils_host_id_get_timestamp (void) +nm_utils_host_id_get_timestamp_ns (void) { - struct stat stat_buf; - - if (!_host_id_get ()->is_good) - return 0; - - if (stat (SECRET_KEY_FILE, &stat_buf) != 0) - return 0; - - return stat_buf.st_mtim.tv_sec; + return _host_id_get ()->timestamp_ns; } /*****************************************************************************/ diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index 51a695f9fa..16ca50b607 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -291,7 +291,7 @@ const struct _NMUuid *nm_utils_boot_id_bin (void); gboolean nm_utils_host_id_get (const guint8 **out_host_id, gsize *out_host_id_len); -gint64 nm_utils_host_id_get_timestamp (void); +gint64 nm_utils_host_id_get_timestamp_ns (void); /* IPv6 Interface Identifier helpers */