From 86bb09c93be39d5a8c6602cdfb7958bde5c29059 Mon Sep 17 00:00:00 2001 From: Tom Sobczynski Date: Tue, 25 Apr 2023 19:19:30 -0400 Subject: [PATCH 1/6] dns: generate correct search domain for hostnames on non-public TLD dns-manager uses the Mozilla Public Suffix List to determine an appropriate search domain when generating /etc/resolv.conf. It is presumed that if the hostname is "example.com", the user does not want to automatically search "com" for unqualified hostnames, which is reasonable. To implement that, prior to the fix, domain_is_valid() implicitly used the PSL "prevailing star rule", which had the consequence of assuming that any top-level domain (TLD) is public whether it is on the official suffix list or not. That meant "example.local" or "example.localdomain" would not result in searching "local" or "localdomain" respectively, but rather /etc/resolv.conf would contain the full hostname "example.local" as the search domain and not give users what they expect. The fix here uses the newer PSL API function that allows us to turn off the "prevailing star rule" so that "local" and "localdomain" are NOT considered public TLDs because they are not literally on the suffix list. That in turn gives us the search domain "local" or "localdomain" in /etc/resolv.conf and allows unqualified hostname lookups "e.g., resolvectl query example" to find example.local while example.com still maintains the previous behavior (i.e., search domain of "example.com" rather than "com"). [thaller@redhat.com: reworded commit message] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1281 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1613 --- src/core/dns/nm-dns-manager.c | 51 +++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index fb65afcadc..e4181de30a 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -173,13 +173,41 @@ NM_DEFINE_SINGLETON_GETTER(NMDnsManager, nm_dns_manager_get, NM_TYPE_DNS_MANAGER /*****************************************************************************/ static gboolean -domain_is_valid(const char *domain, gboolean check_public_suffix) +domain_is_valid(const char *domain, gboolean check_public_suffix, gboolean assume_any_tld_is_public) { +#if WITH_LIBPSL + /* ifdef to fall back to old API function on platforms with older LIBPSL */ +#ifdef PSL_TYPE_NO_STAR_RULE + int type = PSL_TYPE_ANY; +#endif +#endif + if (*domain == '\0') return FALSE; #if WITH_LIBPSL + /* ifdef to fall back to old API function on platforms with older LIBPSL */ +#ifdef PSL_TYPE_NO_STAR_RULE + /* + * If we use PSL_TYPE_ANY, any TLD (top-level domain, i.e., domain + * with no dots) is considered *public* by the PSL library even if + * it is *not* on the official suffix list. This is the implicit + * behavior of the older API function psl_is_public_suffix(). + * To inhibit that and only deem TLDs explicitly listed in the PSL + * as public, we need to turn off the "prevailing star rule" with + * PSL_TYPE_NO_STAR_RULE. + * For documentation on psl_is_public_suffix2(), see: + * https://rockdaboot.github.io/libpsl/libpsl-Public-Suffix-List-functions.html#psl-is-public-suffix2 + * For more on the public suffix format, including wildcards: + * https://github.com/publicsuffix/list/wiki/Format#format + */ + if (!assume_any_tld_is_public) + type = PSL_TYPE_NO_STAR_RULE; + if (check_public_suffix && psl_is_public_suffix2(psl_builtin(), domain, type)) + return FALSE; +#else if (check_public_suffix && psl_is_public_suffix(psl_builtin(), domain)) return FALSE; +#endif #endif return TRUE; } @@ -533,7 +561,7 @@ add_dns_domains(GPtrArray *array, str = searches[i]; if (!include_routing && domain_is_routing(str)) continue; - if (!domain_is_valid(nm_utils_parse_dns_domain(str, NULL), FALSE)) + if (!domain_is_valid(nm_utils_parse_dns_domain(str, NULL), FALSE, TRUE)) continue; add_string_item(array, str, dup); } @@ -542,7 +570,7 @@ add_dns_domains(GPtrArray *array, str = domains[i]; if (!include_routing && domain_is_routing(str)) continue; - if (!domain_is_valid(nm_utils_parse_dns_domain(str, NULL), FALSE)) + if (!domain_is_valid(nm_utils_parse_dns_domain(str, NULL), FALSE, TRUE)) continue; add_string_item(array, str, dup); } @@ -1236,7 +1264,7 @@ merge_global_dns_config(NMResolvConfData *rc, NMGlobalDnsConfig *global_conf) for (i = 0; searches[i]; i++) { if (domain_is_routing(searches[i])) continue; - if (!domain_is_valid(searches[i], FALSE)) + if (!domain_is_valid(searches[i], FALSE, TRUE)) continue; add_string_item(rc->searches, searches[i], TRUE); } @@ -2111,11 +2139,16 @@ nm_dns_manager_set_hostname(NMDnsManager *self, const char *hostname, gboolean s * specified, this makes a good default.) However, if the * hostname is the top level of a domain (eg, "example.com"), * then use the hostname itself as the search (since the user - * is unlikely to want "com" as a search domain).a + * is unlikely to want "com" as a search domain). + * + * Because that logic only applies to public domains, the + * "assume_any_tld_is_public" parameter is FALSE. For + * example, it is likely that the user *does* want "local" + * or "localdomain" as a search domain. */ - if (domain_is_valid(domain, TRUE)) { + if (domain_is_valid(domain, TRUE, FALSE)) { /* pass */ - } else if (domain_is_valid(hostname, TRUE)) { + } else if (domain_is_valid(hostname, TRUE, FALSE)) { domain = hostname; } @@ -2124,8 +2157,10 @@ nm_dns_manager_set_hostname(NMDnsManager *self, const char *hostname, gboolean s } } - if (!nm_strdup_reset(&priv->hostdomain, domain)) + if (!nm_strdup_reset(&priv->hostdomain, domain)) { + _LOGT("Established |%s| as host domain.", priv->hostdomain); return; + } if (skip_update) return; From b4338de9844d5b80b0df3b8f95d61fb8b1af639b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 May 2023 11:25:36 +0200 Subject: [PATCH 2/6] dns: fix logging for resetting the host-domain The previous logging happened, when the value did not change. Log instead, when the value changes. Fixes: 86bb09c93be3 ('dns: generate correct search domain for hostnames on non-public TLD') --- src/core/dns/nm-dns-manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index e4181de30a..d321379ae0 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -2157,10 +2157,10 @@ nm_dns_manager_set_hostname(NMDnsManager *self, const char *hostname, gboolean s } } - if (!nm_strdup_reset(&priv->hostdomain, domain)) { - _LOGT("Established |%s| as host domain.", priv->hostdomain); + if (!nm_strdup_reset(&priv->hostdomain, domain)) return; - } + + _LOGT("set host domain to %s%s%s", NM_PRINT_FMT_QUOTE_STRING(priv->hostdomain)); if (skip_update) return; From 601605dbea6ef68553e50b4ad609e28edbf9f142 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 May 2023 11:35:07 +0200 Subject: [PATCH 3/6] dns: use NM_STR_HAS_SUFFIX() instead of g_str_has_suffix() It translates to a plain memcmp() as the argument is a string literal. --- src/core/dns/nm-dns-manager.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index d321379ae0..34c561e323 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -2128,7 +2128,8 @@ nm_dns_manager_set_hostname(NMDnsManager *self, const char *hostname, gboolean s /* Certain hostnames we don't want to include in resolv.conf 'searches' */ if (hostname && nm_utils_is_specific_hostname(hostname) - && !g_str_has_suffix(hostname, ".in-addr.arpa") && !nm_inet_is_valid(AF_UNSPEC, hostname)) { + && !NM_STR_HAS_SUFFIX(hostname, ".in-addr.arpa") + && !nm_inet_is_valid(AF_UNSPEC, hostname)) { domain = strchr(hostname, '.'); if (domain) { domain++; From 4ddbf32f1bcc181b6e6a953bfcdfc39121692aa9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 May 2023 11:37:27 +0200 Subject: [PATCH 4/6] dns/trivial: rename check_public_suffix parameter of domain_is_valid() Names are important. The previous name was counter intuitive for what the behavior was. --- src/core/dns/nm-dns-manager.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index 34c561e323..56c91aaa37 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -173,7 +173,9 @@ NM_DEFINE_SINGLETON_GETTER(NMDnsManager, nm_dns_manager_get, NM_TYPE_DNS_MANAGER /*****************************************************************************/ static gboolean -domain_is_valid(const char *domain, gboolean check_public_suffix, gboolean assume_any_tld_is_public) +domain_is_valid(const char *domain, + gboolean reject_public_suffix, + gboolean assume_any_tld_is_public) { #if WITH_LIBPSL /* ifdef to fall back to old API function on platforms with older LIBPSL */ @@ -202,10 +204,10 @@ domain_is_valid(const char *domain, gboolean check_public_suffix, gboolean assum */ if (!assume_any_tld_is_public) type = PSL_TYPE_NO_STAR_RULE; - if (check_public_suffix && psl_is_public_suffix2(psl_builtin(), domain, type)) + if (reject_public_suffix && psl_is_public_suffix2(psl_builtin(), domain, type)) return FALSE; #else - if (check_public_suffix && psl_is_public_suffix(psl_builtin(), domain)) + if (reject_public_suffix && psl_is_public_suffix(psl_builtin(), domain)) return FALSE; #endif #endif From db3da65c6c3613f3ec8e3c8b2af991c4c9967ab0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 May 2023 11:08:19 +0200 Subject: [PATCH 5/6] dns: refactor domain_is_valid() to combine #if blocks --- src/core/dns/nm-dns-manager.c | 62 ++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index 56c91aaa37..2eeb48f21a 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -177,40 +177,42 @@ domain_is_valid(const char *domain, gboolean reject_public_suffix, gboolean assume_any_tld_is_public) { -#if WITH_LIBPSL - /* ifdef to fall back to old API function on platforms with older LIBPSL */ -#ifdef PSL_TYPE_NO_STAR_RULE - int type = PSL_TYPE_ANY; -#endif -#endif - if (*domain == '\0') return FALSE; -#if WITH_LIBPSL - /* ifdef to fall back to old API function on platforms with older LIBPSL */ -#ifdef PSL_TYPE_NO_STAR_RULE - /* - * If we use PSL_TYPE_ANY, any TLD (top-level domain, i.e., domain - * with no dots) is considered *public* by the PSL library even if - * it is *not* on the official suffix list. This is the implicit - * behavior of the older API function psl_is_public_suffix(). - * To inhibit that and only deem TLDs explicitly listed in the PSL - * as public, we need to turn off the "prevailing star rule" with - * PSL_TYPE_NO_STAR_RULE. - * For documentation on psl_is_public_suffix2(), see: - * https://rockdaboot.github.io/libpsl/libpsl-Public-Suffix-List-functions.html#psl-is-public-suffix2 - * For more on the public suffix format, including wildcards: - * https://github.com/publicsuffix/list/wiki/Format#format - */ - if (!assume_any_tld_is_public) - type = PSL_TYPE_NO_STAR_RULE; - if (reject_public_suffix && psl_is_public_suffix2(psl_builtin(), domain, type)) - return FALSE; + + if (reject_public_suffix) { + int is_pub; + +#if !WITH_LIBPSL + /* Without libpsl, we cannot detect that the domain is a public suffix, we assume + * the domain is not and valid. */ + is_pub = FALSE; +#elif defined(PSL_TYPE_NO_STAR_RULE) + /* + * If we use PSL_TYPE_ANY, any TLD (top-level domain, i.e., domain + * with no dots) is considered *public* by the PSL library even if + * it is *not* on the official suffix list. This is the implicit + * behavior of the older API function psl_is_public_suffix(). + * To inhibit that and only deem TLDs explicitly listed in the PSL + * as public, we need to turn off the "prevailing star rule" with + * PSL_TYPE_NO_STAR_RULE. + * For documentation on psl_is_public_suffix2(), see: + * https://rockdaboot.github.io/libpsl/libpsl-Public-Suffix-List-functions.html#psl-is-public-suffix2 + * For more on the public suffix format, including wildcards: + * https://github.com/publicsuffix/list/wiki/Format#format + */ + is_pub = + psl_is_public_suffix2(psl_builtin(), + domain, + assume_any_tld_is_public ? PSL_TYPE_ANY : PSL_TYPE_NO_STAR_RULE); #else - if (reject_public_suffix && psl_is_public_suffix(psl_builtin(), domain)) - return FALSE; -#endif + is_pub = psl_is_public_suffix(psl_builtin(), domain); #endif + + if (is_pub) + return FALSE; + } + return TRUE; } From 6a4097fe0b9655130ff7803ca6f550ce99d77aa6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 May 2023 11:28:51 +0200 Subject: [PATCH 6/6] NEWS: update --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 11df54b83f..57bd987e72 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,8 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! * Setting "connection.stable-id=default${CONNECTION}" changed behavior to be identical to the built-in default value when the stable-id is not set. +* When configuring hostnames in non-public TLD (like "example.local"), use + the TLD as default search domain instead of the full hostname. ============================================= NetworkManager-1.42