From 3f8431f0693967547d39c711328f72c9c9b3289b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Nov 2023 19:53:44 +0100 Subject: [PATCH] libnm: refactor "ipv6" argument of _nm_utils_dns_option_validate() _nm_utils_dns_option_validate() allows specifying the address family, and filters based on that. Note that all options are valid for IPv6, but some are not valid for IPv4. It's not obvious, that such filtering is only performed if "option_descs" argument is provied. Otherwise, the "ipv6" argument is ignored. Regardless, it's also confusing to have a boolean "ipv6". When most callers don't want a filtering based on the address family. They actually don't want any filtering at all, as they don't pass an "option_descs". At the same time passing a TRUE/FALSE "ipv6" is redundant and ignored. It should be possible, to explicitly not select an address family (as it's ignored anyway). Instead, make the "gboolean ipv6" argument an "int addr_family". Selecting AF_UNSPEC means clearly to accept any address family. --- src/core/nm-config-data.c | 4 +- src/libnm-core-impl/nm-setting-ip-config.c | 6 +- src/libnm-core-impl/nm-utils.c | 29 ++++++--- src/libnm-core-impl/tests/test-general.c | 72 +++++++++++++--------- src/libnm-core-intern/nm-core-internal.h | 2 +- 5 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index ed6d8381f2..468e56b821 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -1219,7 +1219,7 @@ load_global_dns(GKeyFile *keyfile, gboolean internal) if (strv) { nm_strv_cleanup(strv, TRUE, TRUE, TRUE); for (i = 0, j = 0; strv[i]; i++) { - if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, TRUE, NULL)) + if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, AF_UNSPEC, NULL)) strv[j++] = strv[i]; else g_free(strv[i]); @@ -1453,7 +1453,7 @@ nm_global_dns_config_from_dbus(const GValue *value, GError **error) nm_strv_cleanup(strv, TRUE, TRUE, TRUE); for (i = 0, j = 0; strv && strv[i]; i++) { - if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, TRUE, NULL)) + if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, AF_UNSPEC, NULL)) strv[j++] = strv[i]; else g_free(strv[i]); diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 06e250dcbb..720048c3bb 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -4434,7 +4434,7 @@ nm_setting_ip_config_next_valid_dns_option(NMSettingIPConfig *setting, guint idx if (_nm_utils_dns_option_validate(priv->dns_options->pdata[idx], NULL, NULL, - NM_IS_SETTING_IP6_CONFIG(setting), + NM_SETTING_IP_CONFIG_GET_ADDR_FAMILY(setting), _nm_utils_dns_option_descs)) return idx; } @@ -4462,7 +4462,7 @@ nm_setting_ip_config_add_dns_option(NMSettingIPConfig *setting, const char *dns_ g_return_val_if_fail(dns_option != NULL, FALSE); g_return_val_if_fail(dns_option[0] != '\0', FALSE); - if (!_nm_utils_dns_option_validate(dns_option, NULL, NULL, FALSE, NULL)) + if (!_nm_utils_dns_option_validate(dns_option, NULL, NULL, AF_UNSPEC, NULL)) return FALSE; priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); @@ -6208,7 +6208,7 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps else priv->dns_options = g_ptr_array_new_with_free_func(g_free); for (i = 0; strv[i]; i++) { - if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, FALSE, NULL) + if (_nm_utils_dns_option_validate(strv[i], NULL, NULL, AF_UNSPEC, NULL) && _nm_utils_dns_option_find_idx(priv->dns_options, strv[i]) < 0) g_ptr_array_add(priv->dns_options, g_strdup(strv[i])); } diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index c476d18661..e59e572ef3 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -4694,7 +4694,7 @@ _nm_utils_strstrdictkey_create(const char *v1, const char *v2) static gboolean validate_dns_option(const char *name, gboolean numeric, - gboolean ipv6, + int addr_family, const NMUtilsDNSOptionDesc *option_descs) { const NMUtilsDNSOptionDesc *desc; @@ -4703,8 +4703,15 @@ validate_dns_option(const char *name, return !!*name; for (desc = option_descs; desc->name; desc++) { - if (nm_streq(name, desc->name) && numeric == desc->numeric && (!desc->ipv6_only || ipv6)) - return TRUE; + if (!nm_streq(name, desc->name)) + continue; + if ((!!numeric) != (!!desc->numeric)) + continue; + if (addr_family != AF_UNSPEC) { + if (desc->ipv6_only && addr_family != AF_INET6) + continue; + } + return TRUE; } return FALSE; @@ -4715,7 +4722,9 @@ validate_dns_option(const char *name, * @option: option string * @out_name: (out) (optional) (nullable): the option name * @out_value: (out) (optional): the option value - * @ipv6: whether the option refers to a IPv6 configuration + * @addr_family: AF_INET/AF_INET6 to only allow options for the specified address + * family. AF_UNSPEC to allow either. This argument is ignored, if @option_descs + * is NULL. * @option_descs: (nullable): an array of NMUtilsDNSOptionDesc which describes the * valid options * @@ -4731,7 +4740,7 @@ gboolean _nm_utils_dns_option_validate(const char *option, char **out_name, long *out_value, - gboolean ipv6, + int addr_family, const NMUtilsDNSOptionDesc *option_descs) { gs_free char *option0_free = NULL; @@ -4742,6 +4751,8 @@ _nm_utils_dns_option_validate(const char *option, g_return_val_if_fail(option != NULL, FALSE); + nm_assert_addr_family_or_unspec(addr_family); + NM_SET_OUT(out_name, NULL); NM_SET_OUT(out_value, -1); @@ -4750,7 +4761,7 @@ _nm_utils_dns_option_validate(const char *option, delim = strchr(option, ':'); if (!delim) { - if (!validate_dns_option(option, FALSE, ipv6, option_descs)) + if (!validate_dns_option(option, FALSE, addr_family, option_descs)) return FALSE; NM_SET_OUT(out_name, g_strdup(option)); return TRUE; @@ -4765,7 +4776,7 @@ _nm_utils_dns_option_validate(const char *option, option0 = nm_strndup_a(300, option, delim - option, &option0_free); - if (!validate_dns_option(option0, TRUE, ipv6, option_descs)) + if (!validate_dns_option(option0, TRUE, addr_family, option_descs)) return FALSE; option1_num = _nm_utils_ascii_str_to_int64(option1, 10, 0, G_MAXINT32, -1); @@ -4794,13 +4805,13 @@ _nm_utils_dns_option_find_idx(GPtrArray *array, const char *option) gs_free char *option_name = NULL; guint i; - if (!_nm_utils_dns_option_validate(option, &option_name, NULL, FALSE, NULL)) + if (!_nm_utils_dns_option_validate(option, &option_name, NULL, AF_UNSPEC, NULL)) return -1; for (i = 0; i < array->len; i++) { gs_free char *tmp_name = NULL; - if (_nm_utils_dns_option_validate(array->pdata[i], &tmp_name, NULL, FALSE, NULL)) { + if (_nm_utils_dns_option_validate(array->pdata[i], &tmp_name, NULL, AF_UNSPEC, NULL)) { if (nm_streq(tmp_name, option_name)) return i; } diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c index d4ee8616eb..a873861871 100644 --- a/src/libnm-core-impl/tests/test-general.c +++ b/src/libnm-core-impl/tests/test-general.c @@ -8803,23 +8803,35 @@ test_nm_ptrarray_len(void) static void test_nm_utils_dns_option_validate_do(char *option, - gboolean ipv6, + int addr_family, const NMUtilsDNSOptionDesc *descs, gboolean exp_result, char *exp_name, gboolean exp_value) { - char *name; - long value = 0; - gboolean result; + gs_free char *name = NULL; + long value = 0; + gboolean result; - result = _nm_utils_dns_option_validate(option, &name, &value, ipv6, descs); + if (!descs) { + g_assert(addr_family == AF_UNSPEC); + addr_family = nmtst_rand_select(AF_UNSPEC, AF_INET, AF_INET6); + } + + result = _nm_utils_dns_option_validate(option, &name, &value, addr_family, descs); g_assert(result == exp_result); g_assert_cmpstr(name, ==, exp_name); g_assert(value == exp_value); - g_free(name); + nm_clear_g_free(&name); + + if (result && descs) { + result = _nm_utils_dns_option_validate(option, &name, &value, AF_UNSPEC, descs); + g_assert(result == exp_result); + g_assert_cmpstr(name, ==, exp_name); + g_assert(value == exp_value); + } } static const NMUtilsDNSOptionDesc opt_descs[] = { @@ -8833,34 +8845,34 @@ static const NMUtilsDNSOptionDesc opt_descs[] = { static void test_nm_utils_dns_option_validate(void) { - /* opt ipv6 descs result name value */ - test_nm_utils_dns_option_validate_do("", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do(":", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do(":1", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do(":val", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt", FALSE, NULL, TRUE, "opt", -1); - test_nm_utils_dns_option_validate_do("opt:", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt:12", FALSE, NULL, TRUE, "opt", 12); - test_nm_utils_dns_option_validate_do("opt:12 ", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt:val", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt:2val", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt:2:3", FALSE, NULL, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt-6", FALSE, NULL, TRUE, "opt-6", -1); + /* (opt, addr_family, descs, result, name, value) */ + test_nm_utils_dns_option_validate_do("", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do(":", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do(":1", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do(":val", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt", AF_UNSPEC, NULL, TRUE, "opt", -1); + test_nm_utils_dns_option_validate_do("opt:", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt:12", AF_UNSPEC, NULL, TRUE, "opt", 12); + test_nm_utils_dns_option_validate_do("opt:12 ", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt:val", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt:2val", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt:2:3", AF_UNSPEC, NULL, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt-6", AF_UNSPEC, NULL, TRUE, "opt-6", -1); - test_nm_utils_dns_option_validate_do("opt1", FALSE, opt_descs, TRUE, "opt1", -1); - test_nm_utils_dns_option_validate_do("opt1", TRUE, opt_descs, TRUE, "opt1", -1); - test_nm_utils_dns_option_validate_do("opt1:3", FALSE, opt_descs, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt1", AF_INET, opt_descs, TRUE, "opt1", -1); + test_nm_utils_dns_option_validate_do("opt1", AF_INET6, opt_descs, TRUE, "opt1", -1); + test_nm_utils_dns_option_validate_do("opt1:3", AF_INET, opt_descs, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt2", FALSE, opt_descs, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt2:5", FALSE, opt_descs, TRUE, "opt2", 5); + test_nm_utils_dns_option_validate_do("opt2", AF_INET, opt_descs, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt2:5", AF_INET, opt_descs, TRUE, "opt2", 5); - test_nm_utils_dns_option_validate_do("opt3", FALSE, opt_descs, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt3", TRUE, opt_descs, TRUE, "opt3", -1); + test_nm_utils_dns_option_validate_do("opt3", AF_INET, opt_descs, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt3", AF_INET6, opt_descs, TRUE, "opt3", -1); - test_nm_utils_dns_option_validate_do("opt4", FALSE, opt_descs, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt4", TRUE, opt_descs, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt4:40", FALSE, opt_descs, FALSE, NULL, -1); - test_nm_utils_dns_option_validate_do("opt4:40", TRUE, opt_descs, TRUE, "opt4", 40); + test_nm_utils_dns_option_validate_do("opt4", AF_INET, opt_descs, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt4", AF_INET6, opt_descs, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt4:40", AF_INET, opt_descs, FALSE, NULL, -1); + test_nm_utils_dns_option_validate_do("opt4:40", AF_INET6, opt_descs, TRUE, "opt4", 40); } static void diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index 766c7dad1b..1679a724f2 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -411,7 +411,7 @@ extern const NMUtilsDNSOptionDesc _nm_utils_dns_option_descs[]; gboolean _nm_utils_dns_option_validate(const char *option, char **out_name, long *out_value, - gboolean ipv6, + int addr_family, const NMUtilsDNSOptionDesc *option_descs); gssize _nm_utils_dns_option_find_idx(GPtrArray *array, const char *option);