From 207cf3d5d4c9147ef52e1d54840a3eb90ca4c1ba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 2 May 2021 22:42:51 +0200 Subject: [PATCH] libnm: normalize "connection.uuid" For NetworkManager profiles, "connection.uuid" is the identifier of the profile. It is supposed to be a UUID, however: - the UUID was not ensured to be all-lower case. We should make sure that our UUIDs are in a consistent manner, so that users can rely on the format of the string. - the UUID was never actually interpreted as a UUID. It only was some opaque string, that we use as identifier. We had nm_utils_is_uuid() which checks that the format is valid, however that did not fully validate the format, like it would accept "----7daf444dd78741a59e1ef1b3c8b1c0e8" and "549fac10a25f4bcc912d1ae688c2b4987daf444d" (40 hex characters). Both invalid UUIDs and non-normalized UUID should be normalized. We don't want to break existing profiles that use such UUIDs, thus we don't outright reject them. Let's instead mangle them during nm_connection_normalize(). --- NEWS | 3 + src/libnm-core-impl/nm-connection.c | 18 ++++-- src/libnm-core-impl/nm-setting-connection.c | 16 ++++- src/libnm-core-impl/tests/test-setting.c | 64 +++++++++++++++++--- src/libnm-glib-aux/nm-uuid.c | 65 +++++++++++++++++++++ src/libnm-glib-aux/nm-uuid.h | 4 ++ 6 files changed, 156 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index 368e8de6b3..62e9abd072 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,9 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! mode). * nmcli: fix setting property aliases to empty value to reset the default value. +* Enforce valid "connection.uuid" by normalizing the string to lower + case. This changes the UUID of existing profiles that had an invalid, + non-normalized value. ============================================= NetworkManager-1.30 diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 8abfa65e51..a8435c909c 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -704,14 +704,24 @@ static gboolean _normalize_connection_uuid(NMConnection *self) { NMSettingConnection *s_con = nm_connection_get_setting_connection(self); - char uuid[37]; + char uuid_normalized[37]; + const char * uuid; nm_assert(s_con); - if (nm_setting_connection_get_uuid(s_con)) - return FALSE; + uuid = nm_setting_connection_get_uuid(s_con); - g_object_set(s_con, NM_SETTING_CONNECTION_UUID, nm_uuid_generate_random_str_arr(uuid), NULL); + if (uuid) { + gboolean uuid_is_normalized; + + if (!nm_uuid_is_valid_nm(uuid, &uuid_is_normalized, uuid_normalized)) + return nm_assert_unreachable_val(FALSE); + if (!uuid_is_normalized) + return FALSE; + } else + nm_uuid_generate_random_str_arr(uuid_normalized); + + g_object_set(s_con, NM_SETTING_CONNECTION_UUID, uuid_normalized, NULL); return TRUE; } diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index 8b6bd4e089..8693c85363 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -8,6 +8,7 @@ #include "nm-setting-connection.h" +#include "libnm-glib-aux/nm-uuid.h" #include "libnm-core-aux-intern/nm-common-macros.h" #include "nm-utils.h" #include "nm-utils-private.h" @@ -1065,6 +1066,7 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) const char * normerr_missing_slave_type = NULL; const char * normerr_missing_slave_type_port = NULL; gboolean normerr_base_setting = FALSE; + gboolean uuid_was_normalized = FALSE; if (!priv->id) { g_set_error_literal(error, @@ -1088,7 +1090,7 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) return FALSE; } - if (priv->uuid && !nm_utils_is_uuid(priv->uuid)) { + if (priv->uuid && !nm_uuid_is_valid_nm(priv->uuid, &uuid_was_normalized, NULL)) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, @@ -1460,6 +1462,18 @@ after_interface_name: } } + if (uuid_was_normalized) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_MISSING_PROPERTY, + _("UUID needs normalization")); + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_UUID); + return NM_SETTING_VERIFY_NORMALIZABLE; + } + return TRUE; } diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index c175a816c8..6671536537 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -238,6 +238,8 @@ _do_test_connection_uuid(NMConnection *con, const char *uuid, const char *expect NMSettingConnection *s_con; gs_free char * uuid_old = NULL; gboolean success; + gboolean is_normalized; + char uuid_normalized[37]; nmtst_assert_connection_verifies_without_normalization(con); @@ -257,6 +259,9 @@ _do_test_connection_uuid(NMConnection *con, const char *uuid, const char *expect if (nm_streq0(uuid, expected_uuid)) { nmtst_assert_connection_verifies_without_normalization(con); g_assert(nm_utils_is_uuid(uuid)); + g_assert(nm_uuid_is_valid(uuid)); + g_assert(nm_uuid_is_valid_nm(uuid, &is_normalized, NULL)); + g_assert(!is_normalized); } else if (!expected_uuid) { gs_free_error GError *error = NULL; @@ -265,11 +270,35 @@ _do_test_connection_uuid(NMConnection *con, const char *uuid, const char *expect g_assert_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY); g_assert(!nm_utils_is_uuid(uuid)); - } else - g_assert_not_reached(); + g_assert(!nm_uuid_is_valid(uuid)); + g_assert(!nm_uuid_is_valid_nmlegacy(uuid)); + g_assert(!nm_uuid_is_valid_nm(uuid, NULL, NULL)); + } else { + gs_free_error GError *error = NULL; + + nmtst_assert_connection_verifies_and_normalizable(con); + + success = nm_connection_verify(con, &error); + nmtst_assert_success(success, error); + + if (!nmtst_connection_normalize(con)) + g_assert_not_reached(); + + g_assert_cmpstr(expected_uuid, ==, nm_setting_connection_get_uuid(s_con)); + g_assert(nm_uuid_is_valid(expected_uuid)); + + g_assert(nm_utils_is_uuid(uuid)); + g_assert(nm_uuid_is_valid_nmlegacy(uuid)); + g_assert(nm_uuid_is_valid_nm(uuid, &is_normalized, uuid_normalized)); + + g_assert_cmpstr(expected_uuid, ==, uuid_normalized); + } g_object_set(s_con, NM_SETTING_CONNECTION_UUID, uuid_old, NULL); nmtst_assert_connection_verifies_without_normalization(con); + + if (expected_uuid && !nm_streq(expected_uuid, uuid)) + _do_test_connection_uuid(con, expected_uuid, expected_uuid); } static void @@ -286,19 +315,36 @@ test_connection_uuid(void) #define _do_test_connection_uuid_good(con, uuid) \ _do_test_connection_uuid((con), "" uuid "", "" uuid "") +#define _do_test_connection_uuid_norm(con, uuid, expected_uuid) \ + _do_test_connection_uuid((con), "" uuid "", "" expected_uuid "") + _do_test_connection_uuid_bad(con, "x1e775e3-a316-4eb2-b4d8-4b0f2bcaea53"); _do_test_connection_uuid_bad(con, "1e775e3aa3164eb2b4d84b0f2bcaea53abcdabc"); _do_test_connection_uuid_bad(con, "1e775e3aa3164eb2b4d84b0f2bcaea53abcdabcdd"); _do_test_connection_uuid_good(con, "a1e775e3-a316-4eb2-b4d8-4b0f2bcaea53"); - _do_test_connection_uuid_good(con, "A1E775e3-a316-4eb2-b4d8-4b0f2bcaea53"); - _do_test_connection_uuid_good(con, "A1E775E3-A316-4EB2-B4D8-4B0F2BCAEA53"); - _do_test_connection_uuid_good(con, "-1e775e3aa316-4eb2-b4d8-4b0f2bcaea53"); - _do_test_connection_uuid_good(con, "----1e775e3aa3164eb2b4d84b0f2bcaea53"); - _do_test_connection_uuid_good(con, "1e775e3aa3164eb2b4d84b0f2bcaea53abcdabcd"); - _do_test_connection_uuid_good(con, "1e775e3Aa3164eb2b4d84b0f2bcaea53abcdabcd"); - _do_test_connection_uuid_good(con, "1E775E3AA3164EB2B4D84B0F2BCAEA53ABCDABCD"); + _do_test_connection_uuid_norm(con, + "A1E775e3-a316-4eb2-b4d8-4b0f2bcaea53", + "a1e775e3-a316-4eb2-b4d8-4b0f2bcaea53"); + _do_test_connection_uuid_norm(con, + "A1E775E3-A316-4EB2-B4D8-4B0F2BCAEA53", + "a1e775e3-a316-4eb2-b4d8-4b0f2bcaea53"); + _do_test_connection_uuid_norm(con, + "-1e775e3aa316-4eb2-b4d8-4b0f2bcaea53", + "bdd73688-5c87-5454-917d-f5c3faed39c0"); + _do_test_connection_uuid_norm(con, + "----1e775e3aa3164eb2b4d84b0f2bcaea53", + "8a232814-c6cf-54c9-9384-71a60011d0b2"); + _do_test_connection_uuid_norm(con, + "1e775e3aa3164eb2b4d84b0f2bcaea53abcdabcd", + "ae35a4a8-4029-5770-9fa4-d79a672874c3"); + _do_test_connection_uuid_norm(con, + "1e775e3Aa3164eb2b4d84b0f2bcaea53abcdabcd", + "ae35a4a8-4029-5770-9fa4-d79a672874c3"); + _do_test_connection_uuid_norm(con, + "1E775E3AA3164EB2B4D84B0F2BCAEA53ABCDABCD", + "ae35a4a8-4029-5770-9fa4-d79a672874c3"); } /*****************************************************************************/ diff --git a/src/libnm-glib-aux/nm-uuid.c b/src/libnm-glib-aux/nm-uuid.c index 18badc46d8..bc90352084 100644 --- a/src/libnm-glib-aux/nm-uuid.c +++ b/src/libnm-glib-aux/nm-uuid.c @@ -158,6 +158,9 @@ nm_uuid_is_valid_nmlegacy(const char *str) p++; } + /* While we accept here bogus strings as UUIDs, they must contain only + * hexdigits and '-', and they must be eithr 36 or 40 chars long. */ + if ((num_dashes == 4) && (p - str == 36)) return TRUE; @@ -170,6 +173,68 @@ nm_uuid_is_valid_nmlegacy(const char *str) /*****************************************************************************/ +gboolean +nm_uuid_is_valid_nm(const char *str, + gboolean * out_normalized, + char * out_normalized_str /* [static 37] */) +{ + NMUuid uuid; + gboolean is_normalized; + + if (!str) + return FALSE; + + if (nm_uuid_parse_full(str, &uuid, &is_normalized)) { + if (is_normalized) { + /* @str is a normalized (lower-case), valid UUID. Nothing to normalize, + * and return success. */ + NM_SET_OUT(out_normalized, FALSE); + return TRUE; + } + + /* @str is a valid UUID, but not normalized. That means that it's + * upper case. Normalize the UUID. */ + NM_SET_OUT(out_normalized, TRUE); + if (out_normalized_str) + nm_uuid_unparse(&uuid, out_normalized_str); + return TRUE; + } + + if (nm_uuid_is_valid_nmlegacy(str)) { + /* This is not a valid UUID, but something that we used to + * accept according to nm_uuid_is_valid_nmlegacy(). + * + * Normalize it by sha1 hashing the string. Upper case characters + * are made lower case first. */ + NM_SET_OUT(out_normalized, TRUE); + if (out_normalized_str) { + char str_lower[40 + 1]; + int i; + + nm_assert(strlen(str) < G_N_ELEMENTS(str_lower)); + + /* normalize first to lower-case. */ + g_strlcpy(str_lower, str, sizeof(str_lower)); + for (i = 0; str_lower[i]; i++) + str_lower[i] = g_ascii_tolower(str_lower[i]); + + /* The namespace UUID is chosen randomly. */ + nm_uuid_generate_from_string(&uuid, + str_lower, + -1, + NM_UUID_TYPE_VERSION5, + "4e72f709-ca95-4405-9053-1f43294a618c"); + nm_uuid_unparse(&uuid, out_normalized_str); + } + return TRUE; + } + + /* UUID is not valid. */ + return FALSE; +} + +/*****************************************************************************/ + gboolean nm_uuid_is_null(const NMUuid *uuid) { diff --git a/src/libnm-glib-aux/nm-uuid.h b/src/libnm-glib-aux/nm-uuid.h index 90727532eb..afa16891ce 100644 --- a/src/libnm-glib-aux/nm-uuid.h +++ b/src/libnm-glib-aux/nm-uuid.h @@ -43,6 +43,10 @@ nm_uuid_is_valid(const char *str) gboolean nm_uuid_is_valid_nmlegacy(const char *str); +gboolean nm_uuid_is_valid_nm(const char *str, + gboolean * out_normalized, + char * out_normalized_str /* [static 37] */); + /*****************************************************************************/ char *nm_uuid_generate_random_str(char buf[static 37]);