Blob Blame History Raw
From 861d9f8e1593c573bb04572b1f89f83500df85df Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 07:23:51 +0100
Subject: [PATCH 01/10] shared: add nm_utils_error_is_notfound() helper

Inspired by bolt's bolt_err_notfound() in "bolt-error.c".

(cherry picked from commit b7fc328a66dd66f3f9d9f702e8b4a6d505ea3560)
(cherry picked from commit d0f516a2bda68b22c456ef945e1c1fe8e36986ba)
---
 shared/nm-utils/nm-shared-utils.c | 23 ++++++++++++++++++-----
 shared/nm-utils/nm-shared-utils.h |  2 ++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c
index 022c06528..0d4696cda 100644
--- a/shared/nm-utils/nm-shared-utils.c
+++ b/shared/nm-utils/nm-shared-utils.c
@@ -1037,11 +1037,24 @@ nm_utils_error_is_cancelled (GError *error,
                              gboolean consider_is_disposing)
 {
 	if (error) {
-		if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-			return TRUE;
-		if (   consider_is_disposing
-		    && g_error_matches (error, NM_UTILS_ERROR, NM_UTILS_ERROR_CANCELLED_DISPOSING))
-			return TRUE;
+		if (error->domain == G_IO_ERROR)
+			return NM_IN_SET (error->code, G_IO_ERROR_CANCELLED);
+		if (consider_is_disposing) {
+			if (error->domain == NM_UTILS_ERROR)
+				return NM_IN_SET (error->code, NM_UTILS_ERROR_CANCELLED_DISPOSING);
+		}
+	}
+	return FALSE;
+}
+
+gboolean
+nm_utils_error_is_notfound (GError *error)
+{
+	if (error) {
+		if (error->domain == G_IO_ERROR)
+			return NM_IN_SET (error->code, G_IO_ERROR_NOT_FOUND);
+		if (error->domain == G_FILE_ERROR)
+			return NM_IN_SET (error->code, G_FILE_ERROR_NOENT);
 	}
 	return FALSE;
 }
diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h
index f1ff71e3b..20311d08a 100644
--- a/shared/nm-utils/nm-shared-utils.h
+++ b/shared/nm-utils/nm-shared-utils.h
@@ -586,6 +586,8 @@ void nm_utils_error_set_cancelled (GError **error,
 gboolean nm_utils_error_is_cancelled (GError *error,
                                       gboolean consider_is_disposing);
 
+gboolean nm_utils_error_is_notfound (GError *error);
+
 static inline void
 nm_utils_error_set_literal (GError **error, int error_code, const char *literal)
 {
-- 
2.19.2


From cbc815117511279cbd976658313f59c43debf97a Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 07:29:38 +0100
Subject: [PATCH 02/10] core: fix printing error for failure reading secret-key

g_file_get_contents() fails with G_FILE_ERROR, G_FILE_ERROR_NOENT when the
file does not exist.

That wasn't obvious to me, nm_utils_error_is_notfound() to the rescue.

Fixes: dbcb1d6d97c609d53dac4a86dc45d0e2595d8857
(cherry picked from commit 7b9cd2e3d7206cc895fc2e27df3b7bd987da6ef5)
(cherry picked from commit cfc0c6d5146513fba856894506542facde9fc914)
---
 src/nm-core-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index f0efee461..44f0549e6 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2735,7 +2735,7 @@ _secret_key_read (guint8 **out_secret_key,
 		nm_log_warn (LOGD_CORE, "secret-key: too short secret key in \"%s\" (generate new key)", NMSTATEDIR "/secret_key");
 		nm_clear_g_free (&secret_key);
 	} else {
-		if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) {
+		if (!nm_utils_error_is_notfound (error)) {
 			nm_log_warn (LOGD_CORE, "secret-key: failure reading secret key in \"%s\": %s (generate new key)",
 			             NMSTATEDIR "/secret_key", error->message);
 		}
-- 
2.19.2


From 3484e98ce766858a2cb9c33b91e955140b93e54a Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 07:34:34 +0100
Subject: [PATCH 03/10] core: use define for secret_key filename

(cherry picked from commit 2c7c333297a57d55efb2bd5a447391ea77861423)
(cherry picked from commit 0b451b4c879c3c8178b235a5d9b2b669f4696f81)
---
 src/nm-core-utils.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 44f0549e6..ab433b292 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2716,6 +2716,8 @@ nm_utils_machine_id_is_fake (void)
 
 /*****************************************************************************/
 
+#define SECRET_KEY_FILE      NMSTATEDIR"/secret_key"
+
 static gboolean
 _secret_key_read (guint8 **out_secret_key,
                   gsize *out_key_len)
@@ -2726,18 +2728,18 @@ _secret_key_read (guint8 **out_secret_key,
 	gs_free_error GError *error = NULL;
 
 	/* Let's try to load a saved secret key first. */
-	if (g_file_get_contents (NMSTATEDIR "/secret_key", (char **) &secret_key, &key_len, &error)) {
+	if (g_file_get_contents (SECRET_KEY_FILE, (char **) &secret_key, &key_len, &error)) {
 		if (key_len >= 16)
 			goto out;
 
 		/* the secret key is borked. Log a warning, but proceed below to generate
 		 * a new one. */
-		nm_log_warn (LOGD_CORE, "secret-key: too short secret key in \"%s\" (generate new key)", NMSTATEDIR "/secret_key");
+		nm_log_warn (LOGD_CORE, "secret-key: too short secret key in \"%s\" (generate new key)", SECRET_KEY_FILE);
 		nm_clear_g_free (&secret_key);
 	} else {
 		if (!nm_utils_error_is_notfound (error)) {
 			nm_log_warn (LOGD_CORE, "secret-key: failure reading secret key in \"%s\": %s (generate new key)",
-			             NMSTATEDIR "/secret_key", error->message);
+			             SECRET_KEY_FILE, error->message);
 		}
 		g_clear_error (&error);
 	}
@@ -2763,9 +2765,9 @@ _secret_key_read (guint8 **out_secret_key,
 		goto out;
 	}
 
-	if (!nm_utils_file_set_contents (NMSTATEDIR "/secret_key", (char *) secret_key, key_len, 0077, &error)) {
+	if (!nm_utils_file_set_contents (SECRET_KEY_FILE, (char *) secret_key, key_len, 0077, &error)) {
 		nm_log_warn (LOGD_CORE, "secret-key: failure to persist secret key in \"%s\" (%s) (use non-persistent key)",
-		             NMSTATEDIR "/secret_key", error->message);
+		             SECRET_KEY_FILE, error->message);
 		success = FALSE;
 		goto out;
 	}
@@ -2827,7 +2829,7 @@ nm_utils_secret_key_get_timestamp (void)
 	if (!nm_utils_secret_key_get (&key, &key_len))
 		return 0;
 
-	if (stat (NMSTATEDIR "/secret_key", &stat_buf) != 0)
+	if (stat (SECRET_KEY_FILE, &stat_buf) != 0)
 		return 0;
 
 	return stat_buf.st_mtim.tv_sec;
-- 
2.19.2


From 4ac908e32ac7c79a2ff7781e702d5d0f688bbcb3 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 5 Dec 2018 13:42:33 +0100
Subject: [PATCH 04/10] core: combine secret-key with /etc/machine-id

NetworkManager loads (and generates) a secret key as
"/var/lib/NetworkManager/secret_key".

The secret key is used for seeding a per-host component when generating
hashed, stable data. For example, it contributes to "ipv4.dhcp-client-id=duid"
"ipv6.addr-gen-mode=stable-privacy", "ethernet.cloned-mac-address=stable", etc.
As such, it corresponds to the identity of the host.

Also "/etc/machine-id" is the host's identity. When cloning a virtual machine,
it may be a good idea to generate a new "/etc/machine-id", at least in those
cases where the VM's identity shall be different. Systemd provides various
mechanisms for doing that, like accepting a new machine-id via kernel command line.
For the same reason, the user should also regenerate a new NetworkManager's
secrey key when the host's identity shall change. However, that is less obvious,
less understood and less documented.

Support and use a new variant of secret key. This secret key is combined
with "/etc/machine-id" by sha256 hashing it together. That means, when the
user generates a new machine-id, NetworkManager's per-host key also changes.

Since we don't want to change behavior for existing installations, we
only do this when generating a new secret key file. For that, we encode
a version tag inside the "/var/lib/NetworkManager/secret_key" file.

Note that this is all abstracted by nm_utils_secret_key_get(). For
version 2 secret-keys, it internally combines the secret_key file with
machine-id (via sha256). The advantage is that callers don't care that
the secret-key now also contains the machine-id. Also, since we want to
stick to the previous behavior if we have an old secret-key, this is
nicely abstracted. Otherwise, the caller would not only need to handle
two per-host parts, but it would also need to check the version to
determine whether the machine-id should be explicitly included.
At this point, nm_utils_secret_key_get() should be renamed to
nm_utils_host_key_get().

(cherry picked from commit deb19abf22eca93bdb57f52d162b5d81f1a85fc1)
(cherry picked from commit 7b68a574e9b4e242062db50ca36a8e77822cd646)
---
 src/nm-core-utils.c | 185 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 138 insertions(+), 47 deletions(-)

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index ab433b292..a379e405f 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -41,6 +41,7 @@
 #include "nm-utils/nm-random-utils.h"
 #include "nm-utils/nm-io-utils.h"
 #include "nm-utils/unaligned.h"
+#include "nm-utils/nm-secret-utils.h"
 #include "nm-utils.h"
 #include "nm-core-internal.h"
 #include "nm-setting-connection.h"
@@ -2608,7 +2609,7 @@ _uuid_data_init (UuidData *uuid_data,
 /*****************************************************************************/
 
 static const UuidData *
-_machine_id_get (void)
+_machine_id_get (gboolean allow_fake)
 {
 	static const UuidData *volatile p_uuid_data;
 	const UuidData *d;
@@ -2650,6 +2651,12 @@ again:
 			const char *hash_seed;
 			gsize seed_len;
 
+			if (!allow_fake) {
+				/* we don't allow generating (and memoizing) a fake key.
+				 * Signal that no valid machine-id exists. */
+				return NULL;
+			}
+
 			if (nm_utils_secret_key_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
@@ -2699,84 +2706,168 @@ again:
 const char *
 nm_utils_machine_id_str (void)
 {
-	return _machine_id_get ()->str;
+	return _machine_id_get (TRUE)->str;
 }
 
 const NMUuid *
 nm_utils_machine_id_bin (void)
 {
-	return &_machine_id_get ()->bin;
+	return &_machine_id_get (TRUE)->bin;
 }
 
 gboolean
 nm_utils_machine_id_is_fake (void)
 {
-	return _machine_id_get ()->is_fake;
+	return _machine_id_get (TRUE)->is_fake;
 }
 
 /*****************************************************************************/
 
+/* prefix for version2 secret key. The secret key is hashed with /etc/machine-id. */
+#define SECRET_KEY_V2_PREFIX "nm-v2:"
 #define SECRET_KEY_FILE      NMSTATEDIR"/secret_key"
 
-static gboolean
-_secret_key_read (guint8 **out_secret_key,
-                  gsize *out_key_len)
+static const guint8 *
+_secret_key_hash_v2 (const guint8 *seed_arr,
+                     gsize seed_len,
+                     guint8 *out_digest /* 32 bytes (NM_UTILS_CHECKSUM_LENGTH_SHA256) */)
 {
-	guint8 *secret_key;
-	gboolean success = TRUE;
-	gsize key_len;
-	gs_free_error GError *error = NULL;
+	nm_auto_free_checksum GChecksum *sum = g_checksum_new (G_CHECKSUM_SHA256);
+	const UuidData *machine_id_data;
+	char slen[100];
 
-	/* Let's try to load a saved secret key first. */
-	if (g_file_get_contents (SECRET_KEY_FILE, (char **) &secret_key, &key_len, &error)) {
-		if (key_len >= 16)
-			goto out;
+	/*
+	    (stat -c '%s' /var/lib/NetworkManager/secret_key;
+	     echo -n ' ';
+	     cat /var/lib/NetworkManager/secret_key;
+	     cat /etc/machine-id | tr -d '\n' | sed -n 's/[a-f0-9-]/\0/pg') | sha256sum
+	*/
 
-		/* the secret key is borked. Log a warning, but proceed below to generate
-		 * a new one. */
-		nm_log_warn (LOGD_CORE, "secret-key: too short secret key in \"%s\" (generate new key)", SECRET_KEY_FILE);
-		nm_clear_g_free (&secret_key);
-	} else {
+	nm_sprintf_buf (slen, "%"G_GSIZE_FORMAT" ", seed_len);
+	g_checksum_update (sum, (const guchar *) slen, strlen (slen));
+
+	g_checksum_update (sum, (const guchar *) seed_arr, seed_len);
+
+	machine_id_data = _machine_id_get (FALSE);
+	if (   machine_id_data
+	    && !machine_id_data->is_fake)
+		g_checksum_update (sum, (const guchar *) machine_id_data->str, strlen (machine_id_data->str));
+
+	nm_utils_checksum_get_digest_len (sum, out_digest, NM_UTILS_CHECKSUM_LENGTH_SHA256);
+	return out_digest;
+}
+
+static gboolean
+_secret_key_read (guint8 **out_key,
+                  gsize *out_key_len)
+{
+#define SECRET_KEY_LEN 32u
+	guint8 sha256_digest[NM_UTILS_CHECKSUM_LENGTH_SHA256];
+	nm_auto_clear_secret_ptr NMSecretPtr file_content = { 0 };
+	const guint8 *secret_arr;
+	gsize secret_len;
+	GError *error = NULL;
+	gboolean success;
+
+	if (nm_utils_file_get_contents (-1,
+	                                SECRET_KEY_FILE,
+	                                10*1024,
+	                                NM_UTILS_FILE_GET_CONTENTS_FLAG_SECRET,
+	                                (char **) &file_content.str,
+	                                &file_content.len,
+	                                &error) < 0) {
 		if (!nm_utils_error_is_notfound (error)) {
 			nm_log_warn (LOGD_CORE, "secret-key: failure reading secret key in \"%s\": %s (generate new key)",
 			             SECRET_KEY_FILE, error->message);
 		}
 		g_clear_error (&error);
-	}
-
-	/* RFC7217 mandates the key SHOULD be at least 128 bits.
-	 * Let's use twice as much. */
-	key_len = 32;
-	secret_key = g_malloc (key_len + 1);
-
-	/* the secret-key is binary. Still, ensure that it's NULL terminated, just like
-	 * g_file_set_contents() does. */
-	secret_key[32] = '\0';
+	} else if (   file_content.len >= NM_STRLEN (SECRET_KEY_V2_PREFIX) + SECRET_KEY_LEN
+	           && memcmp (file_content.bin, SECRET_KEY_V2_PREFIX, NM_STRLEN (SECRET_KEY_V2_PREFIX)) == 0) {
+		/* for this type of secret key, we require a prefix followed at least SECRET_KEY_LEN (32) bytes. We
+		 * (also) do that, because older versions of NetworkManager wrote exactly 32 bytes without
+		 * prefix, so we won't wrongly interpret such legacy keys as v2 (if they accidentally have
+		 * a SECRET_KEY_V2_PREFIX prefix, they'll still have the wrong size).
+		 *
+		 * Note that below we generate the random seed in base64 encoding. But that is only done
+		 * to write an ASCII file. There is no base64 decoding and the ASCII is hashed as-is.
+		 * We would accept any binary data just as well (provided a suitable prefix and at least
+		 * 32 bytes).
+		 *
+		 * Note that when hashing the v2 content, we also hash the prefix. There is no strong reason,
+		 * except that it seems simpler not to distinguish between the v2 prefix and the content.
+		 * It's all just part of the seed. */
 
-	if (!nm_utils_random_bytes (secret_key, key_len)) {
-		nm_log_warn (LOGD_CORE, "secret-key: failure to generate good random data for secret-key (use non-persistent key)");
-		success = FALSE;
+		secret_arr = _secret_key_hash_v2 (file_content.bin, file_content.len, sha256_digest);
+		secret_len = NM_UTILS_CHECKSUM_LENGTH_SHA256;
+		success = TRUE;
 		goto out;
-	}
-
-	if (nm_utils_get_testing ()) {
-		/* for test code, we don't write the generated secret-key to disk. */
-		success = FALSE;
+	} else if (file_content.len >= 16) {
+		secret_arr = file_content.bin;
+		secret_len = file_content.len;
+		success = TRUE;
 		goto out;
+	} else {
+		/* the secret key is borked. Log a warning, but proceed below to generate
+		 * a new one. */
+		nm_log_warn (LOGD_CORE, "secret-key: too short secret key in \"%s\" (generate new key)", SECRET_KEY_FILE);
 	}
 
-	if (!nm_utils_file_set_contents (SECRET_KEY_FILE, (char *) secret_key, key_len, 0077, &error)) {
-		nm_log_warn (LOGD_CORE, "secret-key: failure to persist secret key in \"%s\" (%s) (use non-persistent key)",
-		             SECRET_KEY_FILE, error->message);
-		success = FALSE;
-		goto out;
+	/* generate and persist new key */
+	{
+#define SECRET_KEY_LEN_BASE64 ((((SECRET_KEY_LEN / 3) + 1) * 4) + 4)
+		guint8 rnd_buf[SECRET_KEY_LEN];
+		guint8 new_content[NM_STRLEN (SECRET_KEY_V2_PREFIX) + SECRET_KEY_LEN_BASE64];
+		int base64_state = 0;
+		int base64_save = 0;
+		gsize len;
+
+		success = nm_utils_random_bytes (rnd_buf, sizeof (rnd_buf));
+
+		/* Our key is really binary data. But since we anyway generate a random seed
+		 * (with 32 random bytes), don't write it in binary, but instead create
+		 * an pure ASCII (base64) representation. Note that the ASCII will still be taken
+		 * as-is (no base64 decoding is done). The sole purpose is to write a ASCII file
+		 * instead of a binary. The content is gibberish either way. */
+		memcpy (new_content, SECRET_KEY_V2_PREFIX, NM_STRLEN (SECRET_KEY_V2_PREFIX));
+		len = NM_STRLEN (SECRET_KEY_V2_PREFIX);
+		len += g_base64_encode_step (rnd_buf,
+		                             sizeof (rnd_buf),
+		                             FALSE,
+		                             (char *) &new_content[len],
+		                             &base64_state,
+		                             &base64_save);
+		len += g_base64_encode_close (FALSE,
+		                              (char *) &new_content[len],
+		                              &base64_state,
+		                              &base64_save);
+		nm_assert (len <= sizeof (new_content));
+
+		secret_arr = _secret_key_hash_v2 (new_content, len, sha256_digest);
+		secret_len = NM_UTILS_CHECKSUM_LENGTH_SHA256;
+
+		if (!success)
+			nm_log_warn (LOGD_CORE, "secret-key: failure to generate good random data for secret-key (use non-persistent key)");
+		else if (nm_utils_get_testing ()) {
+			/* for test code, we don't write the generated secret-key to disk. */
+		} else if (!nm_utils_file_set_contents (SECRET_KEY_FILE,
+		                                        (const char *) new_content,
+		                                        len,
+		                                        0077,
+		                                        &error)) {
+			nm_log_warn (LOGD_CORE, "secret-key: failure to persist secret key in \"%s\" (%s) (use non-persistent key)",
+			             SECRET_KEY_FILE, error->message);
+			g_clear_error (&error);
+			success = FALSE;
+		} else
+			nm_log_dbg (LOGD_CORE, "secret-key: persist new secret key to \"%s\"", SECRET_KEY_FILE);
+
+		nm_explicit_bzero (rnd_buf, sizeof (rnd_buf));
+		nm_explicit_bzero (new_content, sizeof (new_content));
 	}
 
 out:
-	/* regardless of success or failue, we always return a secret-key. The
-	 * caller may choose to ignore the error and proceed. */
-	*out_key_len = key_len;
-	*out_secret_key = secret_key;
+	*out_key_len = secret_len;
+	*out_key = nm_memdup (secret_arr, secret_len);
 	return success;
 }
 
-- 
2.19.2


From adf0a12121b5adb9a47f9ca2e84a29597115c5a7 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 11 Dec 2018 18:18:17 +0100
Subject: [PATCH 05/10] core: fix race creating secret-key

Reading the secret key may result in generating and
writing a new key to disk.

Do that under the lock.

(cherry picked from commit bc9f18c609b7aac84110b37ec280cb012364ecf4)
(cherry picked from commit db535b693afc25f9d42cbd9178b624b67f080297)
---
 src/nm-core-utils.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index a379e405f..c3d2e3c2a 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2872,7 +2872,7 @@ out:
 }
 
 typedef struct {
-	const guint8 *secret_key;
+	guint8 *secret_key;
 	gsize key_len;
 	bool is_good:1;
 } SecretKeyData;
@@ -2887,19 +2887,14 @@ nm_utils_secret_key_get (const guint8 **out_secret_key,
 again:
 	secret_key = g_atomic_pointer_get (&secret_key_static);
 	if (G_UNLIKELY (!secret_key)) {
-		static gsize init_value = 0;
 		static SecretKeyData secret_key_data;
-		gboolean tmp_success;
-		gs_free guint8 *tmp_secret_key = NULL;
-		gsize tmp_key_len;
+		static gsize init_value = 0;
 
-		tmp_success = _secret_key_read (&tmp_secret_key, &tmp_key_len);
 		if (!g_once_init_enter (&init_value))
 			goto again;
 
-		secret_key_data.secret_key = g_steal_pointer (&tmp_secret_key);
-		secret_key_data.key_len = tmp_key_len;
-		secret_key_data.is_good = tmp_success;
+		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);
 		g_once_init_leave (&init_value, 1);
-- 
2.19.2


From e8a3a7e25472c74d66790fab14bded799631006b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 12:11:53 +0100
Subject: [PATCH 06/10] 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)
(cherry picked from commit 0a4bbb18fe9fb80acf038177ae581b7aef2c7883)
---
 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 9f164a119..80387c714 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 b797fb75a..cf71a7e9f 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); \
 	})
 
-- 
2.19.2


From 604bf0c568f821f50bc19aac91a6db15d31c2a29 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 10:10:38 +0100
Subject: [PATCH 07/10] 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)
(cherry picked from commit cdb7f6f6d2927aac501efba22557778301834d0e)
---
 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 8d5866266..8821dfaa9 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -7528,8 +7528,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)
@@ -7537,12 +7537,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);
@@ -8317,7 +8317,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;
@@ -8334,8 +8334,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];
@@ -8352,12 +8352,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));
@@ -8369,11 +8369,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 c3d2e3c2a..4ac68c2da 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 ab9dade21..c93e69bbd 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);
-- 
2.19.2


