Commit dbcb1d6d authored by Thomas Haller's avatar Thomas Haller

core: let nm_utils_secret_key_read() handle failures internally

and add nm_utils_secret_key_get() to cache the secret-key, to only
obtain it once.

nm_utils_secret_key_read() is not expected to fail. However, in case
of an unexpected error, don't propagate the error to the caller,
but instead handle it internally.

That means, in case of error:
  - log a warning within nm_utils_secret_key_read() itself.
  - always return a generated secret-key. In case of error, the
    key won't be persisted (obviously). But the caller can ignore
    the error and just proceed with an in-memory key.

Hence, also add nm_utils_secret_key_get() to cache the key. This way,
we only try to actually generate/read the secret-key once.  Since that
might fail and return an in-memory key, we must for future invocations
return the same key, without generating a new one.
parent d0f1dc65
......@@ -2797,57 +2797,103 @@ nm_utils_file_get_contents (int dirfd,
/*****************************************************************************/
guint8 *
nm_utils_secret_key_read (gsize *out_key_len, GError **error)
static gboolean
_secret_key_read (guint8 **out_secret_key,
gsize *out_key_len)
{
guint8 *secret_key = NULL;
guint8 *secret_key;
gboolean success = TRUE;
gsize key_len;
/* out_key_len is not optional, because without it you cannot safely
* access the returned memory. */
*out_key_len = 0;
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, NULL)) {
if (key_len < 16) {
g_set_error_literal (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN,
"Key is too short to be usable");
key_len = 0;
}
if (g_file_get_contents (NMSTATEDIR "/secret_key", (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_clear_g_free (&secret_key);
} else {
mode_t key_mask;
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) {
nm_log_warn (LOGD_CORE, "secret-key: failure reading secret key in \"%s\": %s (generate new key)",
NMSTATEDIR "/secret_key", 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);
/* 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);
if (!nm_utils_random_bytes (secret_key, key_len)) {
g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN,
"Can't get random data to generate secret key");
key_len = 0;
goto out;
}
/* the secret-key is binary. Still, ensure that it's NULL terminated, just like
* g_file_set_contents() does. */
secret_key[32] = '\0';
/* the secret-key is binary. Still, ensure that it's NULL terminated, just like
* g_file_set_contents() does. */
secret_key[32] = '\0';
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;
goto out;
}
key_mask = umask (0077);
if (!g_file_set_contents (NMSTATEDIR "/secret_key", (char *) secret_key, key_len, error)) {
g_prefix_error (error, "Can't write " NMSTATEDIR "/secret_key: ");
key_len = 0;
}
umask (key_mask);
if (!nm_utils_file_set_contents (NMSTATEDIR "/secret_key", (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);
success = FALSE;
goto out;
}
out:
if (key_len) {
*out_key_len = key_len;
return secret_key;
/* 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;
return success;
}
typedef struct {
const guint8 *secret_key;
gsize key_len;
bool is_good:1;
} SecretKeyData;
gboolean
nm_utils_secret_key_get (const guint8 **out_secret_key,
gsize *out_key_len)
{
static volatile const SecretKeyData *secret_key_static;
const SecretKeyData *secret_key;
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;
tmp_success = _secret_key_read (&tmp_secret_key, &tmp_key_len);
if (g_once_init_enter (&init_value)) {
secret_key_data.secret_key = tmp_secret_key;
secret_key_data.key_len = tmp_key_len;
secret_key_data.is_good = tmp_success;
if (g_atomic_pointer_compare_and_exchange (&secret_key_static, NULL, &secret_key_data)) {
g_steal_pointer (&tmp_secret_key);
secret_key = &secret_key_data;
}
g_once_init_leave (&init_value, 1);
}
if (!secret_key)
secret_key = g_atomic_pointer_get (&secret_key_static);
}
g_free (secret_key);
return NULL;
*out_secret_key = secret_key->secret_key;
*out_key_len = secret_key->key_len;
return secret_key->is_good;
}
/*****************************************************************************/
......@@ -3287,7 +3333,7 @@ _set_stable_privacy (NMUtilsStableType stable_type,
const char *ifname,
const char *network_id,
guint32 dad_counter,
guint8 *secret_key,
const guint8 *secret_key,
gsize key_len,
GError **error)
{
......@@ -3386,8 +3432,8 @@ nm_utils_ipv6_addr_set_stable_privacy (NMUtilsStableType stable_type,
guint32 dad_counter,
GError **error)
{
gs_free guint8 *secret_key = NULL;
gsize key_len = 0;
const guint8 *secret_key;
gsize key_len;
g_return_val_if_fail (network_id, FALSE);
......@@ -3397,9 +3443,7 @@ nm_utils_ipv6_addr_set_stable_privacy (NMUtilsStableType stable_type,
return FALSE;
}
secret_key = nm_utils_secret_key_read (&key_len, error);
if (!secret_key)
return FALSE;
nm_utils_secret_key_get (&secret_key, &key_len);
return _set_stable_privacy (stable_type, addr, ifname, network_id, dad_counter,
secret_key, key_len, error);
......@@ -3535,14 +3579,12 @@ nm_utils_hw_addr_gen_stable_eth (NMUtilsStableType stable_type,
const char *current_mac_address,
const char *generate_mac_address_mask)
{
gs_free guint8 *secret_key = NULL;
gsize key_len = 0;
const guint8 *secret_key;
gsize key_len;
g_return_val_if_fail (stable_id, NULL);
secret_key = nm_utils_secret_key_read (&key_len, NULL);
if (!secret_key)
return NULL;
nm_utils_secret_key_get (&secret_key, &key_len);
return _hw_addr_gen_stable_eth (stable_type,
stable_id,
......
......@@ -283,7 +283,8 @@ gboolean nm_utils_file_set_contents (const gchar *filename,
char *nm_utils_machine_id_read (void);
gboolean nm_utils_machine_id_parse (const char *id_str, /*uuid_t*/ guchar *out_uuid);
guint8 *nm_utils_secret_key_read (gsize *out_key_len, GError **error);
gboolean nm_utils_secret_key_get (const guint8 **out_secret_key,
gsize *out_key_len);
const char *nm_utils_get_boot_id (void);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment