From a352647434f56d78919c4781422d28c95404b842 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 4 Jan 2022 18:28:46 +0100 Subject: [PATCH 1/3] core: rename related things explicitly to "static-hostname" We have at least static and transient hostnames. Let's be clear which one we are talking about. Note that also NM_SETTINGS_HOSTNAME gets renamed to NM_SETTINGS_STATIC_HOSTNAME, because it seems clearer. The only purpose of NM_SETTINGS_STATIC_HOSTNAME is to be the backing property for the "Hostname" D-Bus property for the NMDBusObject glue. So, while the new name makes more sense to me, it's now also inconsistent with it's primary use (the D-Bus property). Still... --- src/core/dhcp/nm-dhcp-client.c | 2 +- src/core/nm-hostname-manager.c | 38 +++++++++++++++++---------------- src/core/nm-hostname-manager.h | 4 ++-- src/core/nm-manager.c | 12 ++++++----- src/core/nm-policy.c | 10 +++++---- src/core/settings/nm-settings.c | 37 ++++++++++++++++++-------------- src/core/settings/nm-settings.h | 2 +- 7 files changed, 58 insertions(+), 47 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index 77f3f333ca..f7c095fb8f 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -959,7 +959,7 @@ config_init(NMDhcpClientConfig *config, const NMDhcpClientConfig *src) const char *hostname; gs_free char *hostname_tmp = NULL; - hostname = nm_hostname_manager_get_hostname(nm_hostname_manager_get()); + hostname = nm_hostname_manager_get_static_hostname(nm_hostname_manager_get()); if (nm_utils_is_specific_hostname(hostname)) { if (config->addr_family == AF_INET) { diff --git a/src/core/nm-hostname-manager.c b/src/core/nm-hostname-manager.c index 59164edee3..dfca58eefe 100644 --- a/src/core/nm-hostname-manager.c +++ b/src/core/nm-hostname-manager.c @@ -52,10 +52,10 @@ /*****************************************************************************/ -NM_GOBJECT_PROPERTIES_DEFINE(NMHostnameManager, PROP_HOSTNAME, ); +NM_GOBJECT_PROPERTIES_DEFINE(NMHostnameManager, PROP_STATIC_HOSTNAME, ); typedef struct { - char *current_hostname; + char *static_hostname; GFileMonitor *monitor; GFileMonitor *dhcp_monitor; gulong monitor_id; @@ -178,10 +178,11 @@ hostname_is_dynamic(void) /*****************************************************************************/ const char * -nm_hostname_manager_get_hostname(NMHostnameManager *self) +nm_hostname_manager_get_static_hostname(NMHostnameManager *self) { g_return_val_if_fail(NM_IS_HOSTNAME_MANAGER(self), NULL); - return NM_HOSTNAME_MANAGER_GET_PRIVATE(self)->current_hostname; + + return NM_HOSTNAME_MANAGER_GET_PRIVATE(self)->static_hostname; } static void @@ -192,18 +193,18 @@ _set_hostname(NMHostnameManager *self, const char *hostname) hostname = nm_str_not_empty(hostname); - if (nm_streq0(hostname, priv->current_hostname)) + if (nm_streq0(hostname, priv->static_hostname)) return; - _LOGI("hostname changed from %s%s%s to %s%s%s", - NM_PRINT_FMT_QUOTED(priv->current_hostname, "\"", priv->current_hostname, "\"", "(none)"), + _LOGI("static hostname changed from %s%s%s to %s%s%s", + NM_PRINT_FMT_QUOTED(priv->static_hostname, "\"", priv->static_hostname, "\"", "(none)"), NM_PRINT_FMT_QUOTED(hostname, "\"", hostname, "\"", "(none)")); - old_hostname = priv->current_hostname; - priv->current_hostname = g_strdup(hostname); + old_hostname = priv->static_hostname; + priv->static_hostname = g_strdup(hostname); g_free(old_hostname); - _notify(self, PROP_HOSTNAME); + _notify(self, PROP_STATIC_HOSTNAME); } static void @@ -499,8 +500,8 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) NMHostnameManager *self = NM_HOSTNAME_MANAGER(object); switch (prop_id) { - case PROP_HOSTNAME: - g_value_set_string(value, nm_hostname_manager_get_hostname(self)); + case PROP_STATIC_HOSTNAME: + g_value_set_string(value, nm_hostname_manager_get_static_hostname(self)); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); @@ -572,7 +573,7 @@ dispose(GObject *object) _file_monitors_clear(self); - nm_clear_g_free(&priv->current_hostname); + nm_clear_g_free(&priv->static_hostname); G_OBJECT_CLASS(nm_hostname_manager_parent_class)->dispose(object); } @@ -586,11 +587,12 @@ nm_hostname_manager_class_init(NMHostnameManagerClass *class) object_class->get_property = get_property; object_class->dispose = dispose; - obj_properties[PROP_HOSTNAME] = g_param_spec_string(NM_HOSTNAME_MANAGER_HOSTNAME, - "", - "", - NULL, - G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + obj_properties[PROP_STATIC_HOSTNAME] = + g_param_spec_string(NM_HOSTNAME_MANAGER_STATIC_HOSTNAME, + "", + "", + NULL, + G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); g_object_class_install_properties(object_class, _PROPERTY_ENUMS_LAST, obj_properties); } diff --git a/src/core/nm-hostname-manager.h b/src/core/nm-hostname-manager.h index 3a2c0cd698..b871d1779e 100644 --- a/src/core/nm-hostname-manager.h +++ b/src/core/nm-hostname-manager.h @@ -21,7 +21,7 @@ #define NM_HOSTNAME_MANAGER_GET_CLASS(obj) \ (G_TYPE_INSTANCE_GET_CLASS((obj), NM_TYPE_HOSTNAME_MANAGER, NMHostnameManagerClass)) -#define NM_HOSTNAME_MANAGER_HOSTNAME "hostname" +#define NM_HOSTNAME_MANAGER_STATIC_HOSTNAME "static-hostname" typedef struct _NMHostnameManager NMHostnameManager; typedef struct _NMHostnameManagerClass NMHostnameManagerClass; @@ -34,7 +34,7 @@ GType nm_hostname_manager_get_type(void); NMHostnameManager *nm_hostname_manager_get(void); -const char *nm_hostname_manager_get_hostname(NMHostnameManager *self); +const char *nm_hostname_manager_get_static_hostname(NMHostnameManager *self); gboolean nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname); diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index d94bdbb986..a30dbe084f 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -2270,7 +2270,7 @@ system_unmanaged_devices_changed_cb(NMSettings *settings, GParamSpec *pspec, gpo } static void -hostname_changed_cb(NMHostnameManager *hostname_manager, GParamSpec *pspec, NMManager *self) +_static_hostname_changed_cb(NMHostnameManager *hostname_manager, GParamSpec *pspec, NMManager *self) { nm_dispatcher_call_hostname(NULL, NULL, NULL); } @@ -6954,7 +6954,7 @@ nm_manager_start(NMManager *self, GError **error) system_unmanaged_devices_changed_cb(priv->settings, NULL, self); - hostname_changed_cb(priv->hostname_manager, NULL, self); + _static_hostname_changed_cb(priv->hostname_manager, NULL, self); if (!nm_settings_start(priv->settings, error)) return FALSE; @@ -7838,8 +7838,8 @@ constructed(GObject *object) priv->hostname_manager = g_object_ref(nm_hostname_manager_get()); g_signal_connect(priv->hostname_manager, - "notify::" NM_HOSTNAME_MANAGER_HOSTNAME, - G_CALLBACK(hostname_changed_cb), + "notify::" NM_HOSTNAME_MANAGER_STATIC_HOSTNAME, + G_CALLBACK(_static_hostname_changed_cb), self); /* @@ -8227,7 +8227,9 @@ dispose(GObject *object) } if (priv->hostname_manager) { - g_signal_handlers_disconnect_by_func(priv->hostname_manager, hostname_changed_cb, self); + g_signal_handlers_disconnect_by_func(priv->hostname_manager, + _static_hostname_changed_cb, + self); g_clear_object(&priv->hostname_manager); } diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index f9f2fc5d78..c4ae8e2abb 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -830,7 +830,7 @@ update_system_hostname(NMPolicy *self, const char *msg) */ /* Try a persistent hostname first */ - configured_hostname = nm_hostname_manager_get_hostname(priv->hostname_manager); + configured_hostname = nm_hostname_manager_get_static_hostname(priv->hostname_manager); if (configured_hostname && nm_utils_is_specific_hostname(configured_hostname)) { _set_hostname(self, configured_hostname, "from system configuration"); priv->dhcp_hostname = FALSE; @@ -1518,7 +1518,9 @@ process_secondaries(NMPolicy *self, NMActiveConnection *active, gboolean connect } static void -hostname_changed(NMHostnameManager *hostname_manager, GParamSpec *pspec, gpointer user_data) +_static_hostname_changed_cb(NMHostnameManager *hostname_manager, + GParamSpec *pspec, + gpointer user_data) { NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF(priv); @@ -2744,8 +2746,8 @@ constructed(GObject *object) self); g_signal_connect(priv->hostname_manager, - "notify::" NM_HOSTNAME_MANAGER_HOSTNAME, - G_CALLBACK(hostname_changed), + "notify::" NM_HOSTNAME_MANAGER_STATIC_HOSTNAME, + G_CALLBACK(_static_hostname_changed_cb), priv); g_signal_connect(priv->manager, diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c index e69f6ac1b3..da09571b0d 100644 --- a/src/core/settings/nm-settings.c +++ b/src/core/settings/nm-settings.c @@ -327,7 +327,7 @@ _sett_conn_entry_find_shadowed_storage(SettConnEntry *sett_conn_entry, NM_GOBJECT_PROPERTIES_DEFINE(NMSettings, PROP_MANAGER, PROP_UNMANAGED_SPECS, - PROP_HOSTNAME, + PROP_STATIC_HOSTNAME, PROP_CAN_MODIFY, PROP_CONNECTIONS, PROP_STARTUP_COMPLETE, ); @@ -3480,9 +3480,11 @@ err: /*****************************************************************************/ static void -_hostname_changed_cb(NMHostnameManager *hostname_manager, GParamSpec *pspec, gpointer user_data) +_static_hostname_changed_cb(NMHostnameManager *hostname_manager, + GParamSpec *pspec, + gpointer user_data) { - _notify(user_data, PROP_HOSTNAME); + _notify(user_data, PROP_STATIC_HOSTNAME); } /*****************************************************************************/ @@ -3892,11 +3894,11 @@ nm_settings_start(NMSettings *self, GError **error) _plugin_connections_reload(self); g_signal_connect(priv->hostname_manager, - "notify::" NM_HOSTNAME_MANAGER_HOSTNAME, - G_CALLBACK(_hostname_changed_cb), + "notify::" NM_HOSTNAME_MANAGER_STATIC_HOSTNAME, + G_CALLBACK(_static_hostname_changed_cb), self); - if (nm_hostname_manager_get_hostname(priv->hostname_manager)) - _notify(self, PROP_HOSTNAME); + if (nm_hostname_manager_get_static_hostname(priv->hostname_manager)) + _notify(self, PROP_STATIC_HOSTNAME); priv->started = TRUE; _startup_complete_check(self, 0); @@ -3923,10 +3925,10 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) g_value_take_boxed(value, _nm_utils_slist_to_strv(nm_settings_get_unmanaged_specs(self), TRUE)); break; - case PROP_HOSTNAME: + case PROP_STATIC_HOSTNAME: g_value_set_string(value, priv->hostname_manager - ? nm_hostname_manager_get_hostname(priv->hostname_manager) + ? nm_hostname_manager_get_static_hostname(priv->hostname_manager) : NULL); break; case PROP_CAN_MODIFY: @@ -4025,7 +4027,7 @@ dispose(GObject *object) if (priv->hostname_manager) { g_signal_handlers_disconnect_by_func(priv->hostname_manager, - G_CALLBACK(_hostname_changed_cb), + G_CALLBACK(_static_hostname_changed_cb), self); g_clear_object(&priv->hostname_manager); } @@ -4167,7 +4169,9 @@ static const NMDBusInterfaceInfoExtended interface_info_settings = { NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Connections", "ao", NM_SETTINGS_CONNECTIONS), - NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Hostname", "s", NM_SETTINGS_HOSTNAME), + NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Hostname", + "s", + NM_SETTINGS_STATIC_HOSTNAME), NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("CanModify", "b", NM_SETTINGS_CAN_MODIFY), ), ), @@ -4200,11 +4204,12 @@ nm_settings_class_init(NMSettingsClass *class) G_TYPE_STRV, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); - obj_properties[PROP_HOSTNAME] = g_param_spec_string(NM_SETTINGS_HOSTNAME, - "", - "", - NULL, - G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + obj_properties[PROP_STATIC_HOSTNAME] = + g_param_spec_string(NM_SETTINGS_STATIC_HOSTNAME, + "", + "", + NULL, + G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); obj_properties[PROP_CAN_MODIFY] = g_param_spec_boolean(NM_SETTINGS_CAN_MODIFY, diff --git a/src/core/settings/nm-settings.h b/src/core/settings/nm-settings.h index 13e9441adf..56cebed037 100644 --- a/src/core/settings/nm-settings.h +++ b/src/core/settings/nm-settings.h @@ -24,7 +24,7 @@ (G_TYPE_INSTANCE_GET_CLASS((obj), NM_TYPE_SETTINGS, NMSettingsClass)) #define NM_SETTINGS_UNMANAGED_SPECS "unmanaged-specs" -#define NM_SETTINGS_HOSTNAME "hostname" +#define NM_SETTINGS_STATIC_HOSTNAME "static-hostname" #define NM_SETTINGS_CAN_MODIFY "can-modify" #define NM_SETTINGS_CONNECTIONS "connections" #define NM_SETTINGS_STARTUP_COMPLETE "startup-complete" From bbff0c9853e3db735bb55ac9c1036db55c1f525d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 4 Jan 2022 19:00:34 +0100 Subject: [PATCH 2/3] core: ensure static-hostname is valid UTF-8 We get the hostname via D-Bus (from hostnamed) or read it from file. In the latter case, it is not ensured that it's valid UTF-8. Non-UTF-8 "strings" are bad, because we might try to expose them on D-Bus, log them or other bad things. Sanitize the string by using backslash escaping. Maybe we should outright reject such binary nonsense, but it's not done here, for no strong reasons. --- src/core/nm-hostname-manager.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/core/nm-hostname-manager.c b/src/core/nm-hostname-manager.c index dfca58eefe..64c2531e61 100644 --- a/src/core/nm-hostname-manager.c +++ b/src/core/nm-hostname-manager.c @@ -188,11 +188,24 @@ nm_hostname_manager_get_static_hostname(NMHostnameManager *self) static void _set_hostname(NMHostnameManager *self, const char *hostname) { - NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); + gs_free char *hostname_free = NULL; char *old_hostname; hostname = nm_str_not_empty(hostname); + if (hostname) { + /* as we also read the file from disk, it might not be in UTF-8 encoding. + * + * A hostname in non-UTF-8 encoding would be odd and cause issues when we + * try to expose them on D-Bus via the NM_SETTINGS_STATIC_HOSTNAME property. + * + * Sanitize somewhat. It's wrong anyway. */ + hostname = nm_utils_str_utf8safe_escape(hostname, + NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL, + &hostname_free); + } + if (nm_streq0(hostname, priv->static_hostname)) return; From 20eb6df2158490bed4efc53d14cf72eabf015051 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 4 Jan 2022 19:05:21 +0100 Subject: [PATCH 3/3] core: simplify code in nm_dns_manager_set_hostname() --- src/core/dns/nm-dns-manager.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index 4e7a4a5424..8a8ab9eb06 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -2001,22 +2001,18 @@ 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) { - NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE(self); - const char *filtered = NULL; + NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE(self); /* 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, '.')) { - filtered = hostname; - } + /* pass */ + } else + hostname = NULL; - if ((!priv->hostname && !filtered) - || (priv->hostname && filtered && !strcmp(priv->hostname, filtered))) + if (!nm_strdup_reset(&priv->hostname, hostname)) return; - g_free(priv->hostname); - priv->hostname = g_strdup(filtered); - if (skip_update) return;