From a7412e2c65e894f578e9c8ed137c81591f71c5d6 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 30 Jan 2023 17:17:19 +0100 Subject: [PATCH 1/3] core: rename and move nm_ip_config_dns_hash() The function operates on a NML3ConfigData, rename it and move it to the right place. (cherry picked from commit ec0a83b224b313839357fb325ccede0f5a0c47c4) --- src/core/dns/nm-dns-manager.c | 2 +- src/core/nm-ip-config.c | 68 ---------------------------------- src/core/nm-ip-config.h | 2 - src/core/nm-l3-config-data.c | 70 +++++++++++++++++++++++++++++++++++ src/core/nm-l3-config-data.h | 2 + 5 files changed, 73 insertions(+), 71 deletions(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index 42519e64f..a216ac13e 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -1209,7 +1209,7 @@ compute_hash(NMDnsManager *self, const NMGlobalDnsConfig *global, guint8 buffer[ * configuration without DNS parameters gives a zero checksum. */ head = _mgr_get_ip_data_lst_head(self); c_list_for_each_entry (ip_data, head, ip_data_lst) - nm_ip_config_dns_hash(ip_data->l3cd, sum, ip_data->addr_family); + nm_l3_config_data_hash_dns(ip_data->l3cd, sum, ip_data->addr_family); } nm_utils_checksum_get_digest_len(sum, buffer, HASH_LEN); diff --git a/src/core/nm-ip-config.c b/src/core/nm-ip-config.c index a87e4c3f1..f87840cc3 100644 --- a/src/core/nm-ip-config.c +++ b/src/core/nm-ip-config.c @@ -509,74 +509,6 @@ nm_ip4_config_class_init(NMIP4ConfigClass *klass) g_object_class_install_properties(object_class, _PROPERTY_ENUMS_LAST_ip4, obj_properties_ip4); } -void -nm_ip_config_dns_hash(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family) -{ - guint i; - int val; - const char *const *strarr; - const in_addr_t *wins; - const char *const *domains; - const char *const *searches; - const char *const *options; - guint num_nameservers; - guint num_wins; - guint num_domains; - guint num_searches; - guint num_options; - - g_return_if_fail(l3cd); - g_return_if_fail(sum); - - strarr = nm_l3_config_data_get_nameservers(l3cd, addr_family, &num_nameservers); - for (i = 0; i < num_nameservers; i++) - g_checksum_update(sum, (gpointer) strarr[i], strlen(strarr[i])); - - if (addr_family == AF_INET) { - wins = nm_l3_config_data_get_wins(l3cd, &num_wins); - for (i = 0; i < num_wins; i++) - g_checksum_update(sum, (guint8 *) &wins[i], 4); - } - - domains = nm_l3_config_data_get_domains(l3cd, addr_family, &num_domains); - for (i = 0; i < num_domains; i++) { - g_checksum_update(sum, (const guint8 *) domains[i], strlen(domains[i])); - } - - 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])); - } - - 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])); - } - - val = nm_l3_config_data_get_mdns(l3cd); - if (val != NM_SETTING_CONNECTION_MDNS_DEFAULT) - g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); - - val = nm_l3_config_data_get_llmnr(l3cd); - if (val != NM_SETTING_CONNECTION_LLMNR_DEFAULT) - g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); - - val = nm_l3_config_data_get_dns_over_tls(l3cd); - if (val != NM_SETTING_CONNECTION_DNS_OVER_TLS_DEFAULT) - g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); - - /* FIXME(ip-config-checksum): the DNS priority should be considered relevant - * and added into the checksum as well, but this can't be done right now - * because in the DNS manager we rely on the fact that an empty - * configuration (i.e. just created) has a zero checksum. This is needed to - * avoid rewriting resolv.conf when there is no change. - * - * The DNS priority initial value depends on the connection type (VPN or - * not), so it's a bit difficult to add it to checksum maintaining the - * assumption of checksum(empty)=0 - */ -} - /*****************************************************************************/ /* public */ diff --git a/src/core/nm-ip-config.h b/src/core/nm-ip-config.h index 3a1d375e9..0dcea83b5 100644 --- a/src/core/nm-ip-config.h +++ b/src/core/nm-ip-config.h @@ -51,8 +51,6 @@ NMIPConfig *nm_ip_config_new(int addr_family, NML3Cfg *l3cfg); void nm_ip_config_take_and_unexport_on_idle(NMIPConfig *self_take); -void nm_ip_config_dns_hash(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family); - /*****************************************************************************/ static inline NML3Cfg * diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index 1c8200c24..2b9e2207c 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -3018,6 +3018,76 @@ nm_l3_config_data_new_from_platform(NMDedupMultiIndex *multi_idx, /*****************************************************************************/ +void +nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family) +{ + guint i; + int val; + const char *const *strarr; + const in_addr_t *wins; + const char *const *domains; + const char *const *searches; + const char *const *options; + guint num_nameservers; + guint num_wins; + guint num_domains; + guint num_searches; + guint num_options; + + g_return_if_fail(l3cd); + g_return_if_fail(sum); + + strarr = nm_l3_config_data_get_nameservers(l3cd, addr_family, &num_nameservers); + for (i = 0; i < num_nameservers; i++) + g_checksum_update(sum, (gpointer) strarr[i], strlen(strarr[i])); + + if (addr_family == AF_INET) { + wins = nm_l3_config_data_get_wins(l3cd, &num_wins); + for (i = 0; i < num_wins; i++) + g_checksum_update(sum, (guint8 *) &wins[i], 4); + } + + domains = nm_l3_config_data_get_domains(l3cd, addr_family, &num_domains); + for (i = 0; i < num_domains; i++) { + g_checksum_update(sum, (const guint8 *) domains[i], strlen(domains[i])); + } + + 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])); + } + + 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])); + } + + val = nm_l3_config_data_get_mdns(l3cd); + if (val != NM_SETTING_CONNECTION_MDNS_DEFAULT) + g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); + + val = nm_l3_config_data_get_llmnr(l3cd); + if (val != NM_SETTING_CONNECTION_LLMNR_DEFAULT) + g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); + + val = nm_l3_config_data_get_dns_over_tls(l3cd); + if (val != NM_SETTING_CONNECTION_DNS_OVER_TLS_DEFAULT) + g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); + + /* FIXME(ip-config-checksum): the DNS priority should be considered relevant + * and added into the checksum as well, but this can't be done right now + * because in the DNS manager we rely on the fact that an empty + * configuration (i.e. just created) has a zero checksum. This is needed to + * avoid rewriting resolv.conf when there is no change. + * + * The DNS priority initial value depends on the connection type (VPN or + * not), so it's a bit difficult to add it to checksum maintaining the + * assumption of checksum(empty)=0 + */ +} + +/*****************************************************************************/ + void nm_l3_config_data_merge(NML3ConfigData *self, const NML3ConfigData *src, diff --git a/src/core/nm-l3-config-data.h b/src/core/nm-l3-config-data.h index 1a181e5cc..14d0766aa 100644 --- a/src/core/nm-l3-config-data.h +++ b/src/core/nm-l3-config-data.h @@ -604,4 +604,6 @@ nmtst_l3_config_data_get_best_gateway(const NML3ConfigData *self, int addr_famil return nm_platform_ip_route_get_gateway(addr_family, NMP_OBJECT_CAST_IP_ROUTE(rt)); } +void nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family); + #endif /* __NM_L3_CONFIG_DATA_H__ */ From b14268290ac4db8fa4839d6fb362ddef151039a4 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 30 Jan 2023 17:20:44 +0100 Subject: [PATCH 2/3] core,libnm: move enum NMDnsIPConfigType The enum will be used outside of core/dns. (cherry picked from commit 8a4632b56a1050ae155fef473582cae713321bce) --- src/core/dns/nm-dns-manager.h | 10 ---------- src/libnm-base/nm-base.h | 8 ++++++++ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/core/dns/nm-dns-manager.h b/src/core/dns/nm-dns-manager.h index 81121d7e5..42f9dec58 100644 --- a/src/core/dns/nm-dns-manager.h +++ b/src/core/dns/nm-dns-manager.h @@ -12,16 +12,6 @@ #include "nm-setting-connection.h" #include "nm-dns-plugin.h" -typedef enum { - NM_DNS_IP_CONFIG_TYPE_REMOVED = -1, - - NM_DNS_IP_CONFIG_TYPE_DEFAULT = 0, - NM_DNS_IP_CONFIG_TYPE_BEST_DEVICE, - NM_DNS_IP_CONFIG_TYPE_VPN, -} NMDnsIPConfigType; - -/*****************************************************************************/ - struct _NMDnsConfigData; struct _NMDnsManager; diff --git a/src/libnm-base/nm-base.h b/src/libnm-base/nm-base.h index a8d6b2c89..74e8142f2 100644 --- a/src/libnm-base/nm-base.h +++ b/src/libnm-base/nm-base.h @@ -415,4 +415,12 @@ typedef struct { #define _NM_CRYPTO_ERROR _nm_crypto_error_quark() GQuark _nm_crypto_error_quark(void); +typedef enum { + NM_DNS_IP_CONFIG_TYPE_REMOVED = -1, + + NM_DNS_IP_CONFIG_TYPE_DEFAULT = 0, + NM_DNS_IP_CONFIG_TYPE_BEST_DEVICE, + NM_DNS_IP_CONFIG_TYPE_VPN, +} NMDnsIPConfigType; + #endif /* __NM_LIBNM_BASE_H__ */ From 2a0f41af0356abdc6e91be24ea7b5ad51d1bc314 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 30 Jan 2023 17:29:52 +0100 Subject: [PATCH 3/3] dns: consider the dns-type and the priority when hashing DNS configs The dns-type must be included in the hash because it contributes to the generated composite configuration. Without this, when the type of a configuration changes (e.g. from DEFAULT to BEST), the DNS manager would determine that there was no change and it wouldn't call update_dns(). https://bugzilla.redhat.com/show_bug.cgi?id=2161957 Fixes: 8995d44a0bae ('core: compare the DNS configurations before updating DNS') (cherry picked from commit 46ccc82a81146132e4bc9c6cc96d7b8b2305f85f) --- src/core/dns/nm-dns-manager.c | 8 +++++-- src/core/nm-l3-config-data.c | 43 ++++++++++++++++++++++++++++++----- src/core/nm-l3-config-data.h | 5 +++- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index a216ac13e..6ee2e816a 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -1208,8 +1208,12 @@ compute_hash(NMDnsManager *self, const NMGlobalDnsConfig *global, guint8 buffer[ /* FIXME(ip-config-checksum): this relies on the fact that an IP * configuration without DNS parameters gives a zero checksum. */ head = _mgr_get_ip_data_lst_head(self); - c_list_for_each_entry (ip_data, head, ip_data_lst) - nm_l3_config_data_hash_dns(ip_data->l3cd, sum, ip_data->addr_family); + c_list_for_each_entry (ip_data, head, ip_data_lst) { + nm_l3_config_data_hash_dns(ip_data->l3cd, + sum, + ip_data->addr_family, + ip_data->ip_config_type); + } } nm_utils_checksum_get_digest_len(sum, buffer, HASH_LEN); diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index 2b9e2207c..2865d9e1c 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -3019,7 +3019,10 @@ nm_l3_config_data_new_from_platform(NMDedupMultiIndex *multi_idx, /*****************************************************************************/ void -nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family) +nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, + GChecksum *sum, + int addr_family, + NMDnsIPConfigType dns_ip_config_type) { guint i; int val; @@ -3033,46 +3036,74 @@ nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, GChecksum *sum, int addr_ guint num_domains; guint num_searches; guint num_options; + gboolean empty = TRUE; g_return_if_fail(l3cd); g_return_if_fail(sum); strarr = nm_l3_config_data_get_nameservers(l3cd, addr_family, &num_nameservers); - for (i = 0; i < num_nameservers; i++) + for (i = 0; i < num_nameservers; i++) { g_checksum_update(sum, (gpointer) strarr[i], strlen(strarr[i])); + empty = FALSE; + } if (addr_family == AF_INET) { wins = nm_l3_config_data_get_wins(l3cd, &num_wins); - for (i = 0; i < num_wins; i++) + for (i = 0; i < num_wins; i++) { g_checksum_update(sum, (guint8 *) &wins[i], 4); + empty = FALSE; + } } domains = nm_l3_config_data_get_domains(l3cd, addr_family, &num_domains); for (i = 0; i < num_domains; i++) { g_checksum_update(sum, (const guint8 *) domains[i], strlen(domains[i])); + 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; } 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); - if (val != NM_SETTING_CONNECTION_MDNS_DEFAULT) + if (val != NM_SETTING_CONNECTION_MDNS_DEFAULT) { g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); + empty = FALSE; + } val = nm_l3_config_data_get_llmnr(l3cd); - if (val != NM_SETTING_CONNECTION_LLMNR_DEFAULT) + if (val != NM_SETTING_CONNECTION_LLMNR_DEFAULT) { g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); + empty = FALSE; + } val = nm_l3_config_data_get_dns_over_tls(l3cd); - if (val != NM_SETTING_CONNECTION_DNS_OVER_TLS_DEFAULT) + if (val != NM_SETTING_CONNECTION_DNS_OVER_TLS_DEFAULT) { g_checksum_update(sum, (const guint8 *) &val, sizeof(val)); + empty = FALSE; + } + + if (!empty) { + int prio = 0; + + /* In the DNS manager we rely on the fact that an empty (i.e. without + * any name server, domain, option, etc. parameters) configuration + * has a zero checksum. This is needed to avoid rewriting resolv.conf + * when not needed. Since the dns-type and the priority are always + * present, hash them only when the rest of configuration is not empty. + */ + g_checksum_update(sum, (const guint8 *) &dns_ip_config_type, sizeof(dns_ip_config_type)); + nm_l3_config_data_get_dns_priority(l3cd, addr_family, &prio); + g_checksum_update(sum, (const guint8 *) &prio, sizeof(prio)); + } /* FIXME(ip-config-checksum): the DNS priority should be considered relevant * and added into the checksum as well, but this can't be done right now diff --git a/src/core/nm-l3-config-data.h b/src/core/nm-l3-config-data.h index 14d0766aa..bfab04d9c 100644 --- a/src/core/nm-l3-config-data.h +++ b/src/core/nm-l3-config-data.h @@ -604,6 +604,9 @@ nmtst_l3_config_data_get_best_gateway(const NML3ConfigData *self, int addr_famil return nm_platform_ip_route_get_gateway(addr_family, NMP_OBJECT_CAST_IP_ROUTE(rt)); } -void nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, GChecksum *sum, int addr_family); +void nm_l3_config_data_hash_dns(const NML3ConfigData *l3cd, + GChecksum *sum, + int addr_family, + NMDnsIPConfigType dns_ip_config_type); #endif /* __NM_L3_CONFIG_DATA_H__ */