policy: fall back to original hostname before trying reverse DNS

Having the original hostname checks in the settings-service code
didn't allow the policy enough granularity to differentiate between
a plugin-provided hostname and the original hostname.  We want to
fall back to the original hostname if there isn't a persistent
hostname (from a plugin) and if there isn't a DHCP-provided
hostname.  Moving the original hostname checks to the policy
makes that possible.  Clarify the precedence order at the same
time, and minimally validate the DHCP hostname as well.

2 files changed, 46 insertions, 33 deletions

diff --git a/src/NetworkManagerPolicy.c b/src/NetworkManagerPolicy.c
index 5c5ae4027b..548ad01079 100644
--- a/src/NetworkManagerPolicy.c
+++ b/src/NetworkManagerPolicy.c
@@ -74,6 +74,8 @@ struct NMPolicy {
 	NMDevice *default_device;
 
 	LookupThread *lookup;
+
+	char *orig_hostname; /* hostname at NM start time */
 };
 
 static gboolean
@@ -416,15 +418,16 @@ update_system_hostname (NMPolicy *policy, NMDevice *best)
 		policy->lookup = NULL;
 	}
 
-	/* A configured hostname (via the system-settings service) overrides
-	 * all automatic hostname determination.  If there is no configured hostname,
-	 * the best device's automatically determined hostname (from DHCP, VPN, PPP,
-	 * etc) is used.  If there is no automatically determined hostname, reverse
-	 * DNS lookup using the best device's IP address is started to determined the
-	 * the hostname.
+	/* Hostname precedence order:
+	 *
+	 * 1) a configured hostname (from system-settings)
+	 * 2) automatic hostname from the default device's config (DHCP, VPN, etc)
+	 * 3) the original hostname when NM started
+	 * 4) reverse-DNS of the best device's IPv4 address
+	 *
 	 */
 
-	/* Try a configured hostname first */
+	/* Try a persistent hostname first */
 	g_object_get (G_OBJECT (policy->manager), NM_MANAGER_HOSTNAME, &configured_hostname, NULL);
 	if (configured_hostname) {
 		set_system_hostname (configured_hostname, "from system configuration");
@@ -437,25 +440,42 @@ update_system_hostname (NMPolicy *policy, NMDevice *best)
 		best = get_best_device (policy->manager, &best_req);
 
 	if (!best) {
-		/* No best device; fall back to localhost.localdomain */
-		set_system_hostname (NULL, "no default device");
+		/* No best device; fall back to original hostname or if there wasn't
+		 * one, 'localhost.localdomain'
+		 */
+		set_system_hostname (policy->orig_hostname, "no default device");
 		return;
 	}
 
 	/* Grab a hostname out of the device's DHCP4 config */
 	dhcp4_config = nm_device_get_dhcp4_config (best);
 	if (dhcp4_config) {
-		const char *dhcp4_hostname;
+		const char *dhcp4_hostname, *p;
 
-		dhcp4_hostname = nm_dhcp4_config_get_option (dhcp4_config, "host_name");
+		p = dhcp4_hostname = nm_dhcp4_config_get_option (dhcp4_config, "host_name");
 		if (dhcp4_hostname && strlen (dhcp4_hostname)) {
-			set_system_hostname (dhcp4_hostname, "from DHCP");
-			return;
+			/* Sanity check */
+			while (*p) {
+				if (!isblank (*p++)) {
+					set_system_hostname (dhcp4_hostname, "from DHCP");
+					return;
+				}
+			}
+			nm_warning ("%s: DHCP-provided hostname '%s' looks invalid; not using it",
+			            __func__, dhcp4_hostname);
 		}
 	}
 
-	/* No configured hostname, no automatically determined hostname either. Start
-	 * reverse DNS of the current IP address to try and find it.
+	/* If no automatically-configured hostname, try using the hostname from
+	 * when NM started up.
+	 */
+	if (policy->orig_hostname) {
+		set_system_hostname (policy->orig_hostname, "from system startup");
+		return;
+	}
+
+	/* No configured hostname, no automatically determined hostname, and
+	 * no bootup hostname. Start reverse DNS of the current IP address.
 	 */
 	ip4_config = nm_device_get_ip4_config (best);
 	if (   !ip4_config
@@ -988,6 +1008,7 @@ nm_policy_new (NMManager *manager, NMVPNManager *vpn_manager)
 	NMPolicy *policy;
 	static gboolean initialized = FALSE;
 	gulong id;
+	char hostname[HOST_NAME_MAX + 2];
 
 	g_return_val_if_fail (NM_IS_MANAGER (manager), NULL);
 	g_return_val_if_fail (initialized == FALSE, NULL);
@@ -996,6 +1017,14 @@ nm_policy_new (NMManager *manager, NMVPNManager *vpn_manager)
 	policy->manager = g_object_ref (manager);
 	policy->update_state_id = 0;
 
+	/* Grab hostname on startup and use that if nothing provides one */
+	memset (hostname, 0, sizeof (hostname));
+	if (gethostname (&hostname[0], HOST_NAME_MAX) == 0) {
+		/* only cache it if it's a valid hostname */
+		if (strlen (hostname) && strcmp (hostname, "localhost") && strcmp (hostname, "localhost.localdomain"))
+			policy->orig_hostname = g_strdup (hostname);
+	}
+
 	policy->vpn_manager = g_object_ref (vpn_manager);
 	id = g_signal_connect (policy->vpn_manager, "connection-activated",
 	                       G_CALLBACK (vpn_connection_activated), policy);
@@ -1086,6 +1115,8 @@ nm_policy_destroy (NMPolicy *policy)
 	}
 	g_slist_free (policy->dev_signal_ids);
 
+	g_free (policy->orig_hostname);
+
 	g_object_unref (policy->manager);
 	g_free (policy);
 }
