merge: branch 'bg/dhcp-overlong-hostname'

https://bugzilla.redhat.com/show_bug.cgi?id=2033643
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/572

src/core/dns/nm-dns-dnsmasq.c

@@ -848,7 +848,7 @@ static GVariant *
 create_update_args(NMDnsDnsmasq            *self,
                    const NMGlobalDnsConfig *global_config,
                    const CList             *ip_data_lst_head,
-                   const char              *hostname)
+                   const char              *hostdomain)
 {
     GVariantBuilder          servers;
     const NMDnsConfigIPData *ip_data;
@@ -1124,7 +1124,7 @@ static gboolean
 update(NMDnsPlugin             *plugin,
        const NMGlobalDnsConfig *global_config,
        const CList             *ip_data_lst_head,
-       const char              *hostname,
+       const char              *hostdomain,
        GError                 **error)
 {
     NMDnsDnsmasq        *self = NM_DNS_DNSMASQ(plugin);
@@ -1135,7 +1135,7 @@ update(NMDnsPlugin             *plugin,
 
     nm_clear_pointer(&priv->set_server_ex_args, g_variant_unref);
     priv->set_server_ex_args =
-        g_variant_ref_sink(create_update_args(self, global_config, ip_data_lst_head, hostname));
+        g_variant_ref_sink(create_update_args(self, global_config, ip_data_lst_head, hostdomain));
 
     send_dnsmasq_update(self);
     return TRUE;

src/core/dns/nm-dns-manager.c

@@ -26,6 +26,7 @@
 
 #include "libnm-core-intern/nm-core-internal.h"
 #include "libnm-glib-aux/nm-str-buf.h"
+#include "libnm-systemd-shared/nm-sd-utils-shared.h"
 
 #include "NetworkManagerUtils.h"
 #include "devices/nm-device.h"
@@ -97,7 +98,7 @@ typedef struct {
 
     bool config_changed : 1;
 
-    char *hostname;
+    char *hostdomain;
     guint updates_queue;
 
     guint8 hash[HASH_LEN];      /* SHA1 hash of current DNS config */
@@ -1260,24 +1261,8 @@ _collect_resolv_conf_data(NMDnsManager      *self,
         }
     }
 
-    /* If the hostname is a FQDN ("dcbw.example.com"), then add the domain part of it
-     * ("example.com") to the searches list, to ensure that we can still resolve its
-     * non-FQ form ("dcbw") too. (Also, if there are no other search domains 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).
-     */
-    if (priv->hostname) {
-        const char *hostdomain = strchr(priv->hostname, '.');
-
-        if (hostdomain && !nm_utils_ipaddr_is_valid(AF_UNSPEC, priv->hostname)) {
-            hostdomain++;
-            if (domain_is_valid(hostdomain, TRUE))
-                add_string_item(rc.searches, hostdomain, TRUE);
-            else if (domain_is_valid(priv->hostname, TRUE))
-                add_string_item(rc.searches, priv->hostname, TRUE);
-        }
-    }
+    if (priv->hostdomain)
+        add_string_item(rc.searches, priv->hostdomain, TRUE);
 
     if (rc.has_trust_ad == NM_TERNARY_TRUE)
         g_ptr_array_add(rc.options, g_strdup(NM_SETTING_DNS_OPTION_TRUST_AD));
@@ -1695,7 +1680,7 @@ update_dns(NMDnsManager *self, gboolean no_caching, gboolean force_emit, GError
         nm_dns_plugin_update(priv->sd_resolve_plugin,
                              global_config,
                              _mgr_get_ip_data_lst_head(self),
-                             priv->hostname,
+                             priv->hostdomain,
                              NULL);
     }
 
@@ -1717,7 +1702,7 @@ update_dns(NMDnsManager *self, gboolean no_caching, gboolean force_emit, GError
         if (!nm_dns_plugin_update(plugin,
                                   global_config,
                                   _mgr_get_ip_data_lst_head(self),
-                                  priv->hostname,
+                                  priv->hostdomain,
                                   &plugin_error)) {
             _LOGW("update-dns: plugin %s update failed: %s", plugin_name, plugin_error->message);
 
@@ -2002,28 +1987,40 @@ done:
     return TRUE;
 }
 
-void
-nm_dns_manager_set_initial_hostname(NMDnsManager *self, const char *hostname)
-{
-    NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE(self);
-
-    g_free(priv->hostname);
-    priv->hostname = g_strdup(hostname);
-}
-
 void
 nm_dns_manager_set_hostname(NMDnsManager *self, const char *hostname, gboolean skip_update)
 {
-    NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE(self);
+    NMDnsManagerPrivate *priv   = NM_DNS_MANAGER_GET_PRIVATE(self);
+    const char          *domain = NULL;
 
     /* Certain hostnames we don't want to include in resolv.conf 'searches' */
-    if (hostname && nm_utils_is_specific_hostname(hostname) && !strstr(hostname, ".in-addr.arpa")
-        && strchr(hostname, '.')) {
-        /* pass */
-    } else
-        hostname = NULL;
+    if (hostname && nm_utils_is_specific_hostname(hostname)
+        && !g_str_has_suffix(hostname, ".in-addr.arpa")
+        && !nm_utils_ipaddr_is_valid(AF_UNSPEC, hostname)) {
+        domain = strchr(hostname, '.');
+        if (domain) {
+            domain++;
+            /* If the hostname is a FQDN ("dcbw.example.com"), then add
+             * the domain part of it ("example.com") to the searches list,
+             * to ensure that we can still resolve its non-FQ form
+             * ("dcbw") too. (Also, if there are no other search domains
+             * 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
+             */
+            if (domain_is_valid(domain, TRUE)) {
+                /* pass */
+            } else if (domain_is_valid(hostname, TRUE)) {
+                domain = hostname;
+            }
 
-    if (!nm_strdup_reset(&priv->hostname, hostname))
+            if (!nm_sd_hostname_is_valid(domain, FALSE))
+                domain = NULL;
+        }
+    }
+
+    if (!nm_strdup_reset(&priv->hostdomain, domain))
         return;
 
     if (skip_update)
@@ -2668,7 +2665,7 @@ finalize(GObject *object)
     NMDnsManager        *self = NM_DNS_MANAGER(object);
     NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE(self);
 
-    g_free(priv->hostname);
+    g_free(priv->hostdomain);
     g_free(priv->mode);
 
     G_OBJECT_CLASS(nm_dns_manager_parent_class)->finalize(object);

src/core/dns/nm-dns-manager.h

@@ -105,7 +105,6 @@ gboolean nm_dns_manager_set_ip_config(NMDnsManager         *self,
                                       NMDnsIPConfigType     ip_config_type,
                                       gboolean              replace_all);
 
-void nm_dns_manager_set_initial_hostname(NMDnsManager *self, const char *hostname);
 void nm_dns_manager_set_hostname(NMDnsManager *self, const char *hostname, gboolean skip_update);
 
 /**

src/core/dns/nm-dns-plugin.c

@@ -60,7 +60,7 @@ gboolean
 nm_dns_plugin_update(NMDnsPlugin             *self,
                      const NMGlobalDnsConfig *global_config,
                      const CList             *ip_config_lst_head,
-                     const char              *hostname,
+                     const char              *hostdomain,
                      GError                 **error)
 {
     g_return_val_if_fail(NM_DNS_PLUGIN_GET_CLASS(self)->update != NULL, FALSE);
@@ -68,7 +68,7 @@ nm_dns_plugin_update(NMDnsPlugin             *self,
     return NM_DNS_PLUGIN_GET_CLASS(self)->update(self,
                                                  global_config,
                                                  ip_config_lst_head,
-                                                 hostname,
+                                                 hostdomain,
                                                  error);
 }
 

src/core/dns/nm-dns-plugin.h

@@ -34,7 +34,7 @@ typedef struct {
     gboolean (*update)(NMDnsPlugin             *self,
                        const NMGlobalDnsConfig *global_config,
                        const CList             *ip_config_lst_head,
-                       const char              *hostname,
+                       const char              *hostdomain,
                        GError                 **error);
 
     void (*stop)(NMDnsPlugin *self);

src/core/dns/nm-dns-systemd-resolved.c

@@ -551,7 +551,7 @@ static gboolean
 update(NMDnsPlugin             *plugin,
        const NMGlobalDnsConfig *global_config,
        const CList             *ip_data_lst_head,
-       const char              *hostname,
+       const char              *hostdomain,
        GError                 **error)
 {
     NMDnsSystemdResolved          *self            = NM_DNS_SYSTEMD_RESOLVED(plugin);

src/core/dns/nm-dns-unbound.c

@@ -28,7 +28,7 @@ static gboolean
 update(NMDnsPlugin             *plugin,
        const NMGlobalDnsConfig *global_config,
        const CList             *ip_config_lst_head,
-       const char              *hostname,
+       const char              *hostdomain,
        GError                 **error)
 {
     char                 *argv[] = {DNSSEC_TRIGGER_PATH, "--async", "--update", NULL};

src/core/nm-core-utils.c

@@ -30,6 +30,7 @@
 #include "libnm-glib-aux/nm-secret-utils.h"
 #include "libnm-glib-aux/nm-time-utils.h"
 #include "libnm-glib-aux/nm-str-buf.h"
+#include "libnm-systemd-shared/nm-sd-utils-shared.h"
 #include "nm-utils.h"
 #include "libnm-core-intern/nm-core-internal.h"
 #include "nm-setting-connection.h"
@@ -5201,3 +5202,50 @@ again:
 
     return g;
 }
+
+/*****************************************************************************/
+
+/**
+ * nm_utils_shorten_hostname:
+ * @hostname: the input hostname
+ * @shortened: (out) (transfer full): on return, the shortened hostname
+ *
+ * Checks whether the input hostname is valid. If not, tries to shorten it
+ * to HOST_NAME_MAX or to the first dot, whatever comes earlier.
+ * The new hostname is returned in @shortened.
+ *
+ * Returns: %TRUE if the input hostname was already valid or if was shortened
+ * successfully; %FALSE otherwise
+ */
+gboolean
+nm_utils_shorten_hostname(const char *hostname, char **shortened)
+{
+    gs_free char *s = NULL;
+    const char   *dot;
+    gsize         l;
+
+    nm_assert(hostname);
+    nm_assert(shortened);
+
+    if (nm_sd_hostname_is_valid(hostname, FALSE)) {
+        *shortened = NULL;
+        return TRUE;
+    }
+
+    dot = strchr(hostname, '.');
+    if (dot)
+        l = (dot - hostname);
+    else
+        l = strlen(hostname);
+    l = MIN(l, (gsize) HOST_NAME_MAX);
+
+    s = g_strndup(hostname, l);
+
+    if (!nm_sd_hostname_is_valid(s, FALSE)) {
+        *shortened = NULL;
+        return FALSE;
+    }
+
+    *shortened = g_steal_pointer(&s);
+    return TRUE;
+}

src/core/nm-core-utils.h

@@ -233,6 +233,7 @@ void nm_utils_log_connection_diff(NMConnection *connection,
                                   const char   *dbus_path);
 
 gboolean nm_utils_is_specific_hostname(const char *name);
+gboolean nm_utils_shorten_hostname(const char *hostname, char **shortened);
 
 struct _NMUuid;
 

src/core/nm-policy.c

@@ -75,8 +75,9 @@ typedef struct {
     guint schedule_activate_all_id; /* idle handler for schedule_activate_all(). */
 
     NMPolicyHostnameMode hostname_mode;
-    char                *orig_hostname; /* hostname at NM start time */
-    char                *cur_hostname;  /* hostname we want to assign */
+    char                *orig_hostname;     /* hostname at NM start time */
+    char                *cur_hostname;      /* hostname we want to assign */
+    char                *cur_hostname_full; /* similar to @last_hostname, but before shortening */
     char *
         last_hostname; /* last hostname NM set (to detect if someone else changed it in the meanwhile) */
 
@@ -560,6 +561,7 @@ _set_hostname(NMPolicy *self, const char *new_hostname, const char *msg)
 {
     NMPolicyPrivate *priv         = NM_POLICY_GET_PRIVATE(self);
     gs_free char    *old_hostname = NULL;
+    gboolean         cur_hostname_full_changed;
     const char      *name;
 
     /* The incoming hostname *can* be NULL, which will get translated to
@@ -568,32 +570,50 @@ _set_hostname(NMPolicy *self, const char *new_hostname, const char *msg)
      * there was no valid hostname to start with.
      */
 
+    if (nm_strdup_reset(&priv->cur_hostname_full, new_hostname)) {
+        gs_free char *shortened = NULL;
+
+        cur_hostname_full_changed = TRUE;
+
+        if (priv->cur_hostname_full
+            && !nm_utils_shorten_hostname(priv->cur_hostname_full, &shortened)) {
+            _LOGW(LOGD_DNS,
+                  "set-hostname: hostname '%s' %s is invalid",
+                  priv->cur_hostname_full,
+                  msg);
+            return;
+        }
+
+        if (shortened) {
+            _LOGI(LOGD_DNS,
+                  "set-hostname: shortened hostname %s from '%s' to '%s'",
+                  msg,
+                  priv->cur_hostname_full,
+                  shortened);
+            nm_strdup_reset_take(&priv->cur_hostname, g_steal_pointer(&shortened));
+        } else
+            nm_strdup_reset(&priv->cur_hostname, priv->cur_hostname_full);
+    } else
+        cur_hostname_full_changed = FALSE;
+
     /* Update the DNS only if the hostname is actually
      * going to change.
      */
-    if (!nm_streq0(priv->cur_hostname, new_hostname)) {
-        g_free(priv->cur_hostname);
-        priv->cur_hostname = g_strdup(new_hostname);
-
+    if (cur_hostname_full_changed) {
         /* Notify the DNS manager of the hostname change so that the domain part, if
          * present, can be added to the search list. Set the @updating_dns flag
          * so that dns_config_changed() doesn't try again to restart DNS lookup.
          */
         priv->updating_dns = TRUE;
         nm_dns_manager_set_hostname(priv->dns_manager,
-                                    priv->cur_hostname,
+                                    priv->cur_hostname_full,
                                     all_devices_not_active(self));
         priv->updating_dns = FALSE;
     }
 
     /* Finally, set kernel hostname */
-    if (!new_hostname)
-        name = FALLBACK_HOSTNAME4;
-    else if (!new_hostname[0]) {
-        g_warn_if_reached();
-        name = FALLBACK_HOSTNAME4;
-    } else
-        name = new_hostname;
+    nm_assert(!priv->cur_hostname || priv->cur_hostname[0]);
+    name = priv->cur_hostname ?: FALLBACK_HOSTNAME4;
 
     /* Don't set the hostname if it isn't actually changing */
     if ((old_hostname = _get_hostname(self)) && (nm_streq(name, old_hostname))) {
@@ -602,8 +622,7 @@ _set_hostname(NMPolicy *self, const char *new_hostname, const char *msg)
     }
 
     /* Keep track of the last set hostname */
-    g_free(priv->last_hostname);
-    priv->last_hostname     = g_strdup(name);
+    nm_strdup_reset(&priv->last_hostname, name);
     priv->changing_hostname = TRUE;
 
     _LOGI(LOGD_DNS, "set-hostname: set hostname to '%s' (%s)", name, msg);
@@ -2764,7 +2783,7 @@ constructed(GObject *object)
                      self);
 
     priv->dns_manager = g_object_ref(nm_dns_manager_get());
-    nm_dns_manager_set_initial_hostname(priv->dns_manager, priv->orig_hostname);
+    nm_dns_manager_set_hostname(priv->dns_manager, priv->orig_hostname, TRUE);
     priv->config_changed_id = g_signal_connect(priv->dns_manager,
                                                NM_DNS_MANAGER_CONFIG_CHANGED,
                                                G_CALLBACK(dns_config_changed),
@@ -2895,6 +2914,7 @@ dispose(GObject *object)
 
     nm_clear_g_free(&priv->orig_hostname);
     nm_clear_g_free(&priv->cur_hostname);
+    nm_clear_g_free(&priv->cur_hostname_full);
     nm_clear_g_free(&priv->last_hostname);
 
     if (priv->hostname_manager) {

src/core/tests/test-utils.c

@@ -212,6 +212,62 @@ test_hw_addr_gen_stable_eth(void)
                     "04:0D:CD:0C:9E:2C");
 }
 
+static void
+test_shorten_hostname(void)
+{
+    gboolean res;
+    char    *shortened = NULL;
+
+    res = nm_utils_shorten_hostname("name1", &shortened);
+    g_assert_cmpint(res, ==, TRUE);
+    g_assert_cmpstr(shortened, ==, NULL);
+    nm_clear_g_free(&shortened);
+
+    res = nm_utils_shorten_hostname("name1.example.com", &shortened);
+    g_assert_cmpint(res, ==, TRUE);
+    g_assert_cmpstr(shortened, ==, NULL);
+    nm_clear_g_free(&shortened);
+
+    res = nm_utils_shorten_hostname(
+        "123456789-123456789-123456789-123456789-123456789-123456789-1234",
+        &shortened);
+    g_assert_cmpint(res, ==, TRUE);
+    g_assert_cmpstr(shortened, ==, NULL);
+    nm_clear_g_free(&shortened);
+
+    res = nm_utils_shorten_hostname(
+        "123456789-123456789-123456789-123456789-123456789-123456789-12345",
+        &shortened);
+    g_assert_cmpint(res, ==, TRUE);
+    g_assert_cmpstr(shortened,
+                    ==,
+                    "123456789-123456789-123456789-123456789-123456789-123456789-1234");
+    nm_clear_g_free(&shortened);
+
+    res = nm_utils_shorten_hostname(
+        "name1.test-dhcp-this-one-here-is-a-very-very-long-domain.example.com",
+        &shortened);
+    g_assert_cmpint(res, ==, TRUE);
+    g_assert_cmpstr(shortened, ==, "name1");
+    nm_clear_g_free(&shortened);
+
+    res = nm_utils_shorten_hostname(
+        "test-dhcp-this-one-here-is-a-very-very-long-hostname-without-domainname",
+        &shortened);
+    g_assert_cmpint(res, ==, TRUE);
+    g_assert_cmpstr(shortened,
+                    ==,
+                    "test-dhcp-this-one-here-is-a-very-very-long-hostname-without-dom");
+    nm_clear_g_free(&shortened);
+
+    res = nm_utils_shorten_hostname(
+        ".test-dhcp-this-one-here-is-a-very-very-long-hostname.example.com",
+        &shortened);
+    g_assert_cmpint(res, ==, FALSE);
+    g_assert_cmpstr(shortened, ==, NULL);
+    nm_clear_g_free(&shortened);
+}
+
 /*****************************************************************************/
 
 NMTST_DEFINE();
@@ -223,6 +279,7 @@ main(int argc, char **argv)
 
     g_test_add_func("/utils/stable_privacy", test_stable_privacy);
     g_test_add_func("/utils/hw_addr_gen_stable_eth", test_hw_addr_gen_stable_eth);
+    g_test_add_func("/utils/shorten-hostname", test_shorten_hostname);
 
     return g_test_run();
 }