From db791db4e1ff7a850b4ac49d24d159e2ccbe005c 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. --- 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 6ffcd263177d01a528a89709609f06550ecded9b 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. --- 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 97330b2e68..ab30a4dbf3 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -7647,8 +7647,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) @@ -7656,12 +7656,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); @@ -8436,7 +8436,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; @@ -8453,8 +8453,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]; @@ -8471,12 +8471,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)); @@ -8488,11 +8488,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 064cfef41d..bf009e1d91 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2455,7 +2455,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 @@ -2526,9 +2526,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; @@ -2556,8 +2556,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]; @@ -2595,7 +2595,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; @@ -2640,7 +2640,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) @@ -2664,53 +2664,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) @@ -3177,20 +3192,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; @@ -3203,7 +3218,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. */ @@ -3214,9 +3229,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)) { @@ -3237,11 +3252,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 @@ -3261,8 +3276,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); @@ -3272,10 +3287,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); } /*****************************************************************************/ @@ -3347,8 +3362,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) @@ -3360,19 +3375,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); @@ -3386,13 +3401,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 * @@ -3402,17 +3417,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 d75ef9ae48..4f1a2bad54 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -278,9 +278,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 */ @@ -348,8 +348,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, @@ -363,8 +363,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 d693e03a74bee600ba14ee8dad666ff6fe658ca2 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(). --- 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 ab30a4dbf3..8f41df8566 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1295,7 +1295,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 bf009e1d91..22ca7afed1 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2467,7 +2467,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"; @@ -2773,13 +2773,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 4f1a2bad54..dd09f3213b 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -275,8 +275,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 e9887d4816805d939e91215ad137b94ffa3c2c5d 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() --- 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 22ca7afed1..8172933457 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2675,24 +2675,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; @@ -2713,6 +2697,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; @@ -2722,10 +2731,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 a68d027ba4537a6e30272e39a9f7927bcdca0eee 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. --- 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 8f41df8566..930b0ab603 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8434,15 +8434,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; @@ -8492,11 +8485,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 8172933457..91da1d5219 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2525,6 +2525,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, @@ -2672,7 +2715,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 * @@ -2692,6 +2737,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); @@ -2728,17 +2782,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 dd09f3213b..a01cac1c0c 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -280,7 +280,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 */