diff --git a/src/system-settings/nm-sysconfig-settings.c b/src/system-settings/nm-sysconfig-settings.c
index 7580319db0..0498fe7160 100644
--- a/src/system-settings/nm-sysconfig-settings.c
+++ b/src/system-settings/nm-sysconfig-settings.c
@@ -90,7 +90,6 @@ typedef struct {
 	gboolean connections_loaded;
 	GHashTable *connections;
 	GSList *unmanaged_specs;
-	char *orig_hostname;
 } NMSysconfigSettingsPrivate;
 
 static void settings_system_interface_init (NMSettingsSystemInterface *klass);
@@ -254,7 +253,6 @@ nm_sysconfig_settings_get_hostname (NMSysconfigSettings *self)
 	NMSysconfigSettingsPrivate *priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
 	GSList *iter;
 	char *hostname = NULL;
-	gboolean have_hostname_providers = FALSE;
 
 	/* Hostname returned is the hostname returned from the first plugin
 	 * that provides one.
@@ -264,8 +262,6 @@ nm_sysconfig_settings_get_hostname (NMSysconfigSettings *self)
 
 		g_object_get (G_OBJECT (iter->data), NM_SYSTEM_CONFIG_INTERFACE_CAPABILITIES, &caps, NULL);
 		if (caps & NM_SYSTEM_CONFIG_INTERFACE_CAP_MODIFY_HOSTNAME) {
-			have_hostname_providers = TRUE;
-
 			g_object_get (G_OBJECT (iter->data), NM_SYSTEM_CONFIG_INTERFACE_HOSTNAME, &hostname, NULL);
 			if (hostname && strlen (hostname))
 				return hostname;
@@ -273,10 +269,6 @@ nm_sysconfig_settings_get_hostname (NMSysconfigSettings *self)
 		}
 	}
 
-	/* If no plugin provided a hostname, try the original hostname of the machine */
-	if (!have_hostname_providers && priv->orig_hostname)
-		hostname = g_strdup (priv->orig_hostname);
-
 	return hostname;
 }
 
@@ -1378,7 +1370,6 @@ finalize (GObject *object)
 	g_slist_foreach (priv->plugins, (GFunc) g_object_unref, NULL);
 	g_slist_free (priv->plugins);
 
-	g_free (priv->orig_hostname);
 	g_free (priv->config_file);
 
 	G_OBJECT_CLASS (nm_sysconfig_settings_parent_class)->finalize (object);
@@ -1476,7 +1467,6 @@ static void
 nm_sysconfig_settings_init (NMSysconfigSettings *self)
 {
 	NMSysconfigSettingsPrivate *priv = NM_SYSCONFIG_SETTINGS_GET_PRIVATE (self);
-	char hostname[HOST_NAME_MAX + 2];
 
 	priv->connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, g_object_unref, NULL);
 
@@ -1488,13 +1478,5 @@ nm_sysconfig_settings_init (NMSysconfigSettings *self)
 		                                          self);
 	} else
 		g_warning ("%s: failed to create PolicyKit authority.", __func__);
-
-	/* Grab hostname on startup and use that if no plugins provide one */
-	memset (hostname, 0, sizeof (hostname));
-	if (gethostname (&hostname[0], HOST_NAME_MAX) == 0) {
-		/* only cache it if it's a valid hostname */
-		if (strlen (hostname) && strcmp (hostname, "localhost") && strcmp (hostname, "localhost.localdomain"))
-			priv->orig_hostname = g_strdup (hostname);
-	}
 }