From f135dfbf60b58123e75a7d560a44b815332ad5d0 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 10:14:51 +0100
Subject: [PATCH 08/10] 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)
(cherry picked from commit 4482c4d4af091baa16f10f5ac42919095e079559)
---
 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 8821dfaa9..738e032e3 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -1288,7 +1288,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 4ac68c2da..18d79bd1d 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 c93e69bbd..51a695f9f 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);
-- 
2.19.2


From 221a0ad9f6594ad0e91211f1fcc0bbe273b7c0cb Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 11:03:02 +0100
Subject: [PATCH 09/10] core: split initializing host-id singleton out of
 nm_utils_host_id_get()

(cherry picked from commit e9887d4816805d939e91215ad137b94ffa3c2c5d)
(cherry picked from commit 164d796cf87e1f99ac2a2581f0f7096e6e230408)
---
 src/nm-core-utils.c | 47 ++++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 18d79bd1d..b4997db21 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -2877,6 +2877,31 @@ typedef struct {
 	bool is_good:1;
 } HostIdData;
 
+static const HostIdData *
+_host_id_get (void)
+{
+	static const HostIdData *volatile host_id_static;
+	const HostIdData *host_id;
+
+again:
+	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;
+
+		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);
+	}
+
+	return host_id;
+}
+
 /**
  * nm_utils_host_id_get:
  * @out_host_id: (out) (transfer none): the binary host key
@@ -2896,25 +2921,9 @@ gboolean
 nm_utils_host_id_get (const guint8 **out_host_id,
                       gsize *out_host_id_len)
 {
-	static const HostIdData *volatile host_id_static;
 	const HostIdData *host_id;
 
-again:
-	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;
-
-		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);
-	}
-
+	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)
-- 
2.19.2


From 51853f1456413b6ba45890932d4d4cf08b4f76b0 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Dec 2018 11:04:12 +0100
Subject: [PATCH 10/10] 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)
(cherry picked from commit 62722347c58fc0dfc8805a41813781e5a4ecacdf)
---
 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 738e032e3..d72c946bd 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -8315,15 +8315,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;
@@ -8373,11 +8366,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 b4997db21..d5e6346de 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 51a695f9f..16ca50b60 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 */
 
-- 
2.19.2