From 294131a2a41f8773aaf15abebb461fb3fbf7a7dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 20 Aug 2025 08:28:01 +0200 Subject: [PATCH 1/4] core: dbus: accept global DNS configuration without a default domain Since 1.44 we accept a global-dns section without any global-dns-domain section, so users can define searches and options without defining any global DNS servers. When set from the D-Bus API it was still rejected. Fix it. Fixes: 1f0d1d78d2a2 ('dns-manager: always apply options from [global-dns]') --- src/core/nm-config-data.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index 5a84a2c894..a23d622fa3 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -1606,17 +1606,6 @@ nm_global_dns_config_from_dbus(const GValue *value, GError **error) g_variant_unref(val); } - /* An empty value is valid and clears the internal configuration */ - if (!nm_global_dns_config_is_empty(dns_config) - && !nm_global_dns_config_lookup_domain(dns_config, "*")) { - g_set_error_literal(error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_FAILED, - "Global DNS configuration is missing the default domain"); - nm_global_dns_config_free(dns_config); - return NULL; - } - global_dns_config_seal_domains(dns_config); return dns_config; } From 1cba0a3cca80d2dc8436d8ffdf58506d9eb95846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Mon, 25 Aug 2025 08:58:21 +0200 Subject: [PATCH 2/4] dns: make [global-dns] to overwrite configs from connections According to the documentation, settings from [global-dns] (searches and options) are always merged with those from connections. However this was not happening if no [global-dns-domain-*] exists, in which case connections were ignored. This happened because in the past both global sections must de defined or undefined. When this was changed to allow defining only [global-dns], allowing it in the function that generates the resolv.conf file was forgotten. Fix that now. Anyway, merging these configs doesn't make much sense. The searches and options defined in connections probably make sense only for the nameservers defined in that same connection. Because of this, make the following change: if global nameservers are defined, use searches and options from [global-dns] only, because those defined in connections may not make sense for the global nameservers. If [global-dns] is missing, assume an empty [global-dns] section. Also, if no global nameservers are defined, but [global-dns] is, make that it overwrites the searches and options defined in connections. This is not ideal, but none of the alternatives is better and at least this is easy to remember. So, the resulting rules from above are: - If [global-dns] is defined, it always overwrite searches and options from connections. - If [global-dns-domain-*] is defined, it always overwrite nameservers from connections. It overwrites searches and options too. Fixes: 1f0d1d78d2a2 ('dns-manager: always apply options from [global-dns]') Fixes: f57a848da5aa ('man: update documentation about global DNS configuration') --- man/NetworkManager.conf.xml | 12 +++++- src/core/dns/nm-dns-manager.c | 78 +++++++++++++++++++++++------------ src/core/nm-config-data.c | 17 +++++++- src/core/nm-config-data.h | 1 + src/core/nm-l3-config-data.c | 23 ++++++----- src/core/nm-l3-config-data.h | 3 +- 6 files changed, 94 insertions(+), 40 deletions(-) diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml index 2b42755971..3f9aec790b 100644 --- a/man/NetworkManager.conf.xml +++ b/man/NetworkManager.conf.xml @@ -1540,8 +1540,12 @@ managed=1 <literal>global-dns</literal> section - This section specifies DNS settings that are applied - globally, in addition to connection-specific ones. + This section specifies DNS settings that are applied globally. They + override the equivalent options defined in individual connections, making + them to be ignored. If a [global-dns-domain-*] section is defined, but this + section isn't, an empty [global-dns] section is assumed, thus overwriting + connection specific configurations too. + @@ -1601,6 +1605,10 @@ managed=1 default domain "*". When the global DNS domains are valid, the name servers and domains defined globally override the ones from active connections. + + If any global DNS domain is defined but a [global-dns] section isn't, + an empty [global-dns] section is assumed, thus overwriting its + connection specific configurations too. diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index d47590dc24..57e732264c 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -586,7 +586,11 @@ add_dns_domains(GPtrArray *array, } static void -merge_one_l3cd(NMResolvConfData *rc, int addr_family, int ifindex, const NML3ConfigData *l3cd) +merge_one_l3cd(NMResolvConfData *rc, + int addr_family, + int ifindex, + const NML3ConfigData *l3cd, + gboolean ignore_searches_and_options) { char buf[NM_INET_ADDRSTRLEN + 50]; gboolean has_trust_ad; @@ -624,30 +628,32 @@ merge_one_l3cd(NMResolvConfData *rc, int addr_family, int ifindex, const NML3Con add_string_item(rc->nameservers, buf, TRUE); } - add_dns_domains(rc->searches, addr_family, l3cd, FALSE, TRUE); + if (!ignore_searches_and_options) { + add_dns_domains(rc->searches, addr_family, l3cd, FALSE, TRUE); - has_trust_ad = FALSE; - strarr = nm_l3_config_data_get_dns_options(l3cd, addr_family, &num); - for (i = 0; i < num; i++) { - const char *option = strarr[i]; + has_trust_ad = FALSE; + strarr = nm_l3_config_data_get_dns_options(l3cd, addr_family, &num); + for (i = 0; i < num; i++) { + const char *option = strarr[i]; - if (nm_streq(option, NM_SETTING_DNS_OPTION_TRUST_AD)) { - has_trust_ad = TRUE; - continue; + if (nm_streq(option, NM_SETTING_DNS_OPTION_TRUST_AD)) { + has_trust_ad = TRUE; + continue; + } + add_dns_option_item(rc->options, option); } - add_dns_option_item(rc->options, option); - } - if (num_nameservers == 0) { - /* If the @l3cd contributes no DNS servers, ignore whether trust-ad is set or unset - * for this @l3cd. */ - } else if (has_trust_ad) { - /* We only set has_trust_ad to TRUE, if all IP configs agree (or don't contribute). - * Once set to FALSE, it doesn't get reset. */ - if (rc->has_trust_ad == NM_TERNARY_DEFAULT) - rc->has_trust_ad = NM_TERNARY_TRUE; - } else - rc->has_trust_ad = NM_TERNARY_FALSE; + if (num_nameservers == 0) { + /* If the @l3cd contributes no DNS servers, ignore whether trust-ad is set or unset + * for this @l3cd. */ + } else if (has_trust_ad) { + /* We only set has_trust_ad to TRUE, if all IP configs agree (or don't contribute). + * Once set to FALSE, it doesn't get reset. */ + if (rc->has_trust_ad == NM_TERNARY_DEFAULT) + rc->has_trust_ad = NM_TERNARY_TRUE; + } else + rc->has_trust_ad = NM_TERNARY_FALSE; + } if (addr_family == AF_INET) { const in_addr_t *nis_servers; @@ -1231,12 +1237,15 @@ compute_hash(NMDnsManager *self, const NMGlobalDnsConfig *global, guint8 buffer[ { nm_auto_free_checksum GChecksum *sum = NULL; NMDnsConfigIPData *ip_data; + gboolean has_global_dns_section = FALSE; sum = g_checksum_new(G_CHECKSUM_SHA1); nm_assert(HASH_LEN == g_checksum_type_get_length(G_CHECKSUM_SHA1)); - if (global) + if (global) { nm_global_dns_config_update_checksum(global, sum); + has_global_dns_section = nm_global_dns_has_global_dns_section(global); + } if (!global || !nm_global_dns_config_lookup_domain(global, "*")) { const CList *head; @@ -1248,7 +1257,8 @@ compute_hash(NMDnsManager *self, const NMGlobalDnsConfig *global, guint8 buffer[ nm_l3_config_data_hash_dns(ip_data->l3cd, sum, ip_data->addr_family, - ip_data->ip_config_type); + ip_data->ip_config_type, + has_global_dns_section); } } @@ -1264,6 +1274,9 @@ merge_global_dns_config(NMResolvConfData *rc, NMGlobalDnsConfig *global_conf) const char *const *servers; guint i; + /* Global config must be processed before connections' config */ + nm_assert(rc->nameservers->len == 0); + if (!global_conf) return FALSE; @@ -1351,12 +1364,17 @@ _collect_resolv_conf_data(NMDnsManager *self, .nis_servers = g_ptr_array_new(), .has_trust_ad = NM_TERNARY_DEFAULT, }; + gboolean has_global_dns_section = FALSE; priv = NM_DNS_MANAGER_GET_PRIVATE(self); - if (global_config) + if (global_config) { merge_global_dns_config(&rc, global_config); + has_global_dns_section = nm_global_dns_has_global_dns_section(global_config); + } + /* If global nameservers are defined, no DNS configs are used from connections at all, + * including searches and options. */ if (!global_config || !nm_global_dns_config_lookup_domain(global_config, "*")) { nm_auto_str_buf NMStrBuf tmp_strbuf = NM_STR_BUF_INIT(0, FALSE); int first_prio = 0; @@ -1390,8 +1408,16 @@ _collect_resolv_conf_data(NMDnsManager *self, skip ? "" : "", get_nameserver_list(ip_data->addr_family, ip_data->l3cd, &tmp_strbuf)); - if (!skip) - merge_one_l3cd(&rc, ip_data->addr_family, ip_data->data->ifindex, ip_data->l3cd); + if (!skip) { + /* Merge the configs from connections. However, if there was a [global-dns] + * it overwrites searches and options from the connections, thus we only + * merge the nameservers. */ + merge_one_l3cd(&rc, + ip_data->addr_family, + ip_data->data->ifindex, + ip_data->l3cd, + has_global_dns_section); + } } } diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index a23d622fa3..24a2d8c922 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -50,9 +50,10 @@ struct _NMGlobalDnsConfig { char **options; GHashTable *domains; const char **domain_list; - gboolean internal; char *cert_authority; NMDnsResolveMode resolve_mode; + gboolean internal; + gboolean has_global_dns; }; /*****************************************************************************/ @@ -941,6 +942,14 @@ next: /*****************************************************************************/ +gboolean +nm_global_dns_has_global_dns_section(const NMGlobalDnsConfig *dns_config) +{ + g_return_val_if_fail(dns_config, FALSE); + + return dns_config->has_global_dns; +} + const char *const * nm_global_dns_config_get_searches(const NMGlobalDnsConfig *dns_config) { @@ -1386,6 +1395,12 @@ load_global_dns(GKeyFile *keyfile, gboolean internal) return NULL; } + /* Defining [global-dns-domain-*] implies defining [global-dns] too (maybe empty) */ + if (default_found) + dns_config->has_global_dns = TRUE; + else + dns_config->has_global_dns = g_key_file_has_group(keyfile, group); + dns_config->internal = internal; global_dns_config_seal_domains(dns_config); return dns_config; diff --git a/src/core/nm-config-data.h b/src/core/nm-config-data.h index d764a7c45f..6666ccf6e8 100644 --- a/src/core/nm-config-data.h +++ b/src/core/nm-config-data.h @@ -274,6 +274,7 @@ gboolean nm_config_data_is_intern_atomic_group(const NMConfigData *self, const c GKeyFile *nm_config_data_clone_keyfile_intern(const NMConfigData *self); +gboolean nm_global_dns_has_global_dns_section(const NMGlobalDnsConfig *dns_config); const char *const *nm_global_dns_config_get_searches(const NMGlobalDnsConfig *dns_config); const char *const *nm_global_dns_config_get_options(const NMGlobalDnsConfig *dns_config); const char *nm_global_dns_config_get_certification_authority(const NMGlobalDnsConfig *dns_config); diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index 2062314581..666aa8a38c 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -3139,7 +3139,8 @@ void nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family, - NMDnsIPConfigType dns_ip_config_type) + NMDnsIPConfigType dns_ip_config_type, + gboolean ignore_searches_and_options) { guint i; int val; @@ -3178,16 +3179,18 @@ nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, empty = FALSE; } - searches = nm_l3_config_data_get_searches(l3cd, addr_family, &num_searches); - for (i = 0; i < num_searches; i++) { - g_checksum_update(sum, (const guint8 *) searches[i], strlen(searches[i])); - empty = FALSE; - } + if (!ignore_searches_and_options) { + searches = nm_l3_config_data_get_searches(l3cd, addr_family, &num_searches); + for (i = 0; i < num_searches; i++) { + g_checksum_update(sum, (const guint8 *) searches[i], strlen(searches[i])); + empty = FALSE; + } - options = nm_l3_config_data_get_dns_options(l3cd, addr_family, &num_options); - for (i = 0; i < num_options; i++) { - g_checksum_update(sum, (const guint8 *) options[i], strlen(options[i])); - empty = FALSE; + options = nm_l3_config_data_get_dns_options(l3cd, addr_family, &num_options); + for (i = 0; i < num_options; i++) { + g_checksum_update(sum, (const guint8 *) options[i], strlen(options[i])); + empty = FALSE; + } } val = nm_l3_config_data_get_mdns(l3cd); diff --git a/src/core/nm-l3-config-data.h b/src/core/nm-l3-config-data.h index bcb6af6777..265e126d89 100644 --- a/src/core/nm-l3-config-data.h +++ b/src/core/nm-l3-config-data.h @@ -615,6 +615,7 @@ nmtst_l3_config_data_get_best_gateway(const NML3ConfigData *self, int addr_famil void nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family, - NMDnsIPConfigType dns_ip_config_type); + NMDnsIPConfigType dns_ip_config_type, + gboolean ignore_searches_and_options); #endif /* __NM_L3_CONFIG_DATA_H__ */ From 7fb4724efa73f87ef5676504631fd8f58e388c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Thu, 4 Sep 2025 16:48:02 +0200 Subject: [PATCH 3/4] core: dns: show in D-Bus if [global-dns] is defined but empty Clients like nmstate needs to know if the [global-dns] section is defined or not, so they know if DNS configs from connections are relevant or not. Expose it in D-Bus by always exposing "searches" and "options" if it's defined, maybe as empty lists. --- src/core/nm-config-data.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index 24a2d8c922..461fd8eda2 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -53,7 +53,6 @@ struct _NMGlobalDnsConfig { char *cert_authority; NMDnsResolveMode resolve_mode; gboolean internal; - gboolean has_global_dns; }; /*****************************************************************************/ @@ -947,7 +946,7 @@ nm_global_dns_has_global_dns_section(const NMGlobalDnsConfig *dns_config) { g_return_val_if_fail(dns_config, FALSE); - return dns_config->has_global_dns; + return dns_config->searches != NULL || dns_config->options != NULL; } const char *const * @@ -1245,6 +1244,7 @@ load_global_dns(GKeyFile *keyfile, gboolean internal) gs_free char *cert_authority = NULL; gs_free char *resolve_mode = NULL; NMDnsResolveMode parsed_resolve_mode; + gboolean has_global_dns_section; if (internal) { group = NM_CONFIG_KEYFILE_GROUP_INTERN_GLOBAL_DNS; @@ -1397,9 +1397,19 @@ load_global_dns(GKeyFile *keyfile, gboolean internal) /* Defining [global-dns-domain-*] implies defining [global-dns] too (maybe empty) */ if (default_found) - dns_config->has_global_dns = TRUE; + has_global_dns_section = TRUE; else - dns_config->has_global_dns = g_key_file_has_group(keyfile, group); + has_global_dns_section = g_key_file_has_group(keyfile, group); + + /* If there exist a [global-dns] section, always initialize "searches" and "options" so + * they appear in D-Bus. Clients can use this to know if it's defined, so they can know + * if DNS configs from connections are relevant or not. */ + if (has_global_dns_section) { + if (!dns_config->searches) + dns_config->searches = nm_strv_empty_new(); + if (!dns_config->options) + dns_config->options = nm_strv_empty_new(); + } dns_config->internal = internal; global_dns_config_seal_domains(dns_config); From 4a46f454da5fbe6668c2eaecdeaeec35068a6ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Thu, 4 Sep 2025 16:54:21 +0200 Subject: [PATCH 4/4] core: keep empty groups from keyfile configs When reading NetworkManager.conf and NetworkManager-intern.conf we might need to know if a group is defined or not, even if it's empty. This is the case, for example, for [global-dns]. If [global-dns] is defined in NM.conf overwrites the config from NM-intern, and if it's defined in any of them they overwrite the configs from connections. Before this patch, defining it as an empty group was ignored: ``` [global-dns] ``` Instead, it was necessary to add at least one key-value to the group. Otherwise the group was silently ignored. ``` [global-dns] searches= ``` Keep empty groups so we can take better decissions about overwritting configs from other sources. --- src/core/nm-config.c | 14 ++++++++++++ src/core/tests/config/global-dns-empty.conf | 3 +++ src/core/tests/config/global-dns-not-set.conf | 5 +++++ src/core/tests/config/test-config.c | 22 ++++++++++++++++++- 4 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 src/core/tests/config/global-dns-empty.conf create mode 100644 src/core/tests/config/global-dns-not-set.conf diff --git a/src/core/nm-config.c b/src/core/nm-config.c index a55d2d1396..d1f2bbed35 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -18,6 +18,7 @@ #include "libnm-core-intern/nm-core-internal.h" #include "libnm-core-intern/nm-keyfile-internal.h" #include "libnm-core-intern/nm-keyfile-utils.h" +#include "libnm-glib-aux/nm-keyfile-aux.h" #define DEFAULT_CONFIG_MAIN_FILE NMCONFDIR "/NetworkManager.conf" #define DEFAULT_CONFIG_DIR NMCONFDIR "/conf.d" @@ -1046,6 +1047,10 @@ read_config(GKeyFile *keyfile, /* internal groups cannot be set by user configuration. */ continue; } + + if (!g_key_file_has_group(keyfile, group)) + nm_key_file_add_group(keyfile, group); + keys = g_key_file_get_keys(kf, group, &nkeys, NULL); if (!keys) continue; @@ -1639,6 +1644,12 @@ intern_config_read(const char *filename, ""); } + if (!g_key_file_has_group(keyfile_intern, group)) { + nm_key_file_add_group(keyfile_intern, group); + if (is_intern) + has_intern = TRUE; + } + for (k = 0; keys[k]; k++) { gs_free char *value_set = NULL; const char *key = keys[k]; @@ -1823,6 +1834,9 @@ intern_config_write(const char *filename, } } + if (!g_key_file_has_group(keyfile, group)) + nm_key_file_add_group(keyfile, group); + for (k = 0; keys[k]; k++) { const char *key = keys[k]; gs_free char *value_set = NULL; diff --git a/src/core/tests/config/global-dns-empty.conf b/src/core/tests/config/global-dns-empty.conf new file mode 100644 index 0000000000..fba823bfe4 --- /dev/null +++ b/src/core/tests/config/global-dns-empty.conf @@ -0,0 +1,3 @@ +# Good configuration, an empty global-dns section must be valid + +[global-dns] diff --git a/src/core/tests/config/global-dns-not-set.conf b/src/core/tests/config/global-dns-not-set.conf new file mode 100644 index 0000000000..133bd4ed43 --- /dev/null +++ b/src/core/tests/config/global-dns-not-set.conf @@ -0,0 +1,5 @@ +# Good configuration, an empty [global-dns] must be implicitly assumed because a domain is defined + +[global-dns-domain-*] +servers=4.5.6.7 +options=myoption1 diff --git a/src/core/tests/config/test-config.c b/src/core/tests/config/test-config.c index 2980cda7fb..78fd10571d 100644 --- a/src/core/tests/config/test-config.c +++ b/src/core/tests/config/test-config.c @@ -387,7 +387,27 @@ test_config_global_dns(void) g_assert(dns); g_object_unref(config); - /* Check that a file with a domain domain, but without a default one gives a NULL configuration */ + /* Check that a file with an empty global-dns section gives a good configuration. + * Check also that searches and options are not NULL, as this is how we expose to + * D-Bus that global-dns is defined. */ + config = + setup_config(NULL, TEST_DIR "/global-dns-empty.conf", "", NULL, "/no/such/dir", "", NULL); + dns = nm_config_data_get_global_dns_config(nm_config_get_data_orig(config)); + g_assert(dns); + g_assert(nm_global_dns_config_get_searches(dns)); + g_assert(nm_global_dns_config_get_options(dns)); + g_object_unref(config); + + /* Check that a file with a domain, but no global-dns, assumes an implicit empty global-dns */ + config = + setup_config(NULL, TEST_DIR "/global-dns-not-set.conf", "", NULL, "/no/such/dir", "", NULL); + dns = nm_config_data_get_global_dns_config(nm_config_get_data_orig(config)); + g_assert(dns); + g_assert(nm_global_dns_config_get_searches(dns)); + g_assert(nm_global_dns_config_get_options(dns)); + g_object_unref(config); + + /* Check that a file with a domain, but without a default one, gives a NULL configuration */ config = setup_config(NULL, TEST_DIR "/global-dns-invalid.conf", "", NULL, "/no/such/dir", "", NULL); dns = nm_config_data_get_global_dns_config(nm_config_get_data_orig(config));