From 8b2d115f9d2a842555e91f3007f9ca0fc99623bc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 20 May 2019 16:38:37 +0200 Subject: [PATCH 01/19] shared: add nm_utils_g_slist_find_str() util --- shared/nm-glib-aux/nm-shared-utils.c | 28 ++++++++++++++++++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 5 +++++ 2 files changed, 33 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index dcca78cf93..6ed86f5916 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -2683,6 +2683,34 @@ _nm_utils_strv_cmp_n (const char *const*strv1, /*****************************************************************************/ +/** + * nm_utils_g_slist_find_str: + * @list: the #GSList with NUL terminated strings to search + * @needle: the needle string to look for. + * + * Search the list for @needle and return the first found match + * (or %NULL if not found). Uses strcmp() for finding the first matching + * element. + * + * Returns: the #GSList element with @needle as string value or + * %NULL if not found. + */ +GSList * +nm_utils_g_slist_find_str (const GSList *list, + const char *needle) +{ + nm_assert (needle); + + for (; list; list = list->next) { + nm_assert (list->data); + if (nm_streq (list->data, needle)) + return (GSList *) list; + } + return NULL; +} + +/*****************************************************************************/ + gpointer _nm_utils_user_data_pack (int nargs, gconstpointer *args) { diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 915e1381da..8f11a9b0c3 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -978,6 +978,11 @@ nm_utils_strv_make_deep_copied_nonnull (const char **strv) /*****************************************************************************/ +GSList *nm_utils_g_slist_find_str (const GSList *list, + const char *needle); + +/*****************************************************************************/ + gssize nm_utils_ptrarray_find_binary_search (gconstpointer *list, gsize len, gconstpointer needle, From 5b721ba90d33a6e3113825a31cfcfffba77ddf3f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 May 2019 14:38:54 +0200 Subject: [PATCH 02/19] shared: add nm_c_list_elem_find_first() and minor cleanups of NMCListElem API --- shared/nm-glib-aux/nm-c-list.h | 52 ++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/shared/nm-glib-aux/nm-c-list.h b/shared/nm-glib-aux/nm-c-list.h index 4ea95f74ea..d835bbc1d4 100644 --- a/shared/nm-glib-aux/nm-c-list.h +++ b/shared/nm-glib-aux/nm-c-list.h @@ -32,6 +32,8 @@ _what && c_list_contains (list, &_what->member); \ }) +/*****************************************************************************/ + typedef struct { CList lst; void *data; @@ -47,21 +49,22 @@ nm_c_list_elem_new_stale (void *data) return elem; } -static inline void * -nm_c_list_elem_get (CList *lst) +static inline gboolean +nm_c_list_elem_free_full (NMCListElem *elem, GDestroyNotify free_fcn) { - if (!lst) - return NULL; - return c_list_entry (lst, NMCListElem, lst)->data; + if (!elem) + return FALSE; + c_list_unlink_stale (&elem->lst); + if (free_fcn) + free_fcn (elem->data); + g_slice_free (NMCListElem, elem); + return TRUE; } -static inline void +static inline gboolean nm_c_list_elem_free (NMCListElem *elem) { - if (elem) { - c_list_unlink_stale (&elem->lst); - g_slice_free (NMCListElem, elem); - } + return nm_c_list_elem_free_full (elem, NULL); } static inline void @@ -69,12 +72,31 @@ nm_c_list_elem_free_all (CList *head, GDestroyNotify free_fcn) { NMCListElem *elem; - while ((elem = c_list_first_entry (head, NMCListElem, lst))) { - if (free_fcn) - free_fcn (elem->data); - c_list_unlink_stale (&elem->lst); - g_slice_free (NMCListElem, elem); + while ((elem = c_list_first_entry (head, NMCListElem, lst))) + nm_c_list_elem_free_full (elem, free_fcn); +} + +/** + * nm_c_list_elem_find_first: + * @head: the @CList head of a list containing #NMCListElem elements. + * Note that the head is not itself part of the list. + * @needle: the needle pointer. + * + * Iterates the list and returns the first #NMCListElem with the matching @needle, + * using pointer equality. + * + * Returns: the found list element or %NULL if not found. + */ +static inline NMCListElem * +nm_c_list_elem_find_first (CList *head, gconstpointer needle) +{ + NMCListElem *elem; + + c_list_for_each_entry (elem, head, lst) { + if (elem->data == needle) + return elem; } + return NULL; } /*****************************************************************************/ From b1f5e971f3c81a56c781004a43efdd77f635b696 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 12 Jun 2019 10:13:54 +0200 Subject: [PATCH 03/19] shared: add nm_g_variant_ref_sink() util --- shared/nm-glib-aux/nm-macros-internal.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/shared/nm-glib-aux/nm-macros-internal.h b/shared/nm-glib-aux/nm-macros-internal.h index 42c11d63ec..5b594a2ab5 100644 --- a/shared/nm-glib-aux/nm-macros-internal.h +++ b/shared/nm-glib-aux/nm-macros-internal.h @@ -1249,6 +1249,14 @@ nm_g_variant_ref (GVariant *v) return v; } +static inline GVariant * +nm_g_variant_ref_sink (GVariant *v) +{ + if (v) + g_variant_ref_sink (v); + return v; +} + static inline void nm_g_variant_unref (GVariant *v) { From c165c6a671f03a2e48549f247435d953710ce7dc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 21 May 2019 18:13:53 +0200 Subject: [PATCH 04/19] libnm: don't assert against %NULL string in nm_utils_is_uuid() For a "is" check, it's inconvenient to assert against the parameter being %NULL. We should accept %NULL and just say that it's not a valid uuid. This relaxes previous API. --- libnm-core/nm-utils.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 5322a8d747..dc0b5b814e 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -4713,11 +4713,14 @@ nm_utils_iface_valid_name (const char *name) /** * nm_utils_is_uuid: - * @str: a string that might be a UUID + * @str: (allow-none): a string that might be a UUID * * Checks if @str is a UUID * * Returns: %TRUE if @str is a UUID, %FALSE if not + * + * In older versions, nm_utils_is_uuid() did not accept %NULL as @str + * argument. Don't pass %NULL if you run against older versions of libnm. */ gboolean nm_utils_is_uuid (const char *str) @@ -4725,7 +4728,8 @@ nm_utils_is_uuid (const char *str) const char *p = str; int num_dashes = 0; - g_return_val_if_fail (str, FALSE); + if (!p) + return FALSE; while (*p) { if (*p == '-') From 6da5ad29623b58ff6d77202c86e7fc11842b35f7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 27 May 2019 16:16:41 +0200 Subject: [PATCH 05/19] libnm: cleanup GSList/GPtrArray to/from strv conversion --- libnm-core/nm-core-internal.h | 4 +-- libnm-core/nm-utils.c | 54 +++++++++++++++++++++-------------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index a5114ba8d8..406b170e15 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -276,10 +276,10 @@ GPtrArray *_nm_utils_copy_object_array (const GPtrArray *array); gssize _nm_utils_ptrarray_find_first (gconstpointer *list, gssize len, gconstpointer needle); GSList * _nm_utils_strv_to_slist (char **strv, gboolean deep_copy); -char ** _nm_utils_slist_to_strv (GSList *slist, gboolean deep_copy); +char ** _nm_utils_slist_to_strv (const GSList *slist, gboolean deep_copy); GPtrArray * _nm_utils_strv_to_ptrarray (char **strv); -char ** _nm_utils_ptrarray_to_strv (GPtrArray *ptrarray); +char ** _nm_utils_ptrarray_to_strv (const GPtrArray *ptrarray); gboolean _nm_utils_check_file (const char *filename, gint64 check_owner, diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index dc0b5b814e..31ef2978a1 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -955,43 +955,51 @@ _nm_utils_bytes_from_dbus (GVariant *dbus_value, g_value_take_boxed (prop_value, bytes); } +/*****************************************************************************/ + GSList * _nm_utils_strv_to_slist (char **strv, gboolean deep_copy) { - int i; GSList *list = NULL; + gsize i; - if (strv) { - if (deep_copy) { - for (i = 0; strv[i]; i++) - list = g_slist_prepend (list, g_strdup (strv[i])); - } else { - for (i = 0; strv[i]; i++) - list = g_slist_prepend (list, strv[i]); - } + if (!strv) + return NULL; + + if (deep_copy) { + for (i = 0; strv[i]; i++) + list = g_slist_prepend (list, g_strdup (strv[i])); + } else { + for (i = 0; strv[i]; i++) + list = g_slist_prepend (list, strv[i]); } - return g_slist_reverse (list); } char ** -_nm_utils_slist_to_strv (GSList *slist, gboolean deep_copy) +_nm_utils_slist_to_strv (const GSList *slist, gboolean deep_copy) { - GSList *iter; + const GSList *iter; char **strv; - int len, i; + guint len, i; - len = g_slist_length (slist); - if (!len) + if (!slist) return NULL; + + len = g_slist_length ((GSList *) slist); + strv = g_new (char *, len + 1); if (deep_copy) { - for (i = 0, iter = slist; iter; iter = iter->next, i++) + for (i = 0, iter = slist; iter; iter = iter->next, i++) { + nm_assert (iter->data); strv[i] = g_strdup (iter->data); + } } else { - for (i = 0, iter = slist; iter; iter = iter->next, i++) + for (i = 0, iter = slist; iter; iter = iter->next, i++) { + nm_assert (iter->data); strv[i] = iter->data; + } } strv[i] = NULL; @@ -1002,9 +1010,11 @@ GPtrArray * _nm_utils_strv_to_ptrarray (char **strv) { GPtrArray *ptrarray; - int i; + gsize i, l; - ptrarray = g_ptr_array_new_with_free_func (g_free); + l = NM_PTRARRAY_LEN (strv); + + ptrarray = g_ptr_array_new_full (l, g_free); if (strv) { for (i = 0; strv[i]; i++) @@ -1015,10 +1025,10 @@ _nm_utils_strv_to_ptrarray (char **strv) } char ** -_nm_utils_ptrarray_to_strv (GPtrArray *ptrarray) +_nm_utils_ptrarray_to_strv (const GPtrArray *ptrarray) { char **strv; - int i; + guint i; if (!ptrarray) return g_new0 (char *, 1); @@ -1032,6 +1042,8 @@ _nm_utils_ptrarray_to_strv (GPtrArray *ptrarray) return strv; } +/*****************************************************************************/ + static gboolean device_supports_ap_ciphers (guint32 dev_caps, guint32 ap_flags, From 18acdeeba5ab7a6679ac1b95c227e133fc608eec Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 20 May 2019 15:28:35 +0200 Subject: [PATCH 06/19] settings: don't remember path of setting plugin It was only kept to compare whether we loaded the same plugin multiple times. Note that load_plugins() already checks for duplicate plugin names, so it actually could not happen that we tried to load the same file more than once. --- src/settings/nm-settings.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 723732df1e..ab7b7b1aef 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -86,7 +86,6 @@ EXPORT(nm_settings_connection_update) /*****************************************************************************/ -static NM_CACHED_QUARK_FCN ("plugin-module-path", plugin_module_path_quark) static NM_CACHED_QUARK_FCN ("default-wired-connection", _default_wired_connection_quark) static NM_CACHED_QUARK_FCN ("default-wired-device", _default_wired_device_quark) @@ -620,26 +619,17 @@ add_plugin (NMSettings *self, NMSettingsPlugin *plugin, const char *path) static gboolean add_plugin_load_file (NMSettings *self, const char *pname, GError **error) { - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); gs_free char *full_name = NULL; gs_free char *path = NULL; gs_unref_object NMSettingsPlugin *plugin = NULL; GModule *module; NMSettingsPluginFactoryFunc factory_func; - GSList *iter; struct stat st; int errsv; full_name = g_strdup_printf ("nm-settings-plugin-%s", pname); path = g_module_build_path (NMPLUGINDIR, full_name); - for (iter = priv->plugins; iter; iter = iter->next) { - if (nm_streq0 (path, - g_object_get_qdata (iter->data, - plugin_module_path_quark ()))) - return TRUE; - } - if (stat (path, &st) != 0) { errsv = errno; _LOGW ("could not load plugin '%s' from file '%s': %s", pname, path, nm_strerror_native (errsv)); @@ -688,10 +678,6 @@ add_plugin_load_file (NMSettings *self, const char *pname, GError **error) } add_plugin (self, NM_SETTINGS_PLUGIN (plugin), path); - g_object_set_qdata_full (G_OBJECT (plugin), - plugin_module_path_quark (), - g_steal_pointer (&path), - g_free); return TRUE; } From 50be2f5244886954c3addb24eb5bd6d0e67ed236 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 20 May 2019 15:30:18 +0200 Subject: [PATCH 07/19] settings: log settings plugin name Instead of [1558284380.2045] settings: Loaded settings plugin: SettingsPluginIfcfg ("/usr/lib64/NetworkManager/1.19.2/libnm-settings-plugin-ifcfg-rh.so") log [1558284380.2045] settings: Loaded settings plugin: ifcfg-rh ("/usr/lib64/NetworkManager/1.19.2/libnm-settings-plugin-ifcfg-rh.so") Note how `man NetworkManager.conf` documents "main.plugins" configuration option where settings plugins have names like "keyfile" and "ifcfg-rh". It's not helpful to log the GObject type name, which is an implementation detail. --- src/settings/nm-settings.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index ab7b7b1aef..817cbc1374 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -596,7 +596,10 @@ unrecognized_specs_changed (NMSettingsPlugin *config, } static void -add_plugin (NMSettings *self, NMSettingsPlugin *plugin, const char *path) +add_plugin (NMSettings *self, + NMSettingsPlugin *plugin, + const char *pname, + const char *path) { NMSettingsPrivate *priv; @@ -612,7 +615,7 @@ add_plugin (NMSettings *self, NMSettingsPlugin *plugin, const char *path) nm_settings_plugin_initialize (plugin); _LOGI ("Loaded settings plugin: %s (%s%s%s)", - G_OBJECT_TYPE_NAME (plugin), + pname, NM_PRINT_FMT_QUOTED (path, "\"", path, "\"", "internal")); } @@ -677,7 +680,7 @@ add_plugin_load_file (NMSettings *self, const char *pname, GError **error) return FALSE; } - add_plugin (self, NM_SETTINGS_PLUGIN (plugin), path); + add_plugin (self, NM_SETTINGS_PLUGIN (plugin), pname, path); return TRUE; } @@ -687,7 +690,7 @@ add_plugin_keyfile (NMSettings *self) gs_unref_object NMSKeyfilePlugin *keyfile_plugin = NULL; keyfile_plugin = nms_keyfile_plugin_new (); - add_plugin (self, NM_SETTINGS_PLUGIN (keyfile_plugin), NULL); + add_plugin (self, NM_SETTINGS_PLUGIN (keyfile_plugin), "keyfile", NULL); } static gboolean From d7056d13d0f63187ae1081bbe356d23e8f2585f2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 20 May 2019 15:51:02 +0200 Subject: [PATCH 08/19] settings: drop nm_settings_plugin_initialize() and initialize on demand As nm_settings_plugin_initialize() could not fail (it returned no value indicating failure), there is no reason to explicitly call this. Instead just initialize the plugin when needed. Also, we don't need the plugin to initialize early before nm_settings_plugin_get_connections(). --- src/settings/nm-settings-plugin.c | 9 -------- src/settings/nm-settings-plugin.h | 5 ----- src/settings/nm-settings.c | 3 +-- .../plugins/ifupdown/nms-ifupdown-plugin.c | 21 +++++++++++++++---- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/settings/nm-settings-plugin.c b/src/settings/nm-settings-plugin.c index 1d5086f52b..81a168b24d 100644 --- a/src/settings/nm-settings-plugin.c +++ b/src/settings/nm-settings-plugin.c @@ -40,15 +40,6 @@ G_DEFINE_TYPE (NMSettingsPlugin, nm_settings_plugin, G_TYPE_OBJECT) /*****************************************************************************/ -void -nm_settings_plugin_initialize (NMSettingsPlugin *self) -{ - g_return_if_fail (NM_IS_SETTINGS_PLUGIN (self)); - - if (NM_SETTINGS_PLUGIN_GET_CLASS (self)->initialize) - NM_SETTINGS_PLUGIN_GET_CLASS (self)->initialize (self); -} - GSList * nm_settings_plugin_get_connections (NMSettingsPlugin *self) { diff --git a/src/settings/nm-settings-plugin.h b/src/settings/nm-settings-plugin.h index 46dea3d700..11b859978a 100644 --- a/src/settings/nm-settings-plugin.h +++ b/src/settings/nm-settings-plugin.h @@ -41,9 +41,6 @@ typedef struct { typedef struct { GObjectClass parent; - /* Called when the plugin is loaded to initialize it */ - void (*initialize) (NMSettingsPlugin *plugin); - /* Returns a GSList of NMSettingsConnection objects that represent * connections the plugin knows about. The returned list is freed by the * system settings service. @@ -104,8 +101,6 @@ typedef NMSettingsPlugin *(*NMSettingsPluginFactoryFunc) (void); /* Plugin's factory function that returns a #NMSettingsPlugin */ NMSettingsPlugin *nm_settings_plugin_factory (void); -void nm_settings_plugin_initialize (NMSettingsPlugin *config); - GSList *nm_settings_plugin_get_connections (NMSettingsPlugin *plugin); gboolean nm_settings_plugin_load_connection (NMSettingsPlugin *plugin, diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 817cbc1374..0e7dfcd337 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -612,8 +612,6 @@ add_plugin (NMSettings *self, priv->plugins = g_slist_append (priv->plugins, g_object_ref (plugin)); - nm_settings_plugin_initialize (plugin); - _LOGI ("Loaded settings plugin: %s (%s%s%s)", pname, NM_PRINT_FMT_QUOTED (path, "\"", path, "\"", "internal")); @@ -1923,6 +1921,7 @@ nm_settings_start (NMSettings *self, GError **error) return FALSE; load_connections (self); + check_startup_complete (self); priv->hostname_manager = g_object_ref (nm_hostname_manager_get ()); diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 4710edfb17..8f9be772bb 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -55,7 +55,9 @@ typedef struct { */ GHashTable *eni_ifaces; - bool ifupdown_managed; + bool ifupdown_managed:1; + + bool initialized:1; } SettingsPluginIfupdownPrivate; struct _SettingsPluginIfupdown { @@ -88,6 +90,10 @@ NM_DEFINE_SINGLETON_GETTER (SettingsPluginIfupdown, settings_plugin_ifupdown_get /*****************************************************************************/ +static void initialize (SettingsPluginIfupdown *self); + +/*****************************************************************************/ + /* Returns the plugins currently known list of connections. The returned * list is freed by the system settings service. */ @@ -100,6 +106,9 @@ get_connections (NMSettingsPlugin *plugin) GHashTableIter iter; void *value; + if (G_UNLIKELY (!priv->initialized)) + initialize (self); + if (!priv->ifupdown_managed) { _LOGD ("get_connections: not connections due to managed=false"); return NULL; @@ -129,6 +138,9 @@ get_unmanaged_specs (NMSettingsPlugin *plugin) GHashTableIter iter; const char *iface; + if (G_UNLIKELY (!priv->initialized)) + initialize (self); + if (priv->ifupdown_managed) return NULL; @@ -144,9 +156,8 @@ get_unmanaged_specs (NMSettingsPlugin *plugin) /*****************************************************************************/ static void -initialize (NMSettingsPlugin *plugin) +initialize (SettingsPluginIfupdown *self) { - SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); gs_unref_hashtable GHashTable *auto_ifaces = NULL; nm_auto_ifparser if_parser *parser = NULL; @@ -155,6 +166,9 @@ initialize (NMSettingsPlugin *plugin) const char *block_name; NMIfupdownConnection *conn; + nm_assert (!priv->initialized); + priv->initialized = TRUE; + parser = ifparser_parse (ENI_INTERFACES_FILE, 0); c_list_for_each_entry (block, &parser->block_lst_head, block_lst) { @@ -316,7 +330,6 @@ settings_plugin_ifupdown_class_init (SettingsPluginIfupdownClass *klass) object_class->dispose = dispose; - plugin_class->initialize = initialize; plugin_class->get_connections = get_connections; plugin_class->get_unmanaged_specs = get_unmanaged_specs; } From ca1fe95ce0aac9b19a7e6c16fcf52b4046370eb0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 20 May 2019 17:08:35 +0200 Subject: [PATCH 09/19] settings: use nm_utils_g_slist_find_str() in update_specs() NMSettings is complicated enough. We should try to move independent code out of it, so that there is only logic that is essential there. While at it, rework how we copy the GSList items. I don't like GSList as a data structure, but there really is no need to allocate a new list. Just unlink the list element and prepend it in the other list. --- src/settings/nm-settings.c | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 0e7dfcd337..b0f37f62f3 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -535,18 +535,6 @@ nm_settings_get_unmanaged_specs (NMSettings *self) return priv->unmanaged_specs; } -static gboolean -find_spec (GSList *spec_list, const char *spec) -{ - GSList *iter; - - for (iter = spec_list; iter; iter = g_slist_next (iter)) { - if (!strcmp ((const char *) iter->data, spec)) - return TRUE; - } - return FALSE; -} - static void update_specs (NMSettings *self, GSList **specs_ptr, GSList * (*get_specs_func) (NMSettingsPlugin *)) @@ -554,21 +542,24 @@ update_specs (NMSettings *self, GSList **specs_ptr, NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); GSList *iter; - g_slist_free_full (*specs_ptr, g_free); - *specs_ptr = NULL; + g_slist_free_full (g_steal_pointer (specs_ptr), g_free); for (iter = priv->plugins; iter; iter = g_slist_next (iter)) { - GSList *specs, *specs_iter; + GSList *specs; - specs = get_specs_func (NM_SETTINGS_PLUGIN (iter->data)); - for (specs_iter = specs; specs_iter; specs_iter = specs_iter->next) { - if (!find_spec (*specs_ptr, (const char *) specs_iter->data)) { - *specs_ptr = g_slist_prepend (*specs_ptr, specs_iter->data); - } else - g_free (specs_iter->data); + specs = get_specs_func (iter->data); + while (specs) { + GSList *s = specs; + + specs = g_slist_remove_link (specs, s); + if (nm_utils_g_slist_find_str (*specs_ptr, s->data)) { + g_free (s->data); + g_slist_free_1 (s); + continue; + } + s->next = *specs_ptr; + *specs_ptr = s; } - - g_slist_free (specs); } } From 179134bbdce4ba0a92ea9f6573f47f42dca6b231 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 21 May 2019 09:48:00 +0200 Subject: [PATCH 10/19] settings/trivial: move code around "nm-settings.c" has more than 2000 LOC. Code that is related should be grouped better so that it's easier to understand how it belongs together. --- src/settings/nm-settings.c | 1056 ++++++++++++++++++------------------ 1 file changed, 535 insertions(+), 521 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index b0f37f62f3..f1d0ee9495 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -169,9 +169,6 @@ static const GDBusSignalInfo signal_info_connection_removed; static void claim_connection (NMSettings *self, NMSettingsConnection *connection); -static void unmanaged_specs_changed (NMSettingsPlugin *config, gpointer user_data); -static void unrecognized_specs_changed (NMSettingsPlugin *config, gpointer user_data); - static void connection_ready_changed (NMSettingsConnection *conn, GParamSpec *pspec, gpointer user_data); @@ -181,6 +178,9 @@ static void default_wired_clear_tag (NMSettings *self, NMSettingsConnection *connection, gboolean add_to_no_auto_default); +static void openconnect_migrate_hack (NMConnection *connection); +static void _clear_connections_cached_list (NMSettingsPrivate *priv); + /*****************************************************************************/ static void @@ -220,312 +220,20 @@ connection_ready_changed (NMSettingsConnection *conn, check_startup_complete (self); } -static void -plugin_connection_added (NMSettingsPlugin *config, - NMSettingsConnection *connection, - NMSettings *self) -{ - claim_connection (self, connection); -} - -static void -load_connections (NMSettings *self) +const char * +nm_settings_get_startup_complete_blocked_reason (NMSettings *self) { NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - GSList *iter; + const char *uuid = NULL; - for (iter = priv->plugins; iter; iter = g_slist_next (iter)) { - NMSettingsPlugin *plugin = NM_SETTINGS_PLUGIN (iter->data); - GSList *plugin_connections; - GSList *elt; - - plugin_connections = nm_settings_plugin_get_connections (plugin); - - // FIXME: ensure connections from plugins loaded with a lower priority - // get rejected when they conflict with connections from a higher - // priority plugin. - - for (elt = plugin_connections; elt; elt = g_slist_next (elt)) - claim_connection (self, elt->data); - - g_slist_free (plugin_connections); - - g_signal_connect (plugin, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, - G_CALLBACK (plugin_connection_added), self); - g_signal_connect (plugin, NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED, - G_CALLBACK (unmanaged_specs_changed), self); - g_signal_connect (plugin, NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED, - G_CALLBACK (unrecognized_specs_changed), self); - } - - priv->connections_loaded = TRUE; - _notify (self, PROP_CONNECTIONS); - - unmanaged_specs_changed (NULL, self); - unrecognized_specs_changed (NULL, self); -} - -static void -impl_settings_list_connections (NMDBusObject *obj, - const NMDBusInterfaceInfoExtended *interface_info, - const NMDBusMethodInfoExtended *method_info, - GDBusConnection *dbus_connection, - const char *sender, - GDBusMethodInvocation *invocation, - GVariant *parameters) -{ - NMSettings *self = NM_SETTINGS (obj); - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - gs_free const char **strv = NULL; - - strv = nm_dbus_utils_get_paths_for_clist (&priv->connections_lst_head, - priv->connections_len, - G_STRUCT_OFFSET (NMSettingsConnection, _connections_lst), - TRUE); - g_dbus_method_invocation_return_value (invocation, - g_variant_new ("(^ao)", strv)); -} - -NMSettingsConnection * -nm_settings_get_connection_by_uuid (NMSettings *self, const char *uuid) -{ - NMSettingsPrivate *priv; - NMSettingsConnection *candidate; - - g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); - g_return_val_if_fail (uuid != NULL, NULL); - - priv = NM_SETTINGS_GET_PRIVATE (self); - - c_list_for_each_entry (candidate, &priv->connections_lst_head, _connections_lst) { - if (nm_streq (uuid, nm_settings_connection_get_uuid (candidate))) - return candidate; - } - - return NULL; -} - -static void -impl_settings_get_connection_by_uuid (NMDBusObject *obj, - const NMDBusInterfaceInfoExtended *interface_info, - const NMDBusMethodInfoExtended *method_info, - GDBusConnection *dbus_connection, - const char *sender, - GDBusMethodInvocation *invocation, - GVariant *parameters) -{ - NMSettings *self = NM_SETTINGS (obj); - NMSettingsConnection *sett_conn; - gs_unref_object NMAuthSubject *subject = NULL; - GError *error = NULL; - const char *uuid; - - g_variant_get (parameters, "(&s)", &uuid); - - sett_conn = nm_settings_get_connection_by_uuid (self, uuid); - if (!sett_conn) { - error = g_error_new_literal (NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_INVALID_CONNECTION, - "No connection with the UUID was found."); - goto error; - } - - subject = nm_auth_subject_new_unix_process_from_context (invocation); - if (!subject) { - error = g_error_new_literal (NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_PERMISSION_DENIED, - "Unable to determine UID of request."); - goto error; - } - - if (!nm_auth_is_subject_in_acl_set_error (nm_settings_connection_get_connection (sett_conn), - subject, - NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_PERMISSION_DENIED, - &error)) - goto error; - - g_dbus_method_invocation_return_value (invocation, - g_variant_new ("(o)", - nm_dbus_object_get_path (NM_DBUS_OBJECT (sett_conn)))); - return; - -error: - g_dbus_method_invocation_take_error (invocation, error); -} - -static void -_clear_connections_cached_list (NMSettingsPrivate *priv) -{ - if (!priv->connections_cached_list) - return; - - nm_assert (priv->connections_len == NM_PTRARRAY_LEN (priv->connections_cached_list)); - -#if NM_MORE_ASSERTS - /* set the pointer to a bogus value. This makes it more apparent - * if somebody has a reference to the cached list and still uses - * it. That is a bug, this code just tries to make it blow up - * more eagerly. */ - memset (priv->connections_cached_list, - 0xdeaddead, - sizeof (NMSettingsConnection *) * (priv->connections_len + 1)); -#endif - - nm_clear_g_free (&priv->connections_cached_list); -} - -/** - * nm_settings_get_connections: - * @self: the #NMSettings - * @out_len: (out) (allow-none): returns the number of returned - * connections. - * - * Returns: (transfer none): a list of NMSettingsConnections. The list is - * unsorted and NULL terminated. The result is never %NULL, in case of no - * connections, it returns an empty list. - * The returned list is cached internally, only valid until the next - * NMSettings operation. - */ -NMSettingsConnection *const* -nm_settings_get_connections (NMSettings *self, guint *out_len) -{ - NMSettingsPrivate *priv; - NMSettingsConnection **v; - NMSettingsConnection *con; - guint i; - - g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); - - priv = NM_SETTINGS_GET_PRIVATE (self); - - nm_assert (priv->connections_len == c_list_length (&priv->connections_lst_head)); - - if (G_UNLIKELY (!priv->connections_cached_list)) { - v = g_new (NMSettingsConnection *, priv->connections_len + 1); - - i = 0; - c_list_for_each_entry (con, &priv->connections_lst_head, _connections_lst) { - nm_assert (i < priv->connections_len); - v[i++] = con; - } - nm_assert (i == priv->connections_len); - v[i] = NULL; - - priv->connections_cached_list = v; - } - - NM_SET_OUT (out_len, priv->connections_len); - return priv->connections_cached_list; -} - -/** - * nm_settings_get_connections_clone: - * @self: the #NMSetting - * @out_len: (allow-none): optional output argument - * @func: caller-supplied function for filtering connections - * @func_data: caller-supplied data passed to @func - * @sort_compare_func: (allow-none): optional function pointer for - * sorting the returned list. - * @sort_data: user data for @sort_compare_func. - * - * Returns: (transfer container) (element-type NMSettingsConnection): - * an NULL terminated array of #NMSettingsConnection objects that were - * filtered by @func (or all connections if no filter was specified). - * The order is arbitrary. - * Caller is responsible for freeing the returned array with free(), - * the contained values do not need to be unrefed. - */ -NMSettingsConnection ** -nm_settings_get_connections_clone (NMSettings *self, - guint *out_len, - NMSettingsConnectionFilterFunc func, - gpointer func_data, - GCompareDataFunc sort_compare_func, - gpointer sort_data) -{ - NMSettingsConnection *const*list_cached; - NMSettingsConnection **list; - guint len, i, j; - - g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); - - list_cached = nm_settings_get_connections (self, &len); - -#if NM_MORE_ASSERTS - nm_assert (list_cached); - for (i = 0; i < len; i++) - nm_assert (NM_IS_SETTINGS_CONNECTION (list_cached[i])); - nm_assert (!list_cached[i]); -#endif - - list = g_new (NMSettingsConnection *, ((gsize) len + 1)); - if (func) { - for (i = 0, j = 0; i < len; i++) { - if (func (self, list_cached[i], func_data)) - list[j++] = list_cached[i]; - } - list[j] = NULL; - len = j; - } else - memcpy (list, list_cached, sizeof (list[0]) * ((gsize) len + 1)); - - if ( len > 1 - && sort_compare_func) { - g_qsort_with_data (list, len, sizeof (NMSettingsConnection *), - sort_compare_func, sort_data); - } - NM_SET_OUT (out_len, len); - return list; -} - -NMSettingsConnection * -nm_settings_get_connection_by_path (NMSettings *self, const char *path) -{ - NMSettingsPrivate *priv; - NMSettingsConnection *connection; - - g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); - g_return_val_if_fail (path, NULL); - - priv = NM_SETTINGS_GET_PRIVATE (self); - - connection = nm_dbus_manager_lookup_object (nm_dbus_object_get_manager (NM_DBUS_OBJECT (self)), - path); - if ( !connection - || !NM_IS_SETTINGS_CONNECTION (connection)) + if (priv->startup_complete) return NULL; - - nm_assert (c_list_contains (&priv->connections_lst_head, &connection->_connections_lst)); - return connection; + if (priv->startup_complete_blocked_by) + uuid = nm_settings_connection_get_uuid (priv->startup_complete_blocked_by); + return uuid ?: "unknown"; } -gboolean -nm_settings_has_connection (NMSettings *self, NMSettingsConnection *connection) -{ - gboolean has; - - g_return_val_if_fail (NM_IS_SETTINGS (self), FALSE); - g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (connection), FALSE); - - has = !c_list_is_empty (&connection->_connections_lst); - - nm_assert (has == nm_c_list_contains_entry (&NM_SETTINGS_GET_PRIVATE (self)->connections_lst_head, - connection, - _connections_lst)); - nm_assert (({ - NMSettingsConnection *candidate = NULL; - const char *path; - - path = nm_dbus_object_get_path (NM_DBUS_OBJECT (connection)); - if (path) - candidate = nm_settings_get_connection_by_path (self, path); - - (has == (connection == candidate)); - })); - - return has; -} +/*****************************************************************************/ const GSList * nm_settings_get_unmanaged_specs (NMSettings *self) @@ -586,177 +294,55 @@ unrecognized_specs_changed (NMSettingsPlugin *config, nm_settings_plugin_get_unrecognized_specs); } +/*****************************************************************************/ + static void -add_plugin (NMSettings *self, - NMSettingsPlugin *plugin, - const char *pname, - const char *path) +plugin_connection_added (NMSettingsPlugin *config, + NMSettingsConnection *connection, + NMSettings *self) { - NMSettingsPrivate *priv; - - nm_assert (NM_IS_SETTINGS (self)); - nm_assert (NM_IS_SETTINGS_PLUGIN (plugin)); - - priv = NM_SETTINGS_GET_PRIVATE (self); - - nm_assert (!g_slist_find (priv->plugins, plugin)); - - priv->plugins = g_slist_append (priv->plugins, g_object_ref (plugin)); - - _LOGI ("Loaded settings plugin: %s (%s%s%s)", - pname, - NM_PRINT_FMT_QUOTED (path, "\"", path, "\"", "internal")); -} - -static gboolean -add_plugin_load_file (NMSettings *self, const char *pname, GError **error) -{ - gs_free char *full_name = NULL; - gs_free char *path = NULL; - gs_unref_object NMSettingsPlugin *plugin = NULL; - GModule *module; - NMSettingsPluginFactoryFunc factory_func; - struct stat st; - int errsv; - - full_name = g_strdup_printf ("nm-settings-plugin-%s", pname); - path = g_module_build_path (NMPLUGINDIR, full_name); - - if (stat (path, &st) != 0) { - errsv = errno; - _LOGW ("could not load plugin '%s' from file '%s': %s", pname, path, nm_strerror_native (errsv)); - return TRUE; - } - if (!S_ISREG (st.st_mode)) { - _LOGW ("could not load plugin '%s' from file '%s': not a file", pname, path); - return TRUE; - } - if (st.st_uid != 0) { - _LOGW ("could not load plugin '%s' from file '%s': file must be owned by root", pname, path); - return TRUE; - } - if (st.st_mode & (S_IWGRP | S_IWOTH | S_ISUID)) { - _LOGW ("could not load plugin '%s' from file '%s': invalid file permissions", pname, path); - return TRUE; - } - - module = g_module_open (path, G_MODULE_BIND_LOCAL); - if (!module) { - _LOGW ("could not load plugin '%s' from file '%s': %s", - pname, path, g_module_error ()); - return TRUE; - } - - /* errors after this point are fatal, because we loaded the shared library already. */ - - if (!g_module_symbol (module, "nm_settings_plugin_factory", (gpointer) (&factory_func))) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, - "Could not find plugin '%s' factory function.", - pname); - g_module_close (module); - return FALSE; - } - - /* after accessing the plugin we cannot unload it anymore, because the glib - * types cannot be properly unregistered. */ - g_module_make_resident (module); - - plugin = (*factory_func) (); - if (!NM_IS_SETTINGS_PLUGIN (plugin)) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, - "plugin '%s' returned invalid settings plugin", - pname); - return FALSE; - } - - add_plugin (self, NM_SETTINGS_PLUGIN (plugin), pname, path); - return TRUE; + claim_connection (self, connection); } static void -add_plugin_keyfile (NMSettings *self) +load_connections (NMSettings *self) { - gs_unref_object NMSKeyfilePlugin *keyfile_plugin = NULL; + NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); + GSList *iter; - keyfile_plugin = nms_keyfile_plugin_new (); - add_plugin (self, NM_SETTINGS_PLUGIN (keyfile_plugin), "keyfile", NULL); -} + for (iter = priv->plugins; iter; iter = g_slist_next (iter)) { + NMSettingsPlugin *plugin = NM_SETTINGS_PLUGIN (iter->data); + GSList *plugin_connections; + GSList *elt; -static gboolean -load_plugins (NMSettings *self, const char **plugins, GError **error) -{ - const char **iter; - gboolean keyfile_added = FALSE; - gboolean success = TRUE; - gboolean add_ibft = FALSE; - gboolean has_no_ibft; - gssize idx_no_ibft, idx_ibft; + plugin_connections = nm_settings_plugin_get_connections (plugin); - idx_ibft = nm_utils_strv_find_first ((char **) plugins, -1, "ibft"); - idx_no_ibft = nm_utils_strv_find_first ((char **) plugins, -1, "no-ibft"); - has_no_ibft = idx_no_ibft >= 0 && idx_no_ibft > idx_ibft; -#if WITH_SETTINGS_PLUGIN_IBFT - add_ibft = idx_no_ibft < 0 && idx_ibft < 0; -#endif + // FIXME: ensure connections from plugins loaded with a lower priority + // get rejected when they conflict with connections from a higher + // priority plugin. - for (iter = plugins; iter && *iter; iter++) { - const char *pname = *iter; + for (elt = plugin_connections; elt; elt = g_slist_next (elt)) + claim_connection (self, elt->data); - if (!*pname || strchr (pname, '/')) { - _LOGW ("ignore invalid plugin \"%s\"", pname); - continue; - } + g_slist_free (plugin_connections); - if (NM_IN_STRSET (pname, "ifcfg-suse", "ifnet")) { - _LOGW ("skipping deprecated plugin %s", pname); - continue; - } - - if (nm_streq (pname, "no-ibft")) - continue; - if (has_no_ibft && nm_streq (pname, "ibft")) - continue; - - /* keyfile plugin is built-in now */ - if (nm_streq (pname, "keyfile")) { - if (!keyfile_added) { - add_plugin_keyfile (self); - keyfile_added = TRUE; - } - continue; - } - - if (nm_utils_strv_find_first ((char **) plugins, - iter - plugins, - pname) >= 0) { - /* the plugin is already mentioned in the list previously. - * Don't load a duplicate. */ - continue; - } - - success = add_plugin_load_file (self, pname, error); - if (!success) - break; - - if (add_ibft && nm_streq (pname, "ifcfg-rh")) { - /* The plugin ibft is not explicitly mentioned but we just enabled "ifcfg-rh". - * Enable "ibft" by default after "ifcfg-rh". */ - pname = "ibft"; - add_ibft = FALSE; - - success = add_plugin_load_file (self, "ibft", error); - if (!success) - break; - } + g_signal_connect (plugin, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, + G_CALLBACK (plugin_connection_added), self); + g_signal_connect (plugin, NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED, + G_CALLBACK (unmanaged_specs_changed), self); + g_signal_connect (plugin, NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED, + G_CALLBACK (unrecognized_specs_changed), self); } - /* If keyfile plugin was not among configured plugins, add it as the last one */ - if (!keyfile_added && success) - add_plugin_keyfile (self); + priv->connections_loaded = TRUE; + _notify (self, PROP_CONNECTIONS); - return success; + unmanaged_specs_changed (NULL, self); + unrecognized_specs_changed (NULL, self); } +/*****************************************************************************/ + static void connection_updated (NMSettingsConnection *connection, gboolean by_user, gpointer user_data) { @@ -835,45 +421,7 @@ connection_removed (NMSettingsConnection *connection, gpointer user_data) g_object_unref (self); /* Balanced by a ref in claim_connection() */ } -#define NM_DBUS_SERVICE_OPENCONNECT "org.freedesktop.NetworkManager.openconnect" -#define NM_OPENCONNECT_KEY_GATEWAY "gateway" -#define NM_OPENCONNECT_KEY_COOKIE "cookie" -#define NM_OPENCONNECT_KEY_GWCERT "gwcert" -#define NM_OPENCONNECT_KEY_XMLCONFIG "xmlconfig" -#define NM_OPENCONNECT_KEY_LASTHOST "lasthost" -#define NM_OPENCONNECT_KEY_AUTOCONNECT "autoconnect" -#define NM_OPENCONNECT_KEY_CERTSIGS "certsigs" - -static void -openconnect_migrate_hack (NMConnection *connection) -{ - NMSettingVpn *s_vpn; - NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NOT_SAVED; - - /* Huge hack. There were some openconnect changes that needed to happen - * pretty late, too late to get into distros. Migration has already - * happened for many people, and their secret flags are wrong. But we - * don't want to requrie re-migration, so we have to fix it up here. Ugh. - */ - - s_vpn = nm_connection_get_setting_vpn (connection); - if (s_vpn == NULL) - return; - - if (g_strcmp0 (nm_setting_vpn_get_service_type (s_vpn), NM_DBUS_SERVICE_OPENCONNECT) == 0) { - /* These are different for every login session, and should not be stored */ - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GATEWAY, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_COOKIE, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GWCERT, flags, NULL); - - /* These are purely internal data for the auth-dialog, and should be stored */ - flags = 0; - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_XMLCONFIG, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_LASTHOST, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_AUTOCONNECT, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_CERTSIGS, flags, NULL); - } -} +/*****************************************************************************/ static void claim_connection (NMSettings *self, NMSettingsConnection *sett_conn) @@ -982,6 +530,50 @@ claim_connection (NMSettings *self, NMSettingsConnection *sett_conn) nm_settings_connection_added (sett_conn); } +/*****************************************************************************/ + +#define NM_DBUS_SERVICE_OPENCONNECT "org.freedesktop.NetworkManager.openconnect" +#define NM_OPENCONNECT_KEY_GATEWAY "gateway" +#define NM_OPENCONNECT_KEY_COOKIE "cookie" +#define NM_OPENCONNECT_KEY_GWCERT "gwcert" +#define NM_OPENCONNECT_KEY_XMLCONFIG "xmlconfig" +#define NM_OPENCONNECT_KEY_LASTHOST "lasthost" +#define NM_OPENCONNECT_KEY_AUTOCONNECT "autoconnect" +#define NM_OPENCONNECT_KEY_CERTSIGS "certsigs" + +static void +openconnect_migrate_hack (NMConnection *connection) +{ + NMSettingVpn *s_vpn; + NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NOT_SAVED; + + /* Huge hack. There were some openconnect changes that needed to happen + * pretty late, too late to get into distros. Migration has already + * happened for many people, and their secret flags are wrong. But we + * don't want to requrie re-migration, so we have to fix it up here. Ugh. + */ + + s_vpn = nm_connection_get_setting_vpn (connection); + if (s_vpn == NULL) + return; + + if (g_strcmp0 (nm_setting_vpn_get_service_type (s_vpn), NM_DBUS_SERVICE_OPENCONNECT) == 0) { + /* These are different for every login session, and should not be stored */ + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GATEWAY, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_COOKIE, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GWCERT, flags, NULL); + + /* These are purely internal data for the auth-dialog, and should be stored */ + flags = 0; + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_XMLCONFIG, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_LASTHOST, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_AUTOCONNECT, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_CERTSIGS, flags, NULL); + } +} + +/*****************************************************************************/ + static gboolean secrets_filter_cb (NMSetting *setting, const char *secret, @@ -1472,6 +1064,443 @@ impl_settings_reload_connections (NMDBusObject *obj, /*****************************************************************************/ +static void +_clear_connections_cached_list (NMSettingsPrivate *priv) +{ + if (!priv->connections_cached_list) + return; + + nm_assert (priv->connections_len == NM_PTRARRAY_LEN (priv->connections_cached_list)); + +#if NM_MORE_ASSERTS + /* set the pointer to a bogus value. This makes it more apparent + * if somebody has a reference to the cached list and still uses + * it. That is a bug, this code just tries to make it blow up + * more eagerly. */ + memset (priv->connections_cached_list, + 0xdeaddead, + sizeof (NMSettingsConnection *) * (priv->connections_len + 1)); +#endif + + nm_clear_g_free (&priv->connections_cached_list); +} + +static void +impl_settings_list_connections (NMDBusObject *obj, + const NMDBusInterfaceInfoExtended *interface_info, + const NMDBusMethodInfoExtended *method_info, + GDBusConnection *dbus_connection, + const char *sender, + GDBusMethodInvocation *invocation, + GVariant *parameters) +{ + NMSettings *self = NM_SETTINGS (obj); + NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); + gs_free const char **strv = NULL; + + strv = nm_dbus_utils_get_paths_for_clist (&priv->connections_lst_head, + priv->connections_len, + G_STRUCT_OFFSET (NMSettingsConnection, _connections_lst), + TRUE); + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(^ao)", strv)); +} + +NMSettingsConnection * +nm_settings_get_connection_by_uuid (NMSettings *self, const char *uuid) +{ + NMSettingsPrivate *priv; + NMSettingsConnection *candidate; + + g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); + g_return_val_if_fail (uuid != NULL, NULL); + + priv = NM_SETTINGS_GET_PRIVATE (self); + + c_list_for_each_entry (candidate, &priv->connections_lst_head, _connections_lst) { + if (nm_streq (uuid, nm_settings_connection_get_uuid (candidate))) + return candidate; + } + + return NULL; +} + +static void +impl_settings_get_connection_by_uuid (NMDBusObject *obj, + const NMDBusInterfaceInfoExtended *interface_info, + const NMDBusMethodInfoExtended *method_info, + GDBusConnection *dbus_connection, + const char *sender, + GDBusMethodInvocation *invocation, + GVariant *parameters) +{ + NMSettings *self = NM_SETTINGS (obj); + NMSettingsConnection *sett_conn; + gs_unref_object NMAuthSubject *subject = NULL; + GError *error = NULL; + const char *uuid; + + g_variant_get (parameters, "(&s)", &uuid); + + sett_conn = nm_settings_get_connection_by_uuid (self, uuid); + if (!sett_conn) { + error = g_error_new_literal (NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_INVALID_CONNECTION, + "No connection with the UUID was found."); + goto error; + } + + subject = nm_auth_subject_new_unix_process_from_context (invocation); + if (!subject) { + error = g_error_new_literal (NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_PERMISSION_DENIED, + "Unable to determine UID of request."); + goto error; + } + + if (!nm_auth_is_subject_in_acl_set_error (nm_settings_connection_get_connection (sett_conn), + subject, + NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_PERMISSION_DENIED, + &error)) + goto error; + + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(o)", + nm_dbus_object_get_path (NM_DBUS_OBJECT (sett_conn)))); + return; + +error: + g_dbus_method_invocation_take_error (invocation, error); +} + +/** + * nm_settings_get_connections: + * @self: the #NMSettings + * @out_len: (out) (allow-none): returns the number of returned + * connections. + * + * Returns: (transfer none): a list of NMSettingsConnections. The list is + * unsorted and NULL terminated. The result is never %NULL, in case of no + * connections, it returns an empty list. + * The returned list is cached internally, only valid until the next + * NMSettings operation. + */ +NMSettingsConnection *const* +nm_settings_get_connections (NMSettings *self, guint *out_len) +{ + NMSettingsPrivate *priv; + NMSettingsConnection **v; + NMSettingsConnection *con; + guint i; + + g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); + + priv = NM_SETTINGS_GET_PRIVATE (self); + + nm_assert (priv->connections_len == c_list_length (&priv->connections_lst_head)); + + if (G_UNLIKELY (!priv->connections_cached_list)) { + v = g_new (NMSettingsConnection *, priv->connections_len + 1); + + i = 0; + c_list_for_each_entry (con, &priv->connections_lst_head, _connections_lst) { + nm_assert (i < priv->connections_len); + v[i++] = con; + } + nm_assert (i == priv->connections_len); + v[i] = NULL; + + priv->connections_cached_list = v; + } + + NM_SET_OUT (out_len, priv->connections_len); + return priv->connections_cached_list; +} + +/** + * nm_settings_get_connections_clone: + * @self: the #NMSetting + * @out_len: (allow-none): optional output argument + * @func: caller-supplied function for filtering connections + * @func_data: caller-supplied data passed to @func + * @sort_compare_func: (allow-none): optional function pointer for + * sorting the returned list. + * @sort_data: user data for @sort_compare_func. + * + * Returns: (transfer container) (element-type NMSettingsConnection): + * an NULL terminated array of #NMSettingsConnection objects that were + * filtered by @func (or all connections if no filter was specified). + * The order is arbitrary. + * Caller is responsible for freeing the returned array with free(), + * the contained values do not need to be unrefed. + */ +NMSettingsConnection ** +nm_settings_get_connections_clone (NMSettings *self, + guint *out_len, + NMSettingsConnectionFilterFunc func, + gpointer func_data, + GCompareDataFunc sort_compare_func, + gpointer sort_data) +{ + NMSettingsConnection *const*list_cached; + NMSettingsConnection **list; + guint len, i, j; + + g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); + + list_cached = nm_settings_get_connections (self, &len); + +#if NM_MORE_ASSERTS + nm_assert (list_cached); + for (i = 0; i < len; i++) + nm_assert (NM_IS_SETTINGS_CONNECTION (list_cached[i])); + nm_assert (!list_cached[i]); +#endif + + list = g_new (NMSettingsConnection *, ((gsize) len + 1)); + if (func) { + for (i = 0, j = 0; i < len; i++) { + if (func (self, list_cached[i], func_data)) + list[j++] = list_cached[i]; + } + list[j] = NULL; + len = j; + } else + memcpy (list, list_cached, sizeof (list[0]) * ((gsize) len + 1)); + + if ( len > 1 + && sort_compare_func) { + g_qsort_with_data (list, len, sizeof (NMSettingsConnection *), + sort_compare_func, sort_data); + } + NM_SET_OUT (out_len, len); + return list; +} + +NMSettingsConnection * +nm_settings_get_connection_by_path (NMSettings *self, const char *path) +{ + NMSettingsPrivate *priv; + NMSettingsConnection *connection; + + g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); + g_return_val_if_fail (path, NULL); + + priv = NM_SETTINGS_GET_PRIVATE (self); + + connection = nm_dbus_manager_lookup_object (nm_dbus_object_get_manager (NM_DBUS_OBJECT (self)), + path); + if ( !connection + || !NM_IS_SETTINGS_CONNECTION (connection)) + return NULL; + + nm_assert (c_list_contains (&priv->connections_lst_head, &connection->_connections_lst)); + return connection; +} + +gboolean +nm_settings_has_connection (NMSettings *self, NMSettingsConnection *connection) +{ + gboolean has; + + g_return_val_if_fail (NM_IS_SETTINGS (self), FALSE); + g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (connection), FALSE); + + has = !c_list_is_empty (&connection->_connections_lst); + + nm_assert (has == nm_c_list_contains_entry (&NM_SETTINGS_GET_PRIVATE (self)->connections_lst_head, + connection, + _connections_lst)); + nm_assert (({ + NMSettingsConnection *candidate = NULL; + const char *path; + + path = nm_dbus_object_get_path (NM_DBUS_OBJECT (connection)); + if (path) + candidate = nm_settings_get_connection_by_path (self, path); + + (has == (connection == candidate)); + })); + + return has; +} + +/*****************************************************************************/ + +static void +add_plugin (NMSettings *self, + NMSettingsPlugin *plugin, + const char *pname, + const char *path) +{ + NMSettingsPrivate *priv; + + nm_assert (NM_IS_SETTINGS (self)); + nm_assert (NM_IS_SETTINGS_PLUGIN (plugin)); + + priv = NM_SETTINGS_GET_PRIVATE (self); + + nm_assert (!g_slist_find (priv->plugins, plugin)); + + priv->plugins = g_slist_append (priv->plugins, g_object_ref (plugin)); + + _LOGI ("Loaded settings plugin: %s (%s%s%s)", + pname, + NM_PRINT_FMT_QUOTED (path, "\"", path, "\"", "internal")); +} + +static gboolean +add_plugin_load_file (NMSettings *self, const char *pname, GError **error) +{ + gs_free char *full_name = NULL; + gs_free char *path = NULL; + gs_unref_object NMSettingsPlugin *plugin = NULL; + GModule *module; + NMSettingsPluginFactoryFunc factory_func; + struct stat st; + int errsv; + + full_name = g_strdup_printf ("nm-settings-plugin-%s", pname); + path = g_module_build_path (NMPLUGINDIR, full_name); + + if (stat (path, &st) != 0) { + errsv = errno; + _LOGW ("could not load plugin '%s' from file '%s': %s", pname, path, nm_strerror_native (errsv)); + return TRUE; + } + if (!S_ISREG (st.st_mode)) { + _LOGW ("could not load plugin '%s' from file '%s': not a file", pname, path); + return TRUE; + } + if (st.st_uid != 0) { + _LOGW ("could not load plugin '%s' from file '%s': file must be owned by root", pname, path); + return TRUE; + } + if (st.st_mode & (S_IWGRP | S_IWOTH | S_ISUID)) { + _LOGW ("could not load plugin '%s' from file '%s': invalid file permissions", pname, path); + return TRUE; + } + + module = g_module_open (path, G_MODULE_BIND_LOCAL); + if (!module) { + _LOGW ("could not load plugin '%s' from file '%s': %s", + pname, path, g_module_error ()); + return TRUE; + } + + /* errors after this point are fatal, because we loaded the shared library already. */ + + if (!g_module_symbol (module, "nm_settings_plugin_factory", (gpointer) (&factory_func))) { + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "Could not find plugin '%s' factory function.", + pname); + g_module_close (module); + return FALSE; + } + + /* after accessing the plugin we cannot unload it anymore, because the glib + * types cannot be properly unregistered. */ + g_module_make_resident (module); + + plugin = (*factory_func) (); + if (!NM_IS_SETTINGS_PLUGIN (plugin)) { + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "plugin '%s' returned invalid settings plugin", + pname); + return FALSE; + } + + add_plugin (self, NM_SETTINGS_PLUGIN (plugin), pname, path); + return TRUE; +} + +static void +add_plugin_keyfile (NMSettings *self) +{ + gs_unref_object NMSKeyfilePlugin *keyfile_plugin = NULL; + + keyfile_plugin = nms_keyfile_plugin_new (); + add_plugin (self, NM_SETTINGS_PLUGIN (keyfile_plugin), "keyfile", NULL); +} + +static gboolean +load_plugins (NMSettings *self, const char **plugins, GError **error) +{ + const char **iter; + gboolean keyfile_added = FALSE; + gboolean success = TRUE; + gboolean add_ibft = FALSE; + gboolean has_no_ibft; + gssize idx_no_ibft, idx_ibft; + + idx_ibft = nm_utils_strv_find_first ((char **) plugins, -1, "ibft"); + idx_no_ibft = nm_utils_strv_find_first ((char **) plugins, -1, "no-ibft"); + has_no_ibft = idx_no_ibft >= 0 && idx_no_ibft > idx_ibft; +#if WITH_SETTINGS_PLUGIN_IBFT + add_ibft = idx_no_ibft < 0 && idx_ibft < 0; +#endif + + for (iter = plugins; iter && *iter; iter++) { + const char *pname = *iter; + + if (!*pname || strchr (pname, '/')) { + _LOGW ("ignore invalid plugin \"%s\"", pname); + continue; + } + + if (NM_IN_STRSET (pname, "ifcfg-suse", "ifnet")) { + _LOGW ("skipping deprecated plugin %s", pname); + continue; + } + + if (nm_streq (pname, "no-ibft")) + continue; + if (has_no_ibft && nm_streq (pname, "ibft")) + continue; + + /* keyfile plugin is built-in now */ + if (nm_streq (pname, "keyfile")) { + if (!keyfile_added) { + add_plugin_keyfile (self); + keyfile_added = TRUE; + } + continue; + } + + if (nm_utils_strv_find_first ((char **) plugins, + iter - plugins, + pname) >= 0) { + /* the plugin is already mentioned in the list previously. + * Don't load a duplicate. */ + continue; + } + + success = add_plugin_load_file (self, pname, error); + if (!success) + break; + + if (add_ibft && nm_streq (pname, "ifcfg-rh")) { + /* The plugin ibft is not explicitly mentioned but we just enabled "ifcfg-rh". + * Enable "ibft" by default after "ifcfg-rh". */ + pname = "ibft"; + add_ibft = FALSE; + + success = add_plugin_load_file (self, "ibft", error); + if (!success) + break; + } + } + + /* If keyfile plugin was not among configured plugins, add it as the last one */ + if (!keyfile_added && success) + add_plugin_keyfile (self); + + return success; +} + +/*****************************************************************************/ + static void pk_hostname_cb (NMAuthChain *chain, GDBusMethodInvocation *context, @@ -1551,6 +1580,16 @@ impl_settings_save_hostname (NMDBusObject *obj, /*****************************************************************************/ +static void +_hostname_changed_cb (NMHostnameManager *hostname_manager, + GParamSpec *pspec, + gpointer user_data) +{ + _notify (user_data, PROP_HOSTNAME); +} + +/*****************************************************************************/ + static gboolean have_connection_for_device (NMSettings *self, NMDevice *device) { @@ -1859,31 +1898,6 @@ nm_settings_kf_db_write (NMSettings *self) /*****************************************************************************/ -const char * -nm_settings_get_startup_complete_blocked_reason (NMSettings *self) -{ - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - const char *uuid = NULL; - - if (priv->startup_complete) - return NULL; - if (priv->startup_complete_blocked_by) - uuid = nm_settings_connection_get_uuid (priv->startup_complete_blocked_by); - return uuid ?: "unknown"; -} - -/*****************************************************************************/ - -static void -_hostname_changed_cb (NMHostnameManager *hostname_manager, - GParamSpec *pspec, - gpointer user_data) -{ - _notify (user_data, PROP_HOSTNAME); -} - -/*****************************************************************************/ - gboolean nm_settings_start (NMSettings *self, GError **error) { From be0018382d4d60a1b550651e623f0186e729363b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 21 May 2019 10:06:10 +0200 Subject: [PATCH 11/19] settings: in have_connection_for_device() first skip over irrelevant connection types nm_device_check_connection_compatible() is potentially expensive. Check first whether the connection candidate is of a relevant type, hoping that this check is cheaper and thus shortcuts other checks early. --- src/settings/nm-settings.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index f1d0ee9495..f342d6b446 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1594,7 +1594,6 @@ static gboolean have_connection_for_device (NMSettings *self, NMDevice *device) { NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - NMSettingConnection *s_con; NMSettingWired *s_wired; const char *setting_hwaddr; const char *perm_hw_addr; @@ -1607,32 +1606,29 @@ have_connection_for_device (NMSettings *self, NMDevice *device) /* Find a wired connection locked to the given MAC address, if any */ c_list_for_each_entry (sett_conn, &priv->connections_lst_head, _connections_lst) { NMConnection *connection = nm_settings_connection_get_connection (sett_conn); - const char *ctype, *iface; + NMSettingConnection *s_con = nm_connection_get_setting_connection (connection); + const char *ctype; + const char *iface; + + ctype = nm_setting_connection_get_connection_type (s_con); + if (!NM_IN_STRSET (ctype, NM_SETTING_WIRED_SETTING_NAME, + NM_SETTING_PPPOE_SETTING_NAME)) + continue; if (!nm_device_check_connection_compatible (device, connection, NULL)) continue; - s_con = nm_connection_get_setting_connection (connection); - iface = nm_setting_connection_get_interface_name (s_con); - if (iface && strcmp (iface, nm_device_get_iface (device)) != 0) - continue; - - ctype = nm_setting_connection_get_connection_type (s_con); - if ( strcmp (ctype, NM_SETTING_WIRED_SETTING_NAME) - && strcmp (ctype, NM_SETTING_PPPOE_SETTING_NAME)) + if (nm_streq0 (iface, nm_device_get_iface (device))) continue; s_wired = nm_connection_get_setting_wired (connection); - if ( !s_wired && nm_streq (ctype, NM_SETTING_PPPOE_SETTING_NAME)) { /* No wired setting; therefore the PPPoE connection applies to any device */ return TRUE; } - nm_assert (s_wired); - setting_hwaddr = nm_setting_wired_get_mac_address (s_wired); if (setting_hwaddr) { /* A connection mac-locked to this device */ From a63714ec1de4ad174f04569726a23de7928d96cb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 May 2019 15:01:21 +0200 Subject: [PATCH 12/19] settings,keyfile: move openconnect hack from settings to keyfile reader VPN settings (for openconnect) can only be handled by the keyfile settings plugin. In any case, such special casing belongs to the settings plugin and not "nm-settings.c". The reason is that the settings plugin already has an intimate understanding of the content of connections, it knows which fields exist, their meaning, etc. It makes sense special handling of openconnect is done there. See also commit 304d0b869bfe ('core: openconnect migration hack'). Unfortunately it's not clear to me why/whether this is still the right thing to do. --- libnm-core/nm-keyfile.c | 48 ++++++++++++++++++++++++++++++++++++++ src/settings/nm-settings.c | 47 ------------------------------------- 2 files changed, 48 insertions(+), 47 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index f31853c96f..8add4bfccf 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -363,6 +363,53 @@ read_field (char **current, const char **out_err_str, const char *characters, co } } +/*****************************************************************************/ + +#define NM_DBUS_SERVICE_OPENCONNECT "org.freedesktop.NetworkManager.openconnect" +#define NM_OPENCONNECT_KEY_GATEWAY "gateway" +#define NM_OPENCONNECT_KEY_COOKIE "cookie" +#define NM_OPENCONNECT_KEY_GWCERT "gwcert" +#define NM_OPENCONNECT_KEY_XMLCONFIG "xmlconfig" +#define NM_OPENCONNECT_KEY_LASTHOST "lasthost" +#define NM_OPENCONNECT_KEY_AUTOCONNECT "autoconnect" +#define NM_OPENCONNECT_KEY_CERTSIGS "certsigs" + +static void +openconnect_fix_secret_flags (NMSetting *setting) +{ + NMSettingVpn *s_vpn; + NMSettingSecretFlags flags; + + /* Huge hack. There were some openconnect changes that needed to happen + * pretty late, too late to get into distros. Migration has already + * happened for many people, and their secret flags are wrong. But we + * don't want to requrie re-migration, so we have to fix it up here. Ugh. + */ + + if (!NM_IS_SETTING_VPN (setting)) + return; + + s_vpn = NM_SETTING_VPN (setting); + + if (!nm_streq0 (nm_setting_vpn_get_service_type (s_vpn), NM_DBUS_SERVICE_OPENCONNECT)) + return; + + /* These are different for every login session, and should not be stored */ + flags = NM_SETTING_SECRET_FLAG_NOT_SAVED; + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GATEWAY, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_COOKIE, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GWCERT, flags, NULL); + + /* These are purely internal data for the auth-dialog, and should be stored */ + flags = 0; + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_XMLCONFIG, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_LASTHOST, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_AUTOCONNECT, flags, NULL); + nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_CERTSIGS, flags, NULL); +} + +/*****************************************************************************/ + #define IP_ADDRESS_CHARS "0123456789abcdefABCDEF:.%" #define DIGITS "0123456789" #define DELIMITERS "/;," @@ -1024,6 +1071,7 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key) nm_setting_bond_add_option (NM_SETTING_BOND (setting), name, value); } } + openconnect_fix_secret_flags (setting); return; } diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index f342d6b446..18ec00f6dd 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -178,7 +178,6 @@ static void default_wired_clear_tag (NMSettings *self, NMSettingsConnection *connection, gboolean add_to_no_auto_default); -static void openconnect_migrate_hack (NMConnection *connection); static void _clear_connections_cached_list (NMSettingsPrivate *priv); /*****************************************************************************/ @@ -474,10 +473,6 @@ claim_connection (NMSettings *self, NMSettingsConnection *sett_conn) /* Ensure its initial visibility is up-to-date */ nm_settings_connection_recheck_visibility (sett_conn); - /* Evil openconnect migration hack */ - /* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */ - openconnect_migrate_hack (nm_settings_connection_get_connection (sett_conn)); - /* This one unexports the connection, it needs to run late to give the active * connection a chance to deal with its reference to this settings connection. */ g_signal_connect_after (sett_conn, NM_SETTINGS_CONNECTION_REMOVED, @@ -532,48 +527,6 @@ claim_connection (NMSettings *self, NMSettingsConnection *sett_conn) /*****************************************************************************/ -#define NM_DBUS_SERVICE_OPENCONNECT "org.freedesktop.NetworkManager.openconnect" -#define NM_OPENCONNECT_KEY_GATEWAY "gateway" -#define NM_OPENCONNECT_KEY_COOKIE "cookie" -#define NM_OPENCONNECT_KEY_GWCERT "gwcert" -#define NM_OPENCONNECT_KEY_XMLCONFIG "xmlconfig" -#define NM_OPENCONNECT_KEY_LASTHOST "lasthost" -#define NM_OPENCONNECT_KEY_AUTOCONNECT "autoconnect" -#define NM_OPENCONNECT_KEY_CERTSIGS "certsigs" - -static void -openconnect_migrate_hack (NMConnection *connection) -{ - NMSettingVpn *s_vpn; - NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NOT_SAVED; - - /* Huge hack. There were some openconnect changes that needed to happen - * pretty late, too late to get into distros. Migration has already - * happened for many people, and their secret flags are wrong. But we - * don't want to requrie re-migration, so we have to fix it up here. Ugh. - */ - - s_vpn = nm_connection_get_setting_vpn (connection); - if (s_vpn == NULL) - return; - - if (g_strcmp0 (nm_setting_vpn_get_service_type (s_vpn), NM_DBUS_SERVICE_OPENCONNECT) == 0) { - /* These are different for every login session, and should not be stored */ - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GATEWAY, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_COOKIE, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_GWCERT, flags, NULL); - - /* These are purely internal data for the auth-dialog, and should be stored */ - flags = 0; - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_XMLCONFIG, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_LASTHOST, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_AUTOCONNECT, flags, NULL); - nm_setting_set_secret_flags (NM_SETTING (s_vpn), NM_OPENCONNECT_KEY_CERTSIGS, flags, NULL); - } -} - -/*****************************************************************************/ - static gboolean secrets_filter_cb (NMSetting *setting, const char *secret, From 142c1215ee1b8423e7739175a0382a91924629c6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 26 May 2019 18:49:55 +0200 Subject: [PATCH 13/19] auth-chain: track auth-chains in embedded CList NMManager and NMSettings both may have multiple authorization requests ongoing. They need to keep track of them, at the very least to be able to cancel them on shutdown. Since NMAuthChain is not ref-countable, it always has only one clear user/owner. It makes little sense otherwise. Since most callers already want to track their NMAuthChain instances, let NMAuthChain help with that. Embed a "parent" CList field inside NMAuthChain. This avoids requiring an additional GSList allocation to track the element. Also, it allows to link and append an element without iterating the list. This ties the caller and the NMAuthChain a bit tighter together (making them less indepdendent). Generally that is not desirable. But here it seems the logic (of tracking the NMAuthChain) is still trivial and well separated. It's just that NMAuthChain instances now can be linked in a CList. --- src/nm-auth-utils.c | 8 ++++++ src/nm-auth-utils.h | 21 +++++++++++++++ src/nm-manager.c | 55 +++++++++++++++++++------------------- src/settings/nm-settings.c | 18 +++++++------ 4 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c index ab7c1887dd..7235cba1a8 100644 --- a/src/nm-auth-utils.c +++ b/src/nm-auth-utils.c @@ -30,6 +30,9 @@ /*****************************************************************************/ struct NMAuthChain { + + CList parent_lst; + CList data_lst_head; CList auth_call_lst_head; @@ -46,6 +49,8 @@ struct NMAuthChain { bool is_finishing:1; }; +G_STATIC_ASSERT (G_STRUCT_OFFSET (NMAuthChain, parent_lst) == 0); + typedef struct { CList auth_call_lst; NMAuthChain *chain; @@ -435,6 +440,7 @@ nm_auth_chain_new_subject (NMAuthSubject *subject, .user_data = user_data, .context = nm_g_object_ref (context), .subject = g_object_ref (subject), + .parent_lst = C_LIST_INIT (self->parent_lst), .data_lst_head = C_LIST_INIT (self->data_lst_head), .auth_call_lst_head = C_LIST_INIT (self->auth_call_lst_head), }; @@ -479,6 +485,8 @@ _auth_chain_destroy (NMAuthChain *self) AuthCall *call; ChainData *chain_data; + c_list_unlink (&self->parent_lst); + nm_clear_g_object (&self->subject); nm_clear_g_object (&self->context); diff --git a/src/nm-auth-utils.h b/src/nm-auth-utils.h index 3344f9ec4f..5201d26052 100644 --- a/src/nm-auth-utils.h +++ b/src/nm-auth-utils.h @@ -24,6 +24,8 @@ #include "nm-auth-manager.h" +/*****************************************************************************/ + typedef struct NMAuthChain NMAuthChain; typedef void (*NMAuthChainResultFunc) (NMAuthChain *chain, @@ -65,6 +67,25 @@ void nm_auth_chain_destroy (NMAuthChain *chain); NMAuthSubject *nm_auth_chain_get_subject (NMAuthChain *self); +/*****************************************************************************/ + +struct CList; + +static inline NMAuthChain * +nm_auth_chain_parent_lst_entry (struct CList *parent_lst_self) +{ + return (NMAuthChain *) ((void *) parent_lst_self); +} + +static inline struct CList * +nm_auth_chain_parent_lst_list (NMAuthChain *self) +{ + return (struct CList *) ((void *) self); +} + +/*****************************************************************************/ + + /* Caller must free returned error description */ gboolean nm_auth_is_subject_in_acl (NMConnection *connection, NMAuthSubject *subect, diff --git a/src/nm-manager.c b/src/nm-manager.c index e4eff3b029..949361f427 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -185,7 +185,8 @@ typedef struct { GHashTable *device_route_metrics; - GSList *auth_chains; + CList auth_lst_head; + GHashTable *sleep_devices; /* Firmware dir monitor */ @@ -1142,7 +1143,7 @@ _reload_auth_cb (NMAuthChain *chain, nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); flags = GPOINTER_TO_UINT (nm_auth_chain_get_data (chain, "flags")); subject = nm_auth_chain_get_subject (chain); @@ -1212,7 +1213,7 @@ impl_manager_reload (NMDBusObject *obj, return; } - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "flags", GUINT_TO_POINTER (flags), NULL); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_RELOAD, TRUE); } @@ -2330,7 +2331,6 @@ device_auth_done_cb (NMAuthChain *chain, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); gs_free_error GError *error = NULL; NMAuthCallResult result; NMDevice *device; @@ -2340,7 +2340,7 @@ device_auth_done_cb (NMAuthChain *chain, nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); permission = nm_auth_chain_get_data (chain, "perm"); nm_assert (permission); @@ -2414,7 +2414,7 @@ device_auth_request_cb (NMDevice *device, permission_dup = g_strdup (permission); - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "device", g_object_ref (device), g_object_unref); nm_auth_chain_set_data (chain, "callback", callback, NULL); nm_auth_chain_set_data (chain, "user-data", user_data, NULL); @@ -5608,7 +5608,6 @@ deactivate_net_auth_done_cb (NMAuthChain *chain, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GError *error = NULL; NMAuthCallResult result; NMActiveConnection *active; @@ -5616,7 +5615,7 @@ deactivate_net_auth_done_cb (NMAuthChain *chain, nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); path = nm_auth_chain_get_data (chain, "path"); result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL); @@ -5711,7 +5710,7 @@ impl_manager_deactivate_connection (NMDBusObject *obj, goto done; } - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "path", g_strdup (active_path), g_free); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL, TRUE); @@ -6032,14 +6031,13 @@ enable_net_done_cb (NMAuthChain *chain, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMAuthCallResult result; gboolean enable; NMAuthSubject *subject; nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); enable = GPOINTER_TO_UINT (nm_auth_chain_get_data (chain, "enable")); subject = nm_auth_chain_get_subject (chain); @@ -6094,7 +6092,7 @@ impl_manager_enable (NMDBusObject *obj, goto done; } - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "enable", GUINT_TO_POINTER (enable), NULL); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_ENABLE_DISABLE_NETWORK, TRUE); @@ -6128,12 +6126,11 @@ get_permissions_done_cb (NMAuthChain *chain, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GVariantBuilder results; nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); g_variant_builder_init (&results, G_VARIANT_TYPE ("a{ss}")); @@ -6180,7 +6177,7 @@ impl_manager_get_permissions (NMDBusObject *obj, return; } - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_ENABLE_DISABLE_NETWORK, FALSE); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_SLEEP_WAKE, FALSE); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_ENABLE_DISABLE_WIFI, FALSE); @@ -6331,7 +6328,7 @@ check_connectivity_auth_done_cb (NMAuthChain *chain, ConnectivityCheckData *data; NMDevice *device; - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL); @@ -6397,7 +6394,7 @@ impl_manager_check_connectivity (NMDBusObject *obj, return; } - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL, TRUE); } @@ -6767,7 +6764,7 @@ _dbus_set_property_auth_cb (NMAuthChain *chain, g_slice_free (DBusSetPropertyHandle, handle_data); - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); result = nm_auth_chain_get_result (chain, property_info->writable.permission); if (result != NM_AUTH_CALL_RESULT_YES) { @@ -6847,7 +6844,7 @@ nm_manager_dbus_set_property_handle (NMDBusObject *obj, handle_data->export_version_id = nm_dbus_object_get_export_version_id (obj); chain = nm_auth_chain_new_subject (subject, invocation, _dbus_set_property_auth_cb, handle_data); - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_add_call_unsafe (chain, property_info->writable.permission, TRUE); return; @@ -6881,8 +6878,9 @@ checkpoint_auth_done_cb (NMAuthChain *chain, gpointer user_data) { NMManager *self = NM_MANAGER (user_data); - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - char *op, *checkpoint_path = NULL, **devices; + char *op; + char *checkpoint_path = NULL; + char **devices; NMCheckpoint *checkpoint; NMAuthCallResult result; guint32 timeout, flags; @@ -6892,7 +6890,7 @@ checkpoint_auth_done_cb (NMAuthChain *chain, guint32 add_timeout; op = nm_auth_chain_get_data (chain, "audit-op"); - priv->auth_chains = g_slist_remove (priv->auth_chains, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK); if (NM_IN_STRSET (op, NM_AUDIT_OP_CHECKPOINT_DESTROY, @@ -6971,7 +6969,7 @@ impl_manager_checkpoint_create (NMDBusObject *obj, g_variant_get (parameters, "(^aouu)", &devices, &rollback_timeout, &flags); - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "audit-op", NM_AUDIT_OP_CHECKPOINT_CREATE, NULL); nm_auth_chain_set_data (chain, "devices", devices, (GDestroyNotify) g_strfreev); nm_auth_chain_set_data (chain, "flags", GUINT_TO_POINTER (flags), NULL); @@ -7004,7 +7002,7 @@ impl_manager_checkpoint_destroy (NMDBusObject *obj, g_variant_get (parameters, "(&o)", &checkpoint_path); - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "audit-op", NM_AUDIT_OP_CHECKPOINT_DESTROY, NULL); nm_auth_chain_set_data (chain, "checkpoint_path", g_strdup (checkpoint_path), g_free); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK, TRUE); @@ -7035,7 +7033,7 @@ impl_manager_checkpoint_rollback (NMDBusObject *obj, g_variant_get (parameters, "(&o)", &checkpoint_path); - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "audit-op", NM_AUDIT_OP_CHECKPOINT_ROLLBACK, NULL); nm_auth_chain_set_data (chain, "checkpoint_path", g_strdup (checkpoint_path), g_free); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK, TRUE); @@ -7067,7 +7065,7 @@ impl_manager_checkpoint_adjust_rollback_timeout (NMDBusObject *obj, g_variant_get (parameters, "(&ou)", &checkpoint_path, &add_timeout); - priv->auth_chains = g_slist_append (priv->auth_chains, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_set_data (chain, "audit-op", NM_AUDIT_OP_CHECKPOINT_ADJUST_ROLLBACK_TIMEOUT, NULL); nm_auth_chain_set_data (chain, "checkpoint_path", g_strdup (checkpoint_path), g_free); nm_auth_chain_set_data (chain, "add_timeout", GUINT_TO_POINTER (add_timeout), NULL); @@ -7358,6 +7356,7 @@ nm_manager_init (NMManager *self) guint i; GFile *file; + c_list_init (&priv->auth_lst_head); c_list_init (&priv->link_cb_lst); c_list_init (&priv->devices_lst_head); c_list_init (&priv->active_connections_lst_head); @@ -7622,8 +7621,8 @@ dispose (GObject *object) g_slice_free (PlatformLinkCbData, data); } - g_slist_free_full (priv->auth_chains, (GDestroyNotify) nm_auth_chain_destroy); - priv->auth_chains = NULL; + while ((iter = c_list_first (&priv->auth_lst_head))) + nm_auth_chain_destroy (nm_auth_chain_parent_lst_entry (iter)); nm_clear_g_source (&priv->devices_inited_id); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 18ec00f6dd..6a14e195c0 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -114,7 +114,7 @@ typedef struct { NMConfig *config; - GSList *auths; + CList auth_lst_head; GSList *plugins; @@ -660,7 +660,6 @@ pk_add_cb (NMAuthChain *chain, gpointer user_data) { NMSettings *self = NM_SETTINGS (user_data); - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); NMAuthCallResult result; gs_free_error GError *error = NULL; NMConnection *connection = NULL; @@ -673,7 +672,7 @@ pk_add_cb (NMAuthChain *chain, nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); - priv->auths = g_slist_remove (priv->auths, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); perm = nm_auth_chain_get_data (chain, "perm"); nm_assert (perm); @@ -811,7 +810,8 @@ nm_settings_add_connection_dbus (NMSettings *self, goto done; } - priv->auths = g_slist_append (priv->auths, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); + nm_auth_chain_set_data (chain, "perm", (gpointer) perm, NULL); nm_auth_chain_set_data (chain, "connection", g_object_ref (connection), g_object_unref); nm_auth_chain_set_data (chain, "callback", callback, NULL); @@ -1467,7 +1467,7 @@ pk_hostname_cb (NMAuthChain *chain, nm_assert (G_IS_DBUS_METHOD_INVOCATION (context)); - priv->auths = g_slist_remove (priv->auths, chain); + c_list_unlink (nm_auth_chain_parent_lst_list (chain)); result = nm_auth_chain_get_result (chain, NM_AUTH_PERMISSION_SETTINGS_MODIFY_HOSTNAME); @@ -1526,7 +1526,7 @@ impl_settings_save_hostname (NMDBusObject *obj, return; } - priv->auths = g_slist_append (priv->auths, chain); + c_list_link_tail (&priv->auth_lst_head, nm_auth_chain_parent_lst_list (chain)); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_SETTINGS_MODIFY_HOSTNAME, TRUE); nm_auth_chain_set_data (chain, "hostname", g_strdup (hostname), g_free); } @@ -1947,6 +1947,7 @@ nm_settings_init (NMSettings *self) { NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); + c_list_init (&priv->auth_lst_head); c_list_init (&priv->connections_lst_head); priv->agent_mgr = g_object_ref (nm_agent_manager_get ()); @@ -1964,11 +1965,12 @@ dispose (GObject *object) { NMSettings *self = NM_SETTINGS (object); NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); + CList *iter; g_clear_object (&priv->startup_complete_blocked_by); - g_slist_free_full (priv->auths, (GDestroyNotify) nm_auth_chain_destroy); - priv->auths = NULL; + while ((iter = c_list_first (&priv->auth_lst_head))) + nm_auth_chain_destroy (nm_auth_chain_parent_lst_entry (iter)); if (priv->hostname_manager) { g_signal_handlers_disconnect_by_func (priv->hostname_manager, From 25de86abb62488b1a88b7d34f05676a2a901515f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 26 May 2019 18:58:12 +0200 Subject: [PATCH 14/19] manager: cleanup freeing CList in NMManager's dispose() To unlink all elements, I find a while() loop easier to read than c_list_for_each_*safe(). --- src/nm-manager.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 949361f427..8960079b11 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -7605,19 +7605,18 @@ dispose (GObject *object) { NMManager *self = NM_MANAGER (object); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - CList *iter, *iter_safe; - NMActiveConnection *ac, *ac_safe; + CList *iter; nm_assert (c_list_is_empty (&priv->async_op_lst_head)); g_signal_handlers_disconnect_by_func (priv->platform, G_CALLBACK (platform_link_cb), self); - c_list_for_each_safe (iter, iter_safe, &priv->link_cb_lst) { + while ((iter = c_list_first (&priv->link_cb_lst))) { PlatformLinkCbData *data = c_list_entry (iter, PlatformLinkCbData, lst); g_source_remove (data->idle_id); - c_list_unlink_stale (iter); + c_list_unlink_stale (&data->lst); g_slice_free (PlatformLinkCbData, data); } @@ -7646,8 +7645,9 @@ dispose (GObject *object) nm_clear_g_source (&priv->ac_cleanup_id); - c_list_for_each_entry_safe (ac, ac_safe, &priv->active_connections_lst_head, active_connections_lst) - active_connection_remove (self, ac); + while ((iter = c_list_first (&priv->active_connections_lst_head))) + active_connection_remove (self, c_list_entry (iter, NMActiveConnection, active_connections_lst)); + nm_assert (c_list_is_empty (&priv->active_connections_lst_head)); g_clear_object (&priv->primary_connection); g_clear_object (&priv->activating_connection); From ceaf64eee7c2e7e9cf596bbd601420557dd8feee Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 26 May 2019 23:39:25 +0200 Subject: [PATCH 15/19] settings,libnm: move is-adhoc-wpa check to libnm "nm-settings.c" is complex enough. Move this trivial helper function to libnm-core. --- libnm-core/nm-core-internal.h | 2 ++ libnm-core/nm-utils.c | 27 ++++++++++++++++++++++++++ src/settings/nm-settings.c | 36 ++--------------------------------- 3 files changed, 31 insertions(+), 34 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 406b170e15..d369a75949 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -815,4 +815,6 @@ void _nm_bridge_vlan_str_append_rest (const NMBridgeVlan *vlan, GString *string, gboolean leading_space); +gboolean nm_utils_connection_is_adhoc_wpa (NMConnection *connection); + #endif diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 31ef2978a1..44b1703e11 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -6049,3 +6049,30 @@ _nm_utils_bridge_vlan_verify_list (GPtrArray *vlans, return TRUE; } + +gboolean +nm_utils_connection_is_adhoc_wpa (NMConnection *connection) +{ + NMSettingWireless *s_wifi; + NMSettingWirelessSecurity *s_wsec; + const char *key_mgmt; + const char *mode; + + s_wifi = nm_connection_get_setting_wireless (connection); + if (!s_wifi) + return FALSE; + + mode = nm_setting_wireless_get_mode (s_wifi); + if (!nm_streq0 (mode, NM_SETTING_WIRELESS_MODE_ADHOC)) + return FALSE; + + s_wsec = nm_connection_get_setting_wireless_security (connection); + if (!s_wsec) + return FALSE; + + key_mgmt = nm_setting_wireless_security_get_key_mgmt (s_wsec); + if (!nm_streq0 (key_mgmt, "wpa-none")) + return FALSE; + + return TRUE; +} diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 6a14e195c0..0bd0f05cc3 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -711,38 +711,6 @@ pk_add_cb (NMAuthChain *chain, send_agent_owned_secrets (self, added, subject); } -/* FIXME: remove if/when kernel supports adhoc wpa */ -static gboolean -is_adhoc_wpa (NMConnection *connection) -{ - NMSettingWireless *s_wifi; - NMSettingWirelessSecurity *s_wsec; - const char *mode, *key_mgmt; - - /* The kernel doesn't support Ad-Hoc WPA connections well at this time, - * and turns them into open networks. It's been this way since at least - * 2.6.30 or so; until that's fixed, disable WPA-protected Ad-Hoc networks. - */ - - s_wifi = nm_connection_get_setting_wireless (connection); - if (!s_wifi) - return FALSE; - - mode = nm_setting_wireless_get_mode (s_wifi); - if (g_strcmp0 (mode, NM_SETTING_WIRELESS_MODE_ADHOC) != 0) - return FALSE; - - s_wsec = nm_connection_get_setting_wireless_security (connection); - if (!s_wsec) - return FALSE; - - key_mgmt = nm_setting_wireless_security_get_key_mgmt (s_wsec); - if (g_strcmp0 (key_mgmt, "wpa-none") != 0) - return FALSE; - - return TRUE; -} - void nm_settings_add_connection_dbus (NMSettings *self, NMConnection *connection, @@ -772,11 +740,11 @@ nm_settings_add_connection_dbus (NMSettings *self, goto done; } - /* The kernel doesn't support Ad-Hoc WPA connections well at this time, + /* FIXME: The kernel doesn't support Ad-Hoc WPA connections well at this time, * and turns them into open networks. It's been this way since at least * 2.6.30 or so; until that's fixed, disable WPA-protected Ad-Hoc networks. */ - if (is_adhoc_wpa (connection)) { + if (nm_utils_connection_is_adhoc_wpa (connection)) { error = g_error_new_literal (NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "WPA Ad-Hoc disabled due to kernel bugs"); From d92356c8683b18fc80e3a15b79878ef0828300f1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 27 May 2019 16:20:53 +0200 Subject: [PATCH 16/19] settings: use _nm_utils_slist_to_strv() for NMSettings:unmanaged-specs property getter Note that now the empty list will be represented as %NULL instead of an empty strv array. That makes no difference in pratice. The main use of this property is as glue for NMDBusManager to expose the property on D-Bus. Thereby it uses g_dbus_gvalue_to_gvariant() which handles %NULL just fine. --- src/settings/nm-settings.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 0bd0f05cc3..d42d439ff3 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1865,26 +1865,19 @@ get_property (GObject *object, guint prop_id, { NMSettings *self = NM_SETTINGS (object); NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - const GSList *specs, *iter; - guint i; - char **strvs; const char **strv; switch (prop_id) { case PROP_UNMANAGED_SPECS: - specs = nm_settings_get_unmanaged_specs (self); - strvs = g_new (char *, g_slist_length ((GSList *) specs) + 1); - i = 0; - for (iter = specs; iter; iter = iter->next) - strvs[i++] = g_strdup (iter->data); - strvs[i] = NULL; - g_value_take_boxed (value, strvs); + g_value_take_boxed (value, + _nm_utils_slist_to_strv (nm_settings_get_unmanaged_specs (self), + TRUE)); break; case PROP_HOSTNAME: g_value_set_string (value, - priv->hostname_manager - ? nm_hostname_manager_get_hostname (priv->hostname_manager) - : NULL); + priv->hostname_manager + ? nm_hostname_manager_get_hostname (priv->hostname_manager) + : NULL); break; case PROP_CAN_MODIFY: g_value_set_boolean (value, TRUE); From 31382c9727546af6f9d0a4c05b1cf80e631207f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 May 2019 13:30:36 +0200 Subject: [PATCH 17/19] settings: remove unused NMSettingsConnection.supports_secrets() function --- src/settings/nm-settings-connection.c | 9 --------- src/settings/nm-settings-connection.h | 3 --- .../plugins/ifupdown/nms-ifupdown-connection.c | 14 -------------- 3 files changed, 26 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 2fbd158353..e684aab6ee 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -755,13 +755,6 @@ _get_secrets_info_free (NMSettingsConnectionCallId *call_id) g_slice_free (NMSettingsConnectionCallId, call_id); } -static gboolean -supports_secrets (NMSettingsConnection *self, const char *setting_name) -{ - /* All secrets supported */ - return TRUE; -} - typedef struct { NMSettingSecretFlags required; NMSettingSecretFlags forbidden; @@ -2998,8 +2991,6 @@ nm_settings_connection_class_init (NMSettingsConnectionClass *klass) object_class->get_property = get_property; object_class->set_property = set_property; - klass->supports_secrets = supports_secrets; - obj_properties[PROP_UNSAVED] = g_param_spec_boolean (NM_SETTINGS_CONNECTION_UNSAVED, "", "", FALSE, diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index b9eb881cf9..ebed798c31 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -131,9 +131,6 @@ struct _NMSettingsConnectionClass { gboolean (*delete) (NMSettingsConnection *self, GError **error); - - gboolean (*supports_secrets) (NMSettingsConnection *self, - const char *setting_name); }; GType nm_settings_connection_get_type (void); diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-connection.c b/src/settings/plugins/ifupdown/nms-ifupdown-connection.c index cb54b5092c..13187c4454 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-connection.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-connection.c @@ -57,16 +57,6 @@ G_DEFINE_TYPE (NMIfupdownConnection, nm_ifupdown_connection, NM_TYPE_SETTINGS_CO /*****************************************************************************/ -static gboolean -supports_secrets (NMSettingsConnection *connection, const char *setting_name) -{ - _LOGI ("supports_secrets() for setting_name: '%s'", setting_name); - - return (strcmp (setting_name, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME) == 0); -} - -/*****************************************************************************/ - static void nm_ifupdown_connection_init (NMIfupdownConnection *connection) { @@ -98,8 +88,4 @@ nm_ifupdown_connection_new (if_block *block) static void nm_ifupdown_connection_class_init (NMIfupdownConnectionClass *ifupdown_connection_class) { - NMSettingsConnectionClass *connection_class = NM_SETTINGS_CONNECTION_CLASS (ifupdown_connection_class); - - connection_class->supports_secrets = supports_secrets; } - From 1a398421ffc35ce0656a32e5c39801e52c92544a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 29 May 2019 17:10:51 +0200 Subject: [PATCH 18/19] libnm: add nmtst_connection_assert_unchanging() helper --- libnm-core/nm-connection.c | 75 ++++++++++++++++++++++++++--------- libnm-core/nm-core-internal.h | 9 +++++ 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 1a88072400..4e4a835cbf 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -1689,6 +1689,43 @@ nm_connection_normalize (NMConnection *connection, return TRUE; } +/*****************************************************************************/ + +#if NM_MORE_ASSERTS +static void +_nmtst_connection_unchanging_changed_cb (NMConnection *connection, gpointer user_data) +{ + nm_assert_not_reached (); +} + +static void +_nmtst_connection_unchanging_secrets_updated_cb (NMConnection *connection, const char *setting_name, gpointer user_data) +{ + nm_assert_not_reached (); +} + +void +nmtst_connection_assert_unchanging (NMConnection *connection) +{ + nm_assert (NM_IS_CONNECTION (connection)); + + g_signal_connect (connection, + NM_CONNECTION_CHANGED, + G_CALLBACK (_nmtst_connection_unchanging_changed_cb), + NULL); + g_signal_connect (connection, + NM_CONNECTION_SECRETS_CLEARED, + G_CALLBACK (_nmtst_connection_unchanging_changed_cb), + NULL); + g_signal_connect (connection, + NM_CONNECTION_SECRETS_UPDATED, + G_CALLBACK (_nmtst_connection_unchanging_secrets_updated_cb), + NULL); +} +#endif + +/*****************************************************************************/ + /** * nm_connection_update_secrets: * @connection: the #NMConnection @@ -3158,13 +3195,13 @@ nm_connection_default_init (NMConnectionInterface *iface) */ signals[SECRETS_UPDATED] = g_signal_new (NM_CONNECTION_SECRETS_UPDATED, - NM_TYPE_CONNECTION, - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMConnectionInterface, secrets_updated), - NULL, NULL, - g_cclosure_marshal_VOID__STRING, - G_TYPE_NONE, 1, - G_TYPE_STRING); + NM_TYPE_CONNECTION, + G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET (NMConnectionInterface, secrets_updated), + NULL, NULL, + g_cclosure_marshal_VOID__STRING, + G_TYPE_NONE, 1, + G_TYPE_STRING); /** * NMConnection::secrets-cleared: @@ -3175,12 +3212,12 @@ nm_connection_default_init (NMConnectionInterface *iface) */ signals[SECRETS_CLEARED] = g_signal_new (NM_CONNECTION_SECRETS_CLEARED, - NM_TYPE_CONNECTION, - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMConnectionInterface, secrets_cleared), - NULL, NULL, - g_cclosure_marshal_VOID__VOID, - G_TYPE_NONE, 0); + NM_TYPE_CONNECTION, + G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET (NMConnectionInterface, secrets_cleared), + NULL, NULL, + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, 0); /** * NMConnection::changed: @@ -3192,10 +3229,10 @@ nm_connection_default_init (NMConnectionInterface *iface) */ signals[CHANGED] = g_signal_new (NM_CONNECTION_CHANGED, - NM_TYPE_CONNECTION, - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMConnectionInterface, changed), - NULL, NULL, - g_cclosure_marshal_VOID__VOID, - G_TYPE_NONE, 0); + NM_TYPE_CONNECTION, + G_SIGNAL_RUN_FIRST, + G_STRUCT_OFFSET (NMConnectionInterface, changed), + NULL, NULL, + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, 0); } diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index d369a75949..7e55b1fc3c 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -203,6 +203,15 @@ NMSettingVerifyResult _nm_connection_verify (NMConnection *connection, GError ** gboolean _nm_connection_remove_setting (NMConnection *connection, GType setting_type); +#if NM_MORE_ASSERTS +void nmtst_connection_assert_unchanging (NMConnection *connection); +#else +static inline void +nmtst_connection_assert_unchanging (NMConnection *connection) +{ +} +#endif + NMConnection *_nm_simple_connection_new_from_dbus (GVariant *dict, NMSettingParseFlags parse_flags, GError **error); From 954906e3d1405d1a77f4c2cad8ccdecf0598b209 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 1 Jun 2019 11:39:34 +0200 Subject: [PATCH 19/19] libnm: add _nm_connection_ensure_normalized() helper --- libnm-core/nm-connection.c | 54 +++++++++++++++++++++++++++++++++++ libnm-core/nm-core-internal.h | 6 ++++ 2 files changed, 60 insertions(+) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 4e4a835cbf..f986f824e3 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -1689,6 +1689,60 @@ nm_connection_normalize (NMConnection *connection, return TRUE; } +gboolean +_nm_connection_ensure_normalized (NMConnection *connection, + gboolean allow_modify, + const char *enforce_uuid, + NMConnection **out_connection_clone, + GError **error) +{ + gs_unref_object NMConnection *connection_clone = NULL; + gs_free_error GError *local = NULL; + NMSettingVerifyResult vresult; + + nm_assert (NM_IS_CONNECTION (connection)); + nm_assert (out_connection_clone && !*out_connection_clone); + nm_assert (!enforce_uuid || nm_utils_is_uuid (enforce_uuid)); + + vresult = _nm_connection_verify (connection, &local); + if (vresult != NM_SETTING_VERIFY_SUCCESS) { + if (!NM_IN_SET (vresult, NM_SETTING_VERIFY_NORMALIZABLE, + NM_SETTING_VERIFY_NORMALIZABLE_ERROR)) { + g_propagate_error (error, g_steal_pointer (&local)); + return FALSE; + } + if (!allow_modify) { + connection_clone = nm_simple_connection_new_clone (connection); + connection = connection_clone; + } + if (!nm_connection_normalize (connection, NULL, NULL, error)) { + nm_assert_not_reached (); + return FALSE; + } + } + + if (enforce_uuid) { + if (!nm_streq (enforce_uuid, nm_connection_get_uuid (connection))) { + NMSettingConnection *s_con; + + if ( !allow_modify + && !connection_clone) { + connection_clone = nm_simple_connection_new_clone (connection); + connection = connection_clone; + } + s_con = nm_connection_get_setting_connection (connection); + g_object_set (s_con, + NM_SETTING_CONNECTION_UUID, + enforce_uuid, + NULL); + } + } + + if (connection_clone) + *out_connection_clone = g_steal_pointer (&connection_clone); + return TRUE; +} + /*****************************************************************************/ #if NM_MORE_ASSERTS diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 7e55b1fc3c..aed25521cb 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -201,6 +201,12 @@ typedef enum { NMSettingVerifyResult _nm_connection_verify (NMConnection *connection, GError **error); +gboolean _nm_connection_ensure_normalized (NMConnection *connection, + gboolean allow_modify, + const char *enforce_uuid, + NMConnection **out_connection_clone, + GError **error); + gboolean _nm_connection_remove_setting (NMConnection *connection, GType setting_type); #if NM_MORE_ASSERTS