From b5e7e48bc185cb4e544efa8be9144e2bda63081c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Oct 2022 15:16:52 +0200 Subject: [PATCH 1/2] glib-aux: add and use nm_uuid_generate_from_strings_old() For a long time we have a function like nm_uuid_generate_from_strings(). This was recently reworked and renamed, but it preserved behavior. Preserving behavior is important for this function, because for the existing users, we need to keep generating the same UUIDs. Originally, this function was a variadic function with NULL sentinel. That means, when you write nm_uuid_generate_from_strings(uuid_type, type_arg, v1, v2, v3, NULL); and v2 happens to be NULL, then v3 is ignored. That is most likely not what the user intended. Maybe they had a bug and v2 should not be NULL. But nm_uuid_generate_from_strings() should not require that all arguments are non-NULL and it should not ignore arguments after the first NULL. For example, one user works around this via uuid = nm_uuid_generate_from_strings_old("ibft", s_hwaddr, s_vlanid ? "V" : "v", s_vlanid ? s_vlanid : "", s_ipaddr ? "A" : "DHCP", s_ipaddr ? s_ipaddr : ""); which is cumbersome and ugly. That will be fixed next, by adding a function that doesn't suffer from this problem. But "this problem" is part of the API of the function, we cannot just change it. Instead, rename it and all users, so they can keep doing the same. New users of course should no longer use the "old" function. --- src/core/devices/nm-device-ethernet.c | 10 ++++------ .../plugins/keyfile/tests/test-keyfile-settings.c | 3 +-- src/libnm-core-impl/nm-keyfile.c | 5 +---- src/libnm-glib-aux/nm-uuid.c | 2 +- src/libnm-glib-aux/nm-uuid.h | 6 ++++++ src/nm-initrd-generator/nmi-ibft-reader.c | 14 ++++++-------- 6 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/core/devices/nm-device-ethernet.c b/src/core/devices/nm-device-ethernet.c index dff6466a2e..414bfa7b4b 100644 --- a/src/core/devices/nm-device-ethernet.c +++ b/src/core/devices/nm-device-ethernet.c @@ -1725,12 +1725,10 @@ new_default_connection(NMDevice *self) /* Create a stable UUID. The UUID is also the Network_ID for stable-privacy addr-gen-mode, * thus when it changes we will also generate different IPv6 addresses. */ - uuid = nm_uuid_generate_from_strings(NM_UUID_TYPE_VERSION3, - &nm_uuid_ns_1, - "default-wired", - nm_utils_machine_id_str(), - defname, - perm_hw_addr ?: iface); + uuid = nm_uuid_generate_from_strings_old("default-wired", + nm_utils_machine_id_str(), + defname, + perm_hw_addr ?: iface); g_object_set(setting, NM_SETTING_CONNECTION_ID, diff --git a/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c b/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c index eec529ce6c..28923ae6e1 100644 --- a/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c +++ b/src/core/settings/plugins/keyfile/tests/test-keyfile-settings.c @@ -2296,8 +2296,7 @@ test_read_missing_id_uuid(void) gs_free char *expected_uuid = NULL; const char *FILENAME = TEST_KEYFILES_DIR "/Test_Missing_ID_UUID"; - expected_uuid = - nm_uuid_generate_from_strings(NM_UUID_TYPE_VERSION3, &nm_uuid_ns_1, "keyfile", FILENAME); + expected_uuid = nm_uuid_generate_from_strings_old("keyfile", FILENAME); connection = keyfile_read_connection_from_file(FILENAME); diff --git a/src/libnm-core-impl/nm-keyfile.c b/src/libnm-core-impl/nm-keyfile.c index d8dd27821f..ae6d892c28 100644 --- a/src/libnm-core-impl/nm-keyfile.c +++ b/src/libnm-core-impl/nm-keyfile.c @@ -3800,10 +3800,7 @@ nm_keyfile_read_ensure_uuid(NMConnection *connection, const char *fallback_uuid_ if (nm_setting_connection_get_uuid(s_con)) return FALSE; - hashed_uuid = nm_uuid_generate_from_strings(NM_UUID_TYPE_VERSION3, - &nm_uuid_ns_1, - "keyfile", - fallback_uuid_seed); + hashed_uuid = nm_uuid_generate_from_strings_old("keyfile", fallback_uuid_seed); g_object_set(s_con, NM_SETTING_CONNECTION_UUID, hashed_uuid, NULL); return TRUE; } diff --git a/src/libnm-glib-aux/nm-uuid.c b/src/libnm-glib-aux/nm-uuid.c index ae0f7233e7..34c3be335b 100644 --- a/src/libnm-glib-aux/nm-uuid.c +++ b/src/libnm-glib-aux/nm-uuid.c @@ -12,7 +12,7 @@ const NMUuid nm_uuid_ns_zero = NM_UUID_INIT(00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00); -/* arbitrarily chosen namespace UUID for some uses of nm_uuid_generate_from_strings(). +/* arbitrarily chosen namespace UUID for some uses of nm_uuid_generate_from_strings_old(). * Try not to re-use this namespace, instead, generate a unique one. */ const NMUuid nm_uuid_ns_1 = NM_UUID_INIT(b4, 25, e9, fb, 75, 98, 44, b4, 9e, 3b, 5a, 2e, 3a, aa, 49, 05); diff --git a/src/libnm-glib-aux/nm-uuid.h b/src/libnm-glib-aux/nm-uuid.h index 770d7af1ae..4e0941ffb7 100644 --- a/src/libnm-glib-aux/nm-uuid.h +++ b/src/libnm-glib-aux/nm-uuid.h @@ -130,6 +130,12 @@ char *nm_uuid_generate_from_strings_strv(NMUuidType uuid_type, #define nm_uuid_generate_from_strings(uuid_type, type_args, ...) \ nm_uuid_generate_from_strings_strv((uuid_type), (type_args), NM_MAKE_STRV(__VA_ARGS__)) +/* Legacy function. Don't use for new code. */ +#define nm_uuid_generate_from_strings_old(uuid_type, type_args, ...) \ + nm_uuid_generate_from_strings_strv(NM_UUID_TYPE_VERSION3, \ + &nm_uuid_ns_1, \ + NM_MAKE_STRV(__VA_ARGS__)) + /*****************************************************************************/ #endif /* __NM_UUID_H__ */ diff --git a/src/nm-initrd-generator/nmi-ibft-reader.c b/src/nm-initrd-generator/nmi-ibft-reader.c index c0915a8f01..2a3bbdfbcb 100644 --- a/src/nm-initrd-generator/nmi-ibft-reader.c +++ b/src/nm-initrd-generator/nmi-ibft-reader.c @@ -307,14 +307,12 @@ connection_setting_add(GHashTable *nic, s_index ? " " : "", s_index ? s_index : ""); - uuid = nm_uuid_generate_from_strings(NM_UUID_TYPE_VERSION3, - &nm_uuid_ns_1, - "ibft", - s_hwaddr, - s_vlanid ? "V" : "v", - s_vlanid ? s_vlanid : "", - s_ipaddr ? "A" : "DHCP", - s_ipaddr ? s_ipaddr : ""); + uuid = nm_uuid_generate_from_strings_old("ibft", + s_hwaddr, + s_vlanid ? "V" : "v", + s_vlanid ? s_vlanid : "", + s_ipaddr ? "A" : "DHCP", + s_ipaddr ? s_ipaddr : ""); s_con = (NMSetting *) nm_connection_get_setting_connection(connection); if (!s_con) { From 2d8651ba91dc1fd30f08f7ebc2cb0f8bab4d958d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Oct 2022 15:13:03 +0200 Subject: [PATCH 2/2] glib-aux: extend nm_uuid_generate_from_strings() to honor NULL values When you call nm_uuid_generate_from_strings_strv(uuid_type, type_arg, v1, v2); you'd probably expect that both values are honored in some way. However, if v1 happened to be NULL, then previously v2 would be ignored. Extend nm_uuid_generate_from_strings() to accept also NULL values and pass on the length. Currently, there are no users of nm_uuid_generate_from_strings(), so nobody is affected by this change. Also extend nm_uuid_generate_from_strings_strv() to take a length argument. It still accepts "-1" to take the input strv as a NULL terminated array. If a positive length is provided to nm_uuid_generate_from_strings_strv(), it hashes the same UUID as the respective NULL terminated array. But of course only, if there is no NULL inside the array. If there are any NULLs, a distinct UUID gets generated. --- src/libnm-core-impl/tests/test-general.c | 103 ++++++++++++++++------- src/libnm-glib-aux/nm-uuid.c | 38 ++++++++- src/libnm-glib-aux/nm-uuid.h | 13 ++- 3 files changed, 119 insertions(+), 35 deletions(-) diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c index 904d89f337..6a85fe4c79 100644 --- a/src/libnm-core-impl/tests/test-general.c +++ b/src/libnm-core-impl/tests/test-general.c @@ -7946,16 +7946,50 @@ test_nm_utils_uuid_generate_from_string(void) /*****************************************************************************/ static void -_check_uuid(NMUuidType uuid_type, - const NMUuid *type_arg, - const char *expected_uuid, - const char *str, - gssize slen, - char *uuid_test) +_check_uuid(NMUuidType uuid_type, + const NMUuid *type_arg, + const char *expected_uuid, + const char *str, + gssize slen, + const char *const *strv, + gssize strv_len) { + gs_free char *uuid_test = NULL; + gs_free char *uuid_test2 = NULL; + gboolean uuid_test2_valid = TRUE; + + g_assert(str); + g_assert(strv_len < 0 || strv); + + uuid_test = nm_uuid_generate_from_strings_strv(uuid_type, type_arg, strv, strv_len); + g_assert(uuid_test); g_assert(nm_uuid_is_normalized(uuid_test)); - g_assert(str); + + if (strv_len < 0 && strv) { + uuid_test2 = + nm_uuid_generate_from_strings_strv(uuid_type, type_arg, strv, NM_PTRARRAY_LEN(strv)); + } else if (strv_len >= 0) { + gssize l = nm_strv_find_first(strv, strv_len, NULL); + gs_free const char **strv2 = nm_strv_dup_packed(strv, l < 0 ? strv_len : l); + + uuid_test2 = nm_uuid_generate_from_strings_strv(uuid_type, + type_arg, + strv2 ?: NM_STRV_EMPTY_CC(), + -1); + if (l >= 0) { + /* there are NULL strings. The result won't be match. */ + uuid_test2_valid = FALSE; + } + } + if (uuid_test2) { + if (uuid_test2_valid) + g_assert_cmpstr(uuid_test, ==, uuid_test2); + else { + g_assert(nm_uuid_is_normalized(uuid_test)); + g_assert_cmpstr(uuid_test, !=, uuid_test2); + } + } if (!nm_streq(uuid_test, expected_uuid)) { g_error("UUID test failed (1): text=%s, len=%lld, expected=%s, uuid_test=%s", @@ -7978,24 +8012,19 @@ _check_uuid(NMUuidType uuid_type, expected_uuid, uuid_test); } - g_free(uuid_test); } -#define check_uuid(uuid_type, type_arg, expected_uuid, str, ...) \ - ({ \ - const NMUuidType _uuid_type = (uuid_type); \ - const NMUuid *_type_arg = type_arg; \ - const char *_expected_uuid = (expected_uuid); \ - const char *_str = (str); \ - const gsize _strlen = NM_STRLEN(str); \ - \ - _check_uuid( \ - _uuid_type, \ - _type_arg, \ - _expected_uuid, \ - _str, \ - _strlen, \ - nm_uuid_generate_from_strings_strv(_uuid_type, _type_arg, NM_MAKE_STRV(__VA_ARGS__))); \ +#define check_uuid(uuid_type, type_arg, expected_uuid, str, ...) \ + ({ \ + const NMUuidType _uuid_type = (uuid_type); \ + const NMUuid *_type_arg = type_arg; \ + const char *_expected_uuid = (expected_uuid); \ + const char *_str = (str); \ + const gsize _strlen = NM_STRLEN(str); \ + const char *const *_strv = NM_MAKE_STRV(__VA_ARGS__); \ + const gssize _strv_len = NM_NARG(__VA_ARGS__); \ + \ + _check_uuid(_uuid_type, _type_arg, _expected_uuid, _str, _strlen, _strv, _strv_len); \ }) static void @@ -8023,17 +8052,19 @@ test_nm_utils_uuid_generate_from_strings(void) "457229f4-fe49-32f5-8b09-c531d81f44d9", "x", 1, - nm_uuid_generate_from_strings_strv(NM_UUID_TYPE_VERSION3, &nm_uuid_ns_1, NULL)); - check_uuid(NM_UUID_TYPE_VERSION3, - &nm_uuid_ns_1, - "b07c334a-399b-32de-8d50-58e4e08f98e3", - "", - NULL); + NULL, + -1); + check_uuid(NM_UUID_TYPE_VERSION3, &nm_uuid_ns_1, "b07c334a-399b-32de-8d50-58e4e08f98e3", ""); check_uuid(NM_UUID_TYPE_VERSION3, &nm_uuid_ns_1, "b8a426cb-bcb5-30a3-bd8f-6786fea72df9", "\0", ""); + check_uuid(NM_UUID_TYPE_VERSION3, + &nm_uuid_ns_1, + "9232afda-85fc-3b8f-8736-4f99c8d5db9c", + "_n", + NULL); check_uuid(NM_UUID_TYPE_VERSION3, &nm_uuid_ns_1, "12a4a982-7aae-39e1-951e-41aeb1250959", @@ -8074,6 +8105,13 @@ test_nm_utils_uuid_generate_from_strings(void) "a\0a\0", "a", "a"); + check_uuid(NM_UUID_TYPE_VERSION3, + &nm_uuid_ns_1, + "f36cec99-1db8-3baa-8c3f-13e13d980318", + "a\0a\0001_1n", + "a", + NULL, + "a"); check_uuid(NM_UUID_TYPE_VERSION3, &nm_uuid_ns_1, "fd698d86-1b60-3ebe-855f-7aada9950a8d", @@ -8117,6 +8155,13 @@ test_nm_utils_uuid_generate_from_strings(void) "\0b\0", "", "b"); + check_uuid(NM_UUID_TYPE_VERSION5, + _uuid(NM_UUID_NS_URL), + "db3dfd17-c785-509d-a0ca-740fdd68dc68", + "\0b\00011_n", + "", + "b", + NULL); check_uuid(NM_UUID_TYPE_VERSION3, _uuid(NM_UUID_NS_URL), "916dcdd8-5042-3b9b-9763-4312a31e5735", diff --git a/src/libnm-glib-aux/nm-uuid.c b/src/libnm-glib-aux/nm-uuid.c index 34c3be335b..cdfa5f629e 100644 --- a/src/libnm-glib-aux/nm-uuid.c +++ b/src/libnm-glib-aux/nm-uuid.c @@ -413,6 +413,10 @@ nm_uuid_generate_from_string_str(const char *s, * @type_args: the namespace UUID. * @strv: (allow-none): the strv list to hash. Can be NULL, in which * case the result is different from an empty array. + * @len: if negative, @strv is a NULL terminated array. Otherwise, + * it is the length of the strv array. In the latter case it may + * also contain NULL strings. The result hashes differently depending + * on whether we have a NULL terminated strv array or given length. * * Returns a @uuid_type UUID based on the concatenated C strings. * It does not simply concatenate them, but also includes the @@ -425,13 +429,43 @@ nm_uuid_generate_from_string_str(const char *s, char * nm_uuid_generate_from_strings_strv(NMUuidType uuid_type, const NMUuid *type_args, - const char *const *strv) + const char *const *strv, + gssize len) { nm_auto_str_buf NMStrBuf str = NM_STR_BUF_INIT_A(NM_UTILS_GET_NEXT_REALLOC_SIZE_232, TRUE); gsize slen; const char *s; - if (!strv) { + if (len >= 0) { + gboolean has_nulls = FALSE; + gssize i; + + nm_assert(len == 0 || strv); + + for (i = 0; i < len; i++) { + if (strv[i]) + nm_str_buf_append_len(&str, strv[i], strlen(strv[i]) + 1u); + else + has_nulls = TRUE; + } + if (has_nulls) { + /* We either support a NULL terminated strv array, or a ptr array of fixed + * length (@len argument). + * + * If there are no NULLs within the first @len strings, then the result + * is the same. If there are any NULL strings, we need to encode that + * in a unique way. We do that by appending a bitmap of the elements + * whether they were set, plus one 'n' character (without NUL termination). + * None of the other branches below hashes to that, so this will uniquely + * encoded the NULL strings. + */ + for (i = 0; i < len; i++) + nm_str_buf_append_c(&str, strv[i] ? '1' : '_'); + nm_str_buf_append_c(&str, 'n'); + } + slen = str.len; + s = nm_str_buf_get_str_unsafe(&str); + } else if (!strv) { /* NULL is treated differently from an empty strv. We achieve that * by using a non-empty, non-NUL terminated string (which cannot happen * in the other cases). */ diff --git a/src/libnm-glib-aux/nm-uuid.h b/src/libnm-glib-aux/nm-uuid.h index 4e0941ffb7..d26616cf4f 100644 --- a/src/libnm-glib-aux/nm-uuid.h +++ b/src/libnm-glib-aux/nm-uuid.h @@ -125,16 +125,21 @@ char *nm_uuid_generate_from_string_str(const char *s, char *nm_uuid_generate_from_strings_strv(NMUuidType uuid_type, const NMUuid *type_args, - const char *const *strv); + const char *const *strv, + gssize len); -#define nm_uuid_generate_from_strings(uuid_type, type_args, ...) \ - nm_uuid_generate_from_strings_strv((uuid_type), (type_args), NM_MAKE_STRV(__VA_ARGS__)) +#define nm_uuid_generate_from_strings(uuid_type, type_args, ...) \ + nm_uuid_generate_from_strings_strv((uuid_type), \ + (type_args), \ + NM_MAKE_STRV(__VA_ARGS__), \ + NM_NARG(__VA_ARGS__)) /* Legacy function. Don't use for new code. */ #define nm_uuid_generate_from_strings_old(uuid_type, type_args, ...) \ nm_uuid_generate_from_strings_strv(NM_UUID_TYPE_VERSION3, \ &nm_uuid_ns_1, \ - NM_MAKE_STRV(__VA_ARGS__)) + NM_MAKE_STRV(__VA_ARGS__), \ + -1) /*****************************************************************************/