merge: branch 'ih/domain-search-sep'

dns: ensure that no wrong separators are used for DNS search domains

Closes #1740

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2196
This commit is contained in:
Íñigo Huguet 2025-08-26 09:00:18 +00:00
commit 96be0cf049
3 changed files with 103 additions and 0 deletions

View file

@ -1186,6 +1186,46 @@ ip_dns_parser(KeyfileReaderInfo *info, NMSetting *setting, const char *key)
g_object_set(setting, key, list, NULL);
}
static void
ip_dns_search_parser(KeyfileReaderInfo *info, NMSetting *setting, const char *key)
{
gs_strfreev char **list = NULL;
gsize length;
nm_assert(NM_IS_SETTING_IP4_CONFIG(setting) || NM_IS_SETTING_IP6_CONFIG(setting));
list = nm_keyfile_plugin_kf_get_string_list(info->keyfile,
nm_setting_get_name(setting),
key,
&length,
NULL);
nm_assert(length == NM_PTRARRAY_LEN(list));
if (length == 0)
return;
if (length == 1 && strpbrk(list[0], ", ")) {
/* By mistake, we accepted invalid characters like ',' in DNS search domains.
* Now we do some validation that would cause the connection to be rejected by
* the daemon. Let's continue accepting ',' and ' ' as separators but emit a
* warning */
char **list2;
read_handle_warn(info,
key,
key,
NM_KEYFILE_WARN_SEVERITY_WARN,
_("normalizing invalid separator ',' or ' ' in DNS search value '%s', "
"only ';' will be valid separators in keyfiles in the future"),
list[0]);
list2 = g_strsplit_set(list[0], ", ", -1);
g_strfreev(list);
list = list2;
}
g_object_set(setting, key, list, NULL);
}
static void
ip6_addr_gen_mode_parser(KeyfileReaderInfo *info, NMSetting *setting, const char *key)
{
@ -3084,6 +3124,7 @@ static const ParseInfoSetting *const parse_infos[_NM_META_SETTING_TYPE_NUM] = {
.parser = ip_dns_parser,
.writer = dns_writer, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_DNS_OPTIONS, .always_write = TRUE, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_DNS_SEARCH, .parser = ip_dns_search_parser, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_GATEWAY, .parser = gateway_parser, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_ROUTES,
.parser_no_check_key = TRUE,
@ -3112,6 +3153,7 @@ static const ParseInfoSetting *const parse_infos[_NM_META_SETTING_TYPE_NUM] = {
.parser = ip_dns_parser,
.writer = dns_writer, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_DNS_OPTIONS, .always_write = TRUE, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_DNS_SEARCH, .parser = ip_dns_search_parser, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_GATEWAY, .parser = gateway_parser, ),
PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_ROUTES,
.parser_no_check_key = TRUE,

View file

@ -5671,6 +5671,28 @@ verify(NMSetting *setting, NMConnection *connection, GError **error)
}
}
/* Validate DNS search domains */
if (nm_strvarray_get_strv_notempty(priv->dns_search.arr, NULL)) {
for (i = 0; i < priv->dns_search.arr->len; i++) {
const char *dns_search = nm_strvarray_get_idx(priv->dns_search.arr, i);
/* TODO: currently we only check that no wrong list separators have
* been used by mistake. Proper domain name validation would be better. */
if (strpbrk(dns_search, ",; ")) {
g_set_error(error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("DNS search domain '%s' is invalid"),
dns_search);
g_prefix_error(error,
"%s.%s: ",
nm_setting_get_name(setting),
NM_SETTING_IP_CONFIG_DNS_SEARCH);
return FALSE;
}
}
}
/* Validate addresses */
for (i = 0; i < priv->addresses->len; i++) {
NMIPAddress *addr = (NMIPAddress *) priv->addresses->pdata[i];

View file

@ -5460,6 +5460,44 @@ test_settings_dns(void)
}
}
static void
_assert_dns_searches(gboolean valid, ...)
{
NMConnection *con;
NMSettingIPConfig *ip4, *ip6;
const char *dns_search;
va_list args;
con = nmtst_create_minimal_connection("test-dns-search",
NULL,
NM_SETTING_WIRED_SETTING_NAME,
NULL);
nmtst_connection_normalize(con);
ip4 = nm_connection_get_setting_ip4_config(con);
ip6 = nm_connection_get_setting_ip6_config(con);
va_start(args, valid);
while ((dns_search = va_arg(args, const char *))) {
nm_setting_ip_config_add_dns_search(ip4, dns_search);
nm_setting_ip_config_add_dns_search(ip6, dns_search);
}
va_end(args);
g_assert(valid == nm_setting_verify((NMSetting *) ip4, con, NULL));
g_assert(valid == nm_setting_verify((NMSetting *) ip6, con, NULL));
}
static void
test_settings_dns_search_domains(void)
{
_assert_dns_searches(TRUE, "example.com", NULL);
_assert_dns_searches(TRUE, "sub.example.com", NULL);
_assert_dns_searches(TRUE, "example.com", "sub.example.com", NULL);
_assert_dns_searches(FALSE, "example.com,sub.example.com", NULL);
_assert_dns_searches(FALSE, "example.com;sub.example.com", NULL);
_assert_dns_searches(FALSE, "example.com sub.example.com", NULL);
}
/*****************************************************************************/
static void
@ -5570,6 +5608,7 @@ main(int argc, char **argv)
g_test_add_func("/libnm/settings/6lowpan/1", test_6lowpan_1);
g_test_add_func("/libnm/settings/dns", test_settings_dns);
g_test_add_func("/libnm/settings/dns_search_domain", test_settings_dns_search_domains);
g_test_add_func("/libnm/settings/sriov/vf", test_sriov_vf);
g_test_add_func("/libnm/settings/sriov/vf-dup", test_sriov_vf_dup);