From 0250f85b71ad6f956dee544287f5eb255866b5ea Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Nov 2020 15:13:53 +0100 Subject: [PATCH 1/9] dns: cleanup allocation of NMDnsConfigData and NMDnsIPConfigData (cherry picked from commit 190eeb5e9f66005b20812995fba752e6877d0b04) (cherry picked from commit 795eca6e1b6e341bf7b165a51f22f02f1561519f) --- src/dns/nm-dns-manager.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 144ab8029f..a88e64428c 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -231,10 +231,12 @@ _ip_config_data_new (NMDnsConfigData *data, nm_assert (NM_IS_IP_CONFIG (ip_config, AF_UNSPEC)); nm_assert (ip_config_type != NM_DNS_IP_CONFIG_TYPE_REMOVED); - ip_data = g_slice_new0 (NMDnsIPConfigData); - ip_data->data = data; - ip_data->ip_config = g_object_ref (ip_config); - ip_data->ip_config_type = ip_config_type; + ip_data = g_slice_new (NMDnsIPConfigData); + *ip_data = (NMDnsIPConfigData) { + .data = data, + .ip_config = g_object_ref (ip_config), + .ip_config_type = ip_config_type, + }; c_list_link_tail (&data->data_lst_head, &ip_data->data_lst); c_list_link_tail (&NM_DNS_MANAGER_GET_PRIVATE (data->self)->ip_config_lst_head, &ip_data->ip_config_lst); @@ -264,7 +266,7 @@ _ip_config_data_free (NMDnsIPConfigData *ip_data) ip_data); g_object_unref (ip_data->ip_config); - g_slice_free (NMDnsIPConfigData, ip_data); + nm_g_slice_free (ip_data); } static NMDnsIPConfigData * @@ -290,7 +292,7 @@ _config_data_free (NMDnsConfigData *data) _ASSERT_config_data (data); nm_assert (c_list_is_empty (&data->data_lst_head)); - g_slice_free (NMDnsConfigData, data); + nm_g_slice_free (data); } static int @@ -1696,10 +1698,12 @@ nm_dns_manager_set_ip_config (NMDnsManager *self, } if (!data) { - data = g_slice_new0 (NMDnsConfigData); - data->ifindex = ifindex; - data->self = self; - c_list_init (&data->data_lst_head); + data = g_slice_new (NMDnsConfigData); + *data = (NMDnsConfigData) { + .ifindex = ifindex, + .self = self, + .data_lst_head = C_LIST_INIT (data->data_lst_head), + }; _ASSERT_config_data (data); g_hash_table_insert (priv->configs, GINT_TO_POINTER (ifindex), data); } From d578c6033bd77e931c971462df21db97bf6c5cc1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Nov 2020 15:35:48 +0100 Subject: [PATCH 2/9] dns: track NMDnsConfigData as keys of a dictionary There is unnecessary overhead of tracking a separate key and value in a GHashTable. Use g_hash_table_add(). (cherry picked from commit d10d96a45c56fad99cd446d9a3952b6100d46191) (cherry picked from commit 59d48fcc35a119ce247b9103a83d37320749d9e6) --- src/dns/nm-dns-manager.c | 13 ++++++++----- src/dns/nm-dns-manager.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index a88e64428c..09a9e7125d 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -1671,7 +1671,7 @@ nm_dns_manager_set_ip_config (NMDnsManager *self, priv = NM_DNS_MANAGER_GET_PRIVATE (self); - data = g_hash_table_lookup (priv->configs, GINT_TO_POINTER (ifindex)); + data = g_hash_table_lookup (priv->configs, &ifindex); if (!data) ip_data = NULL; else @@ -1687,7 +1687,7 @@ nm_dns_manager_set_ip_config (NMDnsManager *self, /* deleting a config doesn't invalidate the configs' sort order. */ _ip_config_data_free (ip_data); if (c_list_is_empty (&data->data_lst_head)) - g_hash_table_remove (priv->configs, GINT_TO_POINTER (ifindex)); + g_hash_table_remove (priv->configs, &ifindex); goto changed; } @@ -1705,7 +1705,7 @@ nm_dns_manager_set_ip_config (NMDnsManager *self, .data_lst_head = C_LIST_INIT (data->data_lst_head), }; _ASSERT_config_data (data); - g_hash_table_insert (priv->configs, GINT_TO_POINTER (ifindex), data); + g_hash_table_add (priv->configs, data); } if (!ip_data) @@ -2348,8 +2348,11 @@ nm_dns_manager_init (NMDnsManager *self) priv->config = g_object_ref (nm_config_get ()); - priv->configs = g_hash_table_new_full (nm_direct_hash, NULL, - NULL, (GDestroyNotify) _config_data_free); + G_STATIC_ASSERT_EXPR (G_STRUCT_OFFSET (NMDnsConfigData, ifindex) == 0); + priv->configs = g_hash_table_new_full (nm_pint_hash, + nm_pint_equals, + (GDestroyNotify) _config_data_free, + NULL); /* Set the initial hash */ compute_hash (self, NULL, NM_DNS_MANAGER_GET_PRIVATE (self)->hash); diff --git a/src/dns/nm-dns-manager.h b/src/dns/nm-dns-manager.h index 2a0c9deee3..b60414566c 100644 --- a/src/dns/nm-dns-manager.h +++ b/src/dns/nm-dns-manager.h @@ -41,9 +41,9 @@ typedef struct { } NMDnsIPConfigData; typedef struct _NMDnsConfigData { + int ifindex; struct _NMDnsManager *self; CList data_lst_head; - int ifindex; } NMDnsConfigData; #define NM_TYPE_DNS_MANAGER (nm_dns_manager_get_type ()) From 4ed2ceda1046d0655ff3e6ca9e8cccff24b0e8cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Nov 2020 16:23:15 +0100 Subject: [PATCH 3/9] dns: cleanup handling of shadowed priorities rebuild_domain_lists() domain_is_shadowed() only works, because we pre-sort all items. When we call domain_is_shadowed(), then "priority" must be not smaller than any priority already in the dictionary. Let's add an nm_assert() for that. While at it, I also found it ugly to rely on GPOINTER_TO_INT(g_hash_table_lookup(ht, domain)) returning zero to know whether the domain is tracked. While more cumbersome, we should check whether the value is in the hash (and not). Not whether the value does not translate to zero. Add domain_ht_get_priority() for that. (cherry picked from commit 5902f1c91f7c2dbc22a2798d7ff33fd451480454) (cherry picked from commit 2502b885115c672c3f7b6df33dc561cb1de5befd) --- src/dns/nm-dns-manager.c | 45 +++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 09a9e7125d..4315cdc301 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -1255,6 +1255,19 @@ get_ip_rdns_domains (NMIPConfig *ip_config) return _nm_utils_strv_cleanup (strv, FALSE, FALSE, TRUE); } +static gboolean +domain_ht_get_priority (GHashTable *ht, const char *domain, int *out_priority) +{ + gpointer ptr; + + if (!ht || !g_hash_table_lookup_extended (ht, domain, NULL, &ptr)) { + *out_priority = 0; + return FALSE; + } + *out_priority = GPOINTER_TO_INT (ptr); + return TRUE; +} + /* Check if the domain is shadowed by a parent domain with more negative priority */ static gboolean domain_is_shadowed (GHashTable *ht, @@ -1271,24 +1284,28 @@ domain_is_shadowed (GHashTable *ht, nm_assert (!g_hash_table_contains (ht, domain)); - parent_priority = GPOINTER_TO_INT (g_hash_table_lookup (ht, "")); - if ( parent_priority < 0 - && parent_priority < priority) { - *out_parent = ""; - *out_parent_priority = parent_priority; - return TRUE; + if (domain_ht_get_priority (ht, "", &parent_priority)) { + nm_assert (parent_priority <= priority); + if ( parent_priority < 0 + && parent_priority < priority) { + *out_parent = ""; + *out_parent_priority = parent_priority; + return TRUE; + } } parent = strchr (domain, '.'); while ( parent && parent[1]) { parent++; - parent_priority = GPOINTER_TO_INT (g_hash_table_lookup (ht, parent)); - if ( parent_priority < 0 - && parent_priority < priority) { - *out_parent = parent; - *out_parent_priority = parent_priority; - return TRUE; + if (domain_ht_get_priority (ht, parent, &parent_priority)) { + nm_assert (parent_priority <= priority); + if ( parent_priority < 0 + && parent_priority < priority) { + *out_parent = parent; + *out_parent_priority = parent_priority; + return TRUE; + } } parent = strchr (parent, '.'); } @@ -1399,8 +1416,8 @@ rebuild_domain_lists (NMDnsManager *self) domain_clean = nm_utils_parse_dns_domain (domains[i], NULL); /* Remove domains with lower priority */ - old_priority = GPOINTER_TO_INT (nm_g_hash_table_lookup (ht, domain_clean)); - if (old_priority != 0) { + if (domain_ht_get_priority (ht, domain_clean, &old_priority)) { + nm_assert (old_priority <= priority); if (old_priority < priority) { _LOGT ("plugin: drop domain '%s' (i=%d, p=%d) because it already exists with p=%d", domains[i], ip_data->data->ifindex, From 50e04c6bc34bd899f10c430c522d19efee6cc36f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Nov 2020 16:35:36 +0100 Subject: [PATCH 4/9] dns: assert that priorities in rebuild_domain_lists() are increasing This is nm_assert(). The compiler should be able to completely eliminate this code in production. (cherry picked from commit 05f8ccc8179c821a83f0244127514ee3f679212d) (cherry picked from commit 0bfc2b6db9e59e1951072b753ee1e7182388dace) --- src/dns/nm-dns-manager.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 4315cdc301..80ce750da8 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -1320,6 +1320,7 @@ rebuild_domain_lists (NMDnsManager *self) gs_unref_hashtable GHashTable *ht = NULL; gs_unref_hashtable GHashTable *wildcard_entries = NULL; CList *head; + int prev_priority = G_MININT; head = _ip_config_lst_head (self); c_list_for_each_entry (ip_data, head, ip_config_lst) { @@ -1369,7 +1370,10 @@ rebuild_domain_lists (NMDnsManager *self) n_domains = nm_ip_config_get_num_domains (ip_config); priority = nm_ip_config_get_dns_priority (ip_config); + nm_assert (priority != 0); + nm_assert (prev_priority <= priority); + prev_priority = priority; cap_dom = 2u + NM_MAX (n_domains, n_searches); From 5239cb1e070ce39e52a78b1dd7745662438c4f1f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Nov 2020 16:49:19 +0100 Subject: [PATCH 5/9] dns: assert domains are unset at start of rebuild_domain_lists() (cherry picked from commit a875d154de3baac6a8b5601dcb30f0316de4c8cb) (cherry picked from commit 83c760014e3d8ce5e362ba39c462fbd09646f821) --- src/dns/nm-dns-manager.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 80ce750da8..c467cdc2d7 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -1323,6 +1323,16 @@ rebuild_domain_lists (NMDnsManager *self) int prev_priority = G_MININT; head = _ip_config_lst_head (self); + +#if NM_MORE_ASSERTS + /* we call clear_domain_lists() at the end of update. We + * don't expect any domain settings here. */ + c_list_for_each_entry (ip_data, head, ip_config_lst) { + nm_assert (!ip_data->domains.search); + nm_assert (!ip_data->domains.reverse); + } +#endif + c_list_for_each_entry (ip_data, head, ip_config_lst) { NMIPConfig *ip_config = ip_data->ip_config; gboolean add_wildcard = FALSE; @@ -1377,9 +1387,7 @@ rebuild_domain_lists (NMDnsManager *self) cap_dom = 2u + NM_MAX (n_domains, n_searches); - g_free (ip_data->domains.search); domains = g_new (const char *, cap_dom); - ip_data->domains.search = domains; num_dom1 = 0; @@ -1445,7 +1453,9 @@ rebuild_domain_lists (NMDnsManager *self) nm_assert (num_dom2 < cap_dom); domains[num_dom2] = NULL; - g_strfreev (ip_data->domains.reverse); + nm_assert (!ip_data->domains.search); + nm_assert (!ip_data->domains.reverse); + ip_data->domains.search = domains; ip_data->domains.reverse = get_ip_rdns_domains (ip_config); } } From 4d8ba83dffe3a11de5474001da8e98ade722e541 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Nov 2020 17:38:39 +0100 Subject: [PATCH 6/9] dns: fix handling default routing domains with systemd-resolved We used to set "~." domains for all devices that should be used for resolving unknown domains. Systemd-resolved also supports setting "SetLinkDefaultRoute()". We should only set the wildcard domain if we want that this interface is used exclusively. Otherwise, we should only set DefaultRoute. See ([1], [2], [3], [4]). Otherwise the bad effect is if other components (wg-quick) want to set exclusive DNS lookups on their link. That is achieved by explicitly adding "~." and that is also what resolved's `/usr/sbin/resolvconf -x` does. If NetworkManager sets "~." for interfaces that are not important and should not be used exclusively, then this steals the DNS requests from those external components. In NetworkManager we know whether a link should get exclusive lookups based on the "ipv[46].dns-priority" setting. [1] https://www.freedesktop.org/software/systemd/man/org.freedesktop.resolve1.html [2] https://www.freedesktop.org/software/systemd/man/systemd-resolved.service.html [3] https://github.com/systemd/systemd/issues/17529#issuecomment-730522444 [4] https://github.com/systemd/systemd/pull/17678 (cherry picked from commit ee9fab03613e30f818d56217a968d1fabd5ea8d7) (cherry picked from commit b8dab47705dca2203163b8dec13aad0c97f5b646) --- src/dns/nm-dns-dnsmasq.c | 18 +++-- src/dns/nm-dns-manager.c | 118 ++++++++++++++++++++++++------ src/dns/nm-dns-manager.h | 22 ++++++ src/dns/nm-dns-systemd-resolved.c | 23 ++++-- 4 files changed, 147 insertions(+), 34 deletions(-) diff --git a/src/dns/nm-dns-dnsmasq.c b/src/dns/nm-dns-dnsmasq.c index fdff3af16f..73ceeeb31e 100644 --- a/src/dns/nm-dns-dnsmasq.c +++ b/src/dns/nm-dns-dnsmasq.c @@ -788,14 +788,18 @@ add_ip_config (NMDnsDnsmasq *self, GVariantBuilder *servers, const NMDnsIPConfig for (i = 0; i < num; i++) { addr = nm_ip_config_get_nameserver (ip_config, i); ip_addr_to_string (addr_family, addr, iface, ip_addr_to_string_buf); - for (j = 0; ip_data->domains.search[j]; j++) { - domain = nm_utils_parse_dns_domain (ip_data->domains.search[j], NULL); - add_dnsmasq_nameserver (self, - servers, - ip_addr_to_string_buf, - domain[0] ? domain : NULL); - } + if (!ip_data->domains.has_default_route_explicit && ip_data->domains.has_default_route) + add_dnsmasq_nameserver (self, servers, ip_addr_to_string_buf, NULL); + if (ip_data->domains.search) { + for (j = 0; ip_data->domains.search[j]; j++) { + domain = nm_utils_parse_dns_domain (ip_data->domains.search[j], NULL); + add_dnsmasq_nameserver (self, + servers, + ip_addr_to_string_buf, + domain[0] ? domain : NULL); + } + } if (ip_data->domains.reverse) { for (j = 0; ip_data->domains.reverse[j]; j++) { add_dnsmasq_nameserver (self, servers, diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index c467cdc2d7..d6e7b4d10a 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -218,6 +218,26 @@ _ASSERT_ip_config_data (const NMDnsIPConfigData *ip_data) nm_assert (NM_IS_IP_CONFIG (ip_data->ip_config, AF_UNSPEC)); nm_assert (c_list_contains (&ip_data->data->data_lst_head, &ip_data->data_lst)); nm_assert (ip_data->data->ifindex == nm_ip_config_get_ifindex (ip_data->ip_config)); +#if NM_MORE_ASSERTS > 5 + { + gboolean has_default = FALSE; + gsize i; + + for (i = 0; ip_data->domains.search && ip_data->domains.search; i++) { + const char *d = ip_data->domains.search[i]; + + d = nm_utils_parse_dns_domain (d, NULL); + nm_assert (d); + if (d[0] == '\0') + has_default = TRUE; + } + nm_assert (has_default == ip_data->domains.has_default_route_explicit); + if (ip_data->domains.has_default_route_explicit) + nm_assert (ip_data->domains.has_default_route_exclusive); + if (ip_data->domains.has_default_route_exclusive) + nm_assert (ip_data->domains.has_default_route); + } +#endif } static NMDnsIPConfigData * @@ -1330,6 +1350,9 @@ rebuild_domain_lists (NMDnsManager *self) c_list_for_each_entry (ip_data, head, ip_config_lst) { nm_assert (!ip_data->domains.search); nm_assert (!ip_data->domains.reverse); + nm_assert (!ip_data->domains.has_default_route_explicit); + nm_assert (!ip_data->domains.has_default_route_exclusive); + nm_assert (!ip_data->domains.has_default_route); } #endif @@ -1370,8 +1393,11 @@ rebuild_domain_lists (NMDnsManager *self) guint n_domains; guint num_dom1; guint num_dom2; - guint cap_dom; + guint n_domains_allocated; guint i; + gboolean has_default_route_maybe = FALSE; + gboolean has_default_route_explicit = FALSE; + gboolean has_default_route_auto = FALSE; if (!nm_ip_config_get_num_nameservers (ip_config)) continue; @@ -1385,12 +1411,6 @@ rebuild_domain_lists (NMDnsManager *self) nm_assert (prev_priority <= priority); prev_priority = priority; - cap_dom = 2u + NM_MAX (n_domains, n_searches); - - domains = g_new (const char *, cap_dom); - - num_dom1 = 0; - /* Add wildcard lookup domain to connections with the default route. * If there is no default route, add the wildcard domain to all non-VPN * connections */ @@ -1401,12 +1421,17 @@ rebuild_domain_lists (NMDnsManager *self) * whether it is suitable for certain operations (like having an automatically * added "~" domain). */ if (g_hash_table_contains (wildcard_entries, ip_data)) - domains[num_dom1++] = "~"; + has_default_route_maybe = TRUE; } else { if (ip_data->ip_config_type != NM_DNS_IP_CONFIG_TYPE_VPN) - domains[num_dom1++] = "~"; + has_default_route_maybe = TRUE; } + n_domains_allocated = (n_searches > 0 ? n_searches : n_domains) + 1u; + domains = g_new (const char *, n_domains_allocated); + + num_dom1 = 0; + /* searches are preferred over domains */ if (n_searches > 0) { for (i = 0; i < n_searches; i++) @@ -1416,47 +1441,93 @@ rebuild_domain_lists (NMDnsManager *self) domains[num_dom1++] = nm_ip_config_get_domain (ip_config, i); } - nm_assert (num_dom1 < cap_dom); + nm_assert (num_dom1 < n_domains_allocated); num_dom2 = 0; - for (i = 0; i < num_dom1; i++) { + for (i = 0; TRUE; i++) { + const char *domain_full; const char *domain_clean; const char *parent; int old_priority; int parent_priority; + gboolean check_default_route; - domain_clean = nm_utils_parse_dns_domain (domains[i], NULL); + if (i < num_dom1) { + check_default_route = FALSE; + domain_full = domains[i]; + domain_clean = nm_utils_parse_dns_domain (domains[i], NULL); + } else if (i == num_dom1) { + if (!has_default_route_maybe) + continue; + if (has_default_route_explicit) + continue; + check_default_route = TRUE; + domain_full = "~"; + domain_clean = ""; + } else + break; /* Remove domains with lower priority */ if (domain_ht_get_priority (ht, domain_clean, &old_priority)) { nm_assert (old_priority <= priority); if (old_priority < priority) { - _LOGT ("plugin: drop domain '%s' (i=%d, p=%d) because it already exists with p=%d", - domains[i], ip_data->data->ifindex, - priority, old_priority); + _LOGT ("plugin: drop domain %s%s%s (i=%d, p=%d) because it already exists " + "with p=%d", + NM_PRINT_FMT_QUOTED (!check_default_route, + "'", + domain_full, + "'", + ""), + ip_data->data->ifindex, + priority, + old_priority); continue; } } else if (domain_is_shadowed (ht, domain_clean, priority, &parent, &parent_priority)) { - _LOGT ("plugin: drop domain '%s' (i=%d, p=%d) shadowed by '%s' (p=%d)", - domains[i], - ip_data->data->ifindex, priority, - parent, parent_priority); + _LOGT ("plugin: drop domain %s%s%s (i=%d, p=%d) shadowed by '%s' (p=%d)", + NM_PRINT_FMT_QUOTED (!check_default_route, + "'", + domain_full, + "'", + ""), + ip_data->data->ifindex, + priority, + parent, + parent_priority); continue; } - _LOGT ("plugin: add domain '%s' (i=%d, p=%d)", domains[i], ip_data->data->ifindex, priority); + _LOGT ("plugin: add domain %s%s%s (i=%d, p=%d)", + NM_PRINT_FMT_QUOTED (!check_default_route, "'", domain_full, "'", ""), + ip_data->data->ifindex, + priority); + if (!ht) ht = g_hash_table_new (nm_str_hash, g_str_equal); g_hash_table_insert (ht, (gpointer) domain_clean, GINT_TO_POINTER (priority)); - domains[num_dom2++] = domains[i]; + + if (check_default_route) + has_default_route_auto = TRUE; + else { + nm_assert (num_dom2 <= num_dom1); + nm_assert (num_dom2 < n_domains_allocated); + domains[num_dom2++] = domain_full; + if (domain_clean[0] == '\0') + has_default_route_explicit = TRUE; + } } - nm_assert (num_dom2 < cap_dom); + nm_assert (num_dom2 < n_domains_allocated); domains[num_dom2] = NULL; nm_assert (!ip_data->domains.search); nm_assert (!ip_data->domains.reverse); ip_data->domains.search = domains; ip_data->domains.reverse = get_ip_rdns_domains (ip_config); + ip_data->domains.has_default_route_explicit = has_default_route_explicit; + ip_data->domains.has_default_route_exclusive = has_default_route_explicit + || (priority < 0 && has_default_route_auto); + ip_data->domains.has_default_route = ip_data->domains.has_default_route_exclusive + || has_default_route_auto; } } @@ -1470,6 +1541,9 @@ clear_domain_lists (NMDnsManager *self) c_list_for_each_entry (ip_data, head, ip_config_lst) { nm_clear_g_free (&ip_data->domains.search); nm_clear_pointer (&ip_data->domains.reverse, g_strfreev); + ip_data->domains.has_default_route_explicit = FALSE; + ip_data->domains.has_default_route_exclusive = FALSE; + ip_data->domains.has_default_route = FALSE; } } diff --git a/src/dns/nm-dns-manager.h b/src/dns/nm-dns-manager.h index b60414566c..444d5d083c 100644 --- a/src/dns/nm-dns-manager.h +++ b/src/dns/nm-dns-manager.h @@ -37,6 +37,28 @@ typedef struct { struct { const char **search; char **reverse; + + /* Whether "search" explicitly contains a default route "~" + * or "". It is redundant information, but for faster lookup. */ + bool has_default_route_explicit : 1; + + /* Whether an explicit "~" search domain should be added. + * For systemd-resolved, this configured an explicit wildcard + * search domain, and should be used for profiles with negative + * DNS priority. + * + * If "has_default_route_explicit", this is always TRUE and implied. + * + * With systemd-resolved, if TRUE we will set a "." search domain. + */ + bool has_default_route_exclusive : 1; + + /* Whether the device should be used for any domains "~". + * + * If "has_default_route_exclusive", this is always TRUE and implied. + * + * With systemd-resolved, this is the value for SetLinkDefaultRoute(). */ + bool has_default_route : 1; } domains; } NMDnsIPConfigData; diff --git a/src/dns/nm-dns-systemd-resolved.c b/src/dns/nm-dns-systemd-resolved.c index 159cd6f077..b2c9795293 100644 --- a/src/dns/nm-dns-systemd-resolved.c +++ b/src/dns/nm-dns-systemd-resolved.c @@ -146,14 +146,14 @@ update_add_ip_config (NMDnsSystemdResolved *self, gsize addr_size; guint i, n; gboolean is_routing; - const char **iter; const char *domain; gboolean has_config = FALSE; addr_family = nm_ip_config_get_addr_family (data->ip_config); addr_size = nm_utils_addr_family_to_size (addr_family); - if (!data->domains.search || !data->domains.search[0]) + if ((!data->domains.search || !data->domains.search[0]) + && !data->domains.has_default_route_exclusive) return FALSE; n = nm_ip_config_get_num_nameservers (data->ip_config); @@ -169,11 +169,17 @@ update_add_ip_config (NMDnsSystemdResolved *self, has_config = TRUE; } - for (iter = data->domains.search; *iter; iter++) { - domain = nm_utils_parse_dns_domain (*iter, &is_routing); - g_variant_builder_add (domains, "(sb)", domain[0] ? domain : ".", is_routing); + if (!data->domains.has_default_route_explicit && data->domains.has_default_route_exclusive) { + g_variant_builder_add (domains, "(sb)", ".", TRUE); has_config = TRUE; } + if (data->domains.search) { + for (i = 0; data->domains.search[i]; i++) { + domain = nm_utils_parse_dns_domain (data->domains.search[i], &is_routing); + g_variant_builder_add (domains, "(sb)", domain[0] ? domain : ".", is_routing); + has_config = TRUE; + } + } return has_config; } @@ -200,6 +206,7 @@ prepare_one_interface (NMDnsSystemdResolved *self, InterfaceConfig *ic) NMSettingConnectionLlmnr llmnr = NM_SETTING_CONNECTION_LLMNR_DEFAULT; const char *mdns_arg = NULL, *llmnr_arg = NULL; gboolean has_config = FALSE; + gboolean has_default_route = FALSE; g_variant_builder_init (&dns, G_VARIANT_TYPE ("(ia(iay))")); g_variant_builder_add (&dns, "i", ic->ifindex); @@ -215,6 +222,9 @@ prepare_one_interface (NMDnsSystemdResolved *self, InterfaceConfig *ic) has_config |= update_add_ip_config (self, &dns, &domains, data); + if (data->domains.has_default_route) + has_default_route = TRUE; + if (NM_IS_IP4_CONFIG (ip_config)) { mdns = NM_MAX (mdns, nm_ip4_config_mdns_get (NM_IP4_CONFIG (ip_config))); llmnr = NM_MAX (llmnr, nm_ip4_config_llmnr_get (NM_IP4_CONFIG (ip_config))); @@ -265,6 +275,9 @@ prepare_one_interface (NMDnsSystemdResolved *self, InterfaceConfig *ic) _request_item_append (&priv->request_queue_lst_head, "SetLinkDomains", g_variant_builder_end (&domains)); + _request_item_append (&priv->request_queue_lst_head, + "SetLinkDefaultRoute", + g_variant_new ("(ib)", ic->ifindex, has_default_route)); _request_item_append (&priv->request_queue_lst_head, "SetLinkMulticastDNS", g_variant_new ("(is)", ic->ifindex, mdns_arg ?: "")); From e4899911e9f8a3f52753b5de1246256082d2b5cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Nov 2020 17:08:44 +0100 Subject: [PATCH 7/9] dns: more debug logging for DNS settings in rebuild_domain_lists() (cherry picked from commit fbf1683c1a75c6eee002b5ed2eaa81a3f7126c84) (cherry picked from commit 3ce2ea7f12cc21160d43e8aa0ef76f60d7463d95) --- src/dns/nm-dns-manager.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index d6e7b4d10a..9379621c41 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -1528,6 +1528,22 @@ rebuild_domain_lists (NMDnsManager *self) || (priority < 0 && has_default_route_auto); ip_data->domains.has_default_route = ip_data->domains.has_default_route_exclusive || has_default_route_auto; + + { + gs_free char *str1 = NULL; + gs_free char *str2 = NULL; + + _LOGT ("plugin: settings: ifindex=%d, priority=%d, default-route=%d%s, search=%s, " + "reverse=%s", + ip_data->data->ifindex, + priority, + ip_data->domains.has_default_route, + ip_data->domains.has_default_route_explicit + ? " (explicit)" + : (ip_data->domains.has_default_route_exclusive ? " (exclusive)" : ""), + (str1 = g_strjoinv (",", (char **) ip_data->domains.search)), + (str2 = g_strjoinv (",", ip_data->domains.reverse))); + } } } From 38bf76032f0e43c5c8765cd913fb574f61c8abe0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Nov 2020 18:26:41 +0100 Subject: [PATCH 8/9] dns: set first Domains/DefaultRoute in systemd-resolved before DNS (cherry picked from commit 95017dccdd2aa390053d3d3a16273daf937535b0) (cherry picked from commit ab0dcafb6e95baa5cb8566050a134084c1561d2c) --- src/dns/nm-dns-systemd-resolved.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dns/nm-dns-systemd-resolved.c b/src/dns/nm-dns-systemd-resolved.c index b2c9795293..c04586a406 100644 --- a/src/dns/nm-dns-systemd-resolved.c +++ b/src/dns/nm-dns-systemd-resolved.c @@ -269,9 +269,6 @@ prepare_one_interface (NMDnsSystemdResolved *self, InterfaceConfig *ic) if (!nm_str_is_empty (mdns_arg) || !nm_str_is_empty (llmnr_arg)) has_config = TRUE; - _request_item_append (&priv->request_queue_lst_head, - "SetLinkDNS", - g_variant_builder_end (&dns)); _request_item_append (&priv->request_queue_lst_head, "SetLinkDomains", g_variant_builder_end (&domains)); @@ -284,6 +281,9 @@ prepare_one_interface (NMDnsSystemdResolved *self, InterfaceConfig *ic) _request_item_append (&priv->request_queue_lst_head, "SetLinkLLMNR", g_variant_new ("(is)", ic->ifindex, llmnr_arg ?: "")); + _request_item_append (&priv->request_queue_lst_head, + "SetLinkDNS", + g_variant_builder_end (&dns)); return has_config; } From ca61fe81fff06f18edf158d0b69b7fa994274109 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Nov 2020 22:50:24 +0100 Subject: [PATCH 9/9] dns: fix accessing NULL domains.reverse array in rebuild_domain_lists() Fixes: fbf1683c1a75 ('dns: more debug logging for DNS settings in rebuild_domain_lists()') (cherry picked from commit 937c8a466928600390d221da233b6b3eed82c3a6) (cherry picked from commit b6a9242b1ad218fe6422d517c9913d4ca0bb2c45) --- src/dns/nm-dns-manager.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 9379621c41..f490dc835c 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -1542,7 +1542,9 @@ rebuild_domain_lists (NMDnsManager *self) ? " (explicit)" : (ip_data->domains.has_default_route_exclusive ? " (exclusive)" : ""), (str1 = g_strjoinv (",", (char **) ip_data->domains.search)), - (str2 = g_strjoinv (",", ip_data->domains.reverse))); + ( ip_data->domains.reverse + ? (str2 = g_strjoinv (",", ip_data->domains.reverse)) + : "")); } } }