From 0b20b44c99185b7a8d80385579c9b66d2c10170a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 20 Jan 2019 13:49:08 +0100 Subject: [PATCH 01/11] libnm: add _nm_setting_emit_property_changed() function Will be used next. --- libnm-core/nm-setting-private.h | 2 ++ libnm-core/nm-setting.c | 31 ++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 8f61b0ff9d..1db3db9980 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -39,6 +39,8 @@ int _nm_setting_compare_priority (gconstpointer a, gconstpointer b); /*****************************************************************************/ +void _nm_setting_emit_property_changed (NMSetting *setting); + typedef enum NMSettingUpdateSecretResult { NM_SETTING_UPDATE_SECRET_ERROR = FALSE, NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED = TRUE, diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index a62e31e80d..73f94a9628 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -67,7 +67,7 @@ typedef struct { NMSettingPriority priority; } SettingInfo; -NM_GOBJECT_PROPERTIES_DEFINE_BASE ( +NM_GOBJECT_PROPERTIES_DEFINE (NMSetting, PROP_NAME, ); @@ -553,6 +553,35 @@ _nm_setting_class_get_sett_info (NMSettingClass *setting_class) /*****************************************************************************/ +void +_nm_setting_emit_property_changed (NMSetting *setting) +{ + /* Some settings have "properties" that are not implemented as GObject properties. + * + * For example: + * + * - gendata-base settings like NMSettingEthtool. Here properties are just + * GVariant values in the gendata hash. + * + * - NMSettingWireGuard's peers are not backed by a GObject property. Instead + * there is C-API to access/modify peers. + * + * We still want to emit property-changed notifications for such properties, + * in particular because NMConnection registers to such signals to re-emit + * it as NM_CONNECTION_CHANGED signal. In fact, there are unlikely any other + * uses of such a property-changed signal, because generally it doesn't make + * too much sense. + * + * So, instead of adding yet another (artificial) signal "setting-changed", + * hijack the "notify" signal and just notify about changes of the "name". + * Of course, the "name" doesn't really ever change, because it's tied to + * the GObject's type. + */ + _notify (setting, PROP_NAME); +} + +/*****************************************************************************/ + gboolean _nm_setting_use_legacy_property (NMSetting *setting, GVariant *connection_dict, From 879820ccd54be34deda42791f45ab2b284281425 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 20 Jan 2019 14:01:24 +0100 Subject: [PATCH 02/11] libnm: emit "notify:name" signal when changing gendata property (NMSettingEthtool) We want to emit a change notification when gendata-based settings (like NMSettingEthtool) change. But instead of adding a separate signal, just emit a fake "notify:name" notification. --- libnm-core/nm-setting.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 73f94a9628..b26ab6b3d2 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1111,6 +1111,8 @@ duplicate_copy_properties (const NMSettInfoSetting *sett_info, if (sett_info->detail.gendata_info) { GenData *gendata = _gendata_hash (src, FALSE); + nm_assert (!_gendata_hash (dst, FALSE)); + if ( gendata && g_hash_table_size (gendata->hash) > 0) { GHashTableIter iter; @@ -2322,7 +2324,7 @@ _nm_setting_gendata_notify (NMSetting *setting, gendata = _gendata_hash (setting, FALSE); if (!gendata) - return; + goto out; nm_clear_g_free (&gendata->values); @@ -2332,7 +2334,7 @@ _nm_setting_gendata_notify (NMSetting *setting, nm_clear_g_free (&gendata->names); } - /* Note, that currently there is now way to notify the subclass when gendata changed. + /* Note, currently there is no way to notify the subclass when gendata changed. * gendata is only changed in two situations: * 1) from within NMSetting itself, for example when creating a NMSetting instance * from keyfile or a D-Bus GVariant. @@ -2345,6 +2347,9 @@ _nm_setting_gendata_notify (NMSetting *setting, * * If we ever need it, then we would need to call a virtual function to notify the subclass * that gendata changed. */ + +out: + _nm_setting_emit_property_changed (setting); } GVariant * @@ -2467,7 +2472,7 @@ nm_setting_gendata_get_all_values (NMSetting *setting) void _nm_setting_gendata_to_gvalue (NMSetting *setting, - GValue *value) + GValue *value) { GenData *gendata; GHashTable *new; From 52368678d67d7d11ef2c552b6ef1865ba012c0e7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 21 Jan 2019 08:10:14 +0100 Subject: [PATCH 03/11] libnm: extend nm_setting_enumerate_values() to support non-GObject base properties While nm_setting_enumerate_values() should not be used anymore, still extend it to make it workable also for properties that are not based on GObject properties. --- libnm-core/nm-core-internal.h | 7 ++++--- libnm-core/nm-setting.c | 36 +++++++++++++++++++++++--------- libnm-core/nm-setting.h | 39 +++++++++++++++++++++-------------- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index b3d664dcea..9385d505e7 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -614,7 +614,8 @@ gboolean _nm_setting_sriov_sort_vfs (NMSettingSriov *setting); /*****************************************************************************/ -typedef struct _NMSettInfoSetting NMSettInfoSetting; +typedef struct _NMSettInfoSetting NMSettInfoSetting; +typedef struct _NMSettInfoProperty NMSettInfoProperty; typedef GVariant *(*NMSettingPropertyGetFunc) (NMSetting *setting, const char *property); @@ -638,7 +639,7 @@ typedef GVariant *(*NMSettingPropertyTransformToFunc) (const GValue *from); typedef void (*NMSettingPropertyTransformFromFunc) (GVariant *from, GValue *to); -typedef struct { +struct _NMSettInfoProperty { const char *name; GParamSpec *param_spec; const GVariantType *dbus_type; @@ -650,7 +651,7 @@ typedef struct { NMSettingPropertyTransformToFunc to_dbus; NMSettingPropertyTransformFromFunc from_dbus; -} NMSettInfoProperty; +}; typedef struct { const GVariantType *(*get_variant_type) (const struct _NMSettInfoSetting *sett_info, diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index b26ab6b3d2..bed3a3cf16 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1729,6 +1729,27 @@ nm_setting_diff (NMSetting *a, } } +static void +enumerate_values (const NMSettInfoProperty *property_info, + NMSetting *setting, + NMSettingValueIterFn func, + gpointer user_data) +{ + GValue value = G_VALUE_INIT; + + if (!property_info->param_spec) + return; + + g_value_init (&value, G_PARAM_SPEC_VALUE_TYPE (property_info->param_spec)); + g_object_get_property (G_OBJECT (setting), property_info->param_spec->name, &value); + func (setting, + property_info->param_spec->name, + &value, + property_info->param_spec->flags, + user_data); + g_value_unset (&value); +} + /** * nm_setting_enumerate_values: * @setting: the #NMSetting @@ -1784,16 +1805,10 @@ nm_setting_enumerate_values (NMSetting *setting, } for (i = 0; i < sett_info->property_infos_len; i++) { - GParamSpec *prop_spec = _nm_sett_info_property_info_get_sorted (sett_info, i)->param_spec; - GValue value = G_VALUE_INIT; - - if (!prop_spec) - continue; - - g_value_init (&value, G_PARAM_SPEC_VALUE_TYPE (prop_spec)); - g_object_get_property (G_OBJECT (setting), prop_spec->name, &value); - func (setting, prop_spec->name, &value, prop_spec->flags, user_data); - g_value_unset (&value); + NM_SETTING_GET_CLASS (setting)->enumerate_values (_nm_sett_info_property_info_get_sorted (sett_info, i), + setting, + func, + user_data); } } @@ -2606,6 +2621,7 @@ nm_setting_class_init (NMSettingClass *setting_class) setting_class->compare_property = compare_property; setting_class->clear_secrets = clear_secrets; setting_class->duplicate_copy_properties = duplicate_copy_properties; + setting_class->enumerate_values = enumerate_values; /** * NMSetting:name: diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index 0f0ac8f382..0002b80ae6 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -170,6 +170,22 @@ typedef gboolean (*NMSettingClearSecretsWithFlagsFn) (NMSetting *setting, struct _NMMetaSettingInfo; struct _NMSettInfoSetting; +struct _NMSettInfoProperty; + +/** + * NMSettingValueIterFn: + * @setting: The setting for which properties are being iterated, given to + * nm_setting_enumerate_values() + * @key: The value/property name + * @value: The property's value + * @flags: The property's flags, like %NM_SETTING_PARAM_SECRET + * @user_data: User data passed to nm_setting_enumerate_values() + */ +typedef void (*NMSettingValueIterFn) (NMSetting *setting, + const char *key, + const GValue *value, + GParamFlags flags, + gpointer user_data); typedef struct { GObjectClass parent; @@ -225,28 +241,19 @@ typedef struct { NMSetting *src, NMSetting *dst); + /*< private >*/ + void (*enumerate_values) (const struct _NMSettInfoProperty *property_info, + NMSetting *setting, + NMSettingValueIterFn func, + gpointer user_data); + /*< private >*/ const struct _NMMetaSettingInfo *setting_info; /*< private >*/ - gpointer padding[5]; + gpointer padding[4]; } NMSettingClass; -/** - * NMSettingValueIterFn: - * @setting: The setting for which properties are being iterated, given to - * nm_setting_enumerate_values() - * @key: The value/property name - * @value: The property's value - * @flags: The property's flags, like %NM_SETTING_PARAM_SECRET - * @user_data: User data passed to nm_setting_enumerate_values() - */ -typedef void (*NMSettingValueIterFn) (NMSetting *setting, - const char *key, - const GValue *value, - GParamFlags flags, - gpointer user_data); - GType nm_setting_get_type (void); GType nm_setting_lookup_type (const char *name); From b64e24dcd72ed264c3b3a7fa0c6f49e7dabafdfe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 21 Jan 2019 08:46:41 +0100 Subject: [PATCH 04/11] libnm: rework _nm_setting_aggregate() to delegate to setting class Instead of special-casing the aggregate implementation for NMSettingVpn, delegate to a virtual function. This will also work with other settings, that have properties/secrets that are not GObject based properties. --- libnm-core/nm-setting-private.h | 3 -- libnm-core/nm-setting-vpn.c | 16 ++++----- libnm-core/nm-setting.c | 64 +++++++++++++++++++-------------- libnm-core/nm-setting.h | 7 +++- 4 files changed, 50 insertions(+), 40 deletions(-) diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index 1db3db9980..2af74ec4ce 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -101,9 +101,6 @@ gboolean _nm_setting_verify_secret_string (const char *str, gboolean _nm_setting_aggregate (NMSetting *setting, NMConnectionAggregateType type, gpointer arg); -gboolean _nm_setting_vpn_aggregate (NMSettingVpn *setting, - NMConnectionAggregateType type, - gpointer arg); gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type); diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c index 1371140ec4..133620b7d4 100644 --- a/libnm-core/nm-setting-vpn.c +++ b/libnm-core/nm-setting-vpn.c @@ -449,20 +449,17 @@ nm_setting_vpn_foreach_secret (NMSettingVpn *setting, foreach_item_helper (setting, TRUE, func, user_data); } -gboolean -_nm_setting_vpn_aggregate (NMSettingVpn *setting, - NMConnectionAggregateType type, - gpointer arg) +static gboolean +aggregate (NMSetting *setting, + int type_i, + gpointer arg) { - NMSettingVpnPrivate *priv; + NMSettingVpnPrivate *priv = NM_SETTING_VPN_GET_PRIVATE (setting); + NMConnectionAggregateType type = type_i; NMSettingSecretFlags secret_flags; const char *key_name; GHashTableIter iter; - g_return_val_if_fail (NM_IS_SETTING_VPN (setting), FALSE); - - priv = NM_SETTING_VPN_GET_PRIVATE (setting); - switch (type) { case NM_CONNECTION_AGGREGATE_ANY_SECRETS: @@ -984,6 +981,7 @@ nm_setting_vpn_class_init (NMSettingVpnClass *klass) setting_class->need_secrets = need_secrets; setting_class->compare_property = compare_property; setting_class->clear_secrets = clear_secrets; + setting_class->aggregate = aggregate; /** * NMSettingVpn:service-type: diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index bed3a3cf16..c679e89d39 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -1812,37 +1812,17 @@ nm_setting_enumerate_values (NMSetting *setting, } } -/** - * _nm_setting_aggregate: - * @setting: the #NMSetting to aggregate. - * @type: the #NMConnectionAggregateType aggregate type. - * @arg: the in/out arguments for aggregation. They depend on @type. - * - * This is the implementation detail of _nm_connection_aggregate(). It - * makes no sense to call this function directly outside of _nm_connection_aggregate(). - * - * Returns: %TRUE if afterwards the aggregation is complete. That means, - * the only caller _nm_connection_aggregate() will not visit other settings - * after a setting returns %TRUE (indicating that there is nothing further - * to aggregate). Note that is very different from the boolean return - * argument of _nm_connection_aggregate(), which serves a different purpose. - */ -gboolean -_nm_setting_aggregate (NMSetting *setting, - NMConnectionAggregateType type, - gpointer arg) +static gboolean +aggregate (NMSetting *setting, + int type_i, + gpointer arg) { + NMConnectionAggregateType type = type_i; const NMSettInfoSetting *sett_info; guint i; - g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); - g_return_val_if_fail (arg, FALSE); - g_return_val_if_fail (NM_IN_SET (type, NM_CONNECTION_AGGREGATE_ANY_SECRETS, - NM_CONNECTION_AGGREGATE_ANY_SYSTEM_SECRET_FLAGS), - FALSE); - - if (NM_IS_SETTING_VPN (setting)) - return _nm_setting_vpn_aggregate (NM_SETTING_VPN (setting), type, arg); + nm_assert (NM_IN_SET (type, NM_CONNECTION_AGGREGATE_ANY_SECRETS, + NM_CONNECTION_AGGREGATE_ANY_SYSTEM_SECRET_FLAGS)); sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting)); for (i = 0; i < sett_info->property_infos_len; i++) { @@ -1886,6 +1866,35 @@ _nm_setting_aggregate (NMSetting *setting, return FALSE; } +/** + * _nm_setting_aggregate: + * @setting: the #NMSetting to aggregate. + * @type: the #NMConnectionAggregateType aggregate type. + * @arg: the in/out arguments for aggregation. They depend on @type. + * + * This is the implementation detail of _nm_connection_aggregate(). It + * makes no sense to call this function directly outside of _nm_connection_aggregate(). + * + * Returns: %TRUE if afterwards the aggregation is complete. That means, + * the only caller _nm_connection_aggregate() will not visit other settings + * after a setting returns %TRUE (indicating that there is nothing further + * to aggregate). Note that is very different from the boolean return + * argument of _nm_connection_aggregate(), which serves a different purpose. + */ +gboolean +_nm_setting_aggregate (NMSetting *setting, + NMConnectionAggregateType type, + gpointer arg) +{ + g_return_val_if_fail (NM_IS_SETTING (setting), FALSE); + g_return_val_if_fail (arg, FALSE); + g_return_val_if_fail (NM_IN_SET (type, NM_CONNECTION_AGGREGATE_ANY_SECRETS, + NM_CONNECTION_AGGREGATE_ANY_SYSTEM_SECRET_FLAGS), + FALSE); + + return NM_SETTING_GET_CLASS (setting)->aggregate (setting, type, arg); +} + static gboolean clear_secrets (const NMSettInfoSetting *sett_info, guint property_idx, @@ -2622,6 +2631,7 @@ nm_setting_class_init (NMSettingClass *setting_class) setting_class->clear_secrets = clear_secrets; setting_class->duplicate_copy_properties = duplicate_copy_properties; setting_class->enumerate_values = enumerate_values; + setting_class->aggregate = aggregate; /** * NMSetting:name: diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index 0002b80ae6..96868731eb 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -247,11 +247,16 @@ typedef struct { NMSettingValueIterFn func, gpointer user_data); + /*< private >*/ + gboolean (*aggregate) (NMSetting *setting, + int type_i, + gpointer arg); + /*< private >*/ const struct _NMMetaSettingInfo *setting_info; /*< private >*/ - gpointer padding[4]; + gpointer padding[3]; } NMSettingClass; GType nm_setting_get_type (void); From cabc1ddca54c004f3642df707d73e9f3fd9466cc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 09:06:14 +0100 Subject: [PATCH 05/11] settings: fix leaking variant in for_each_secret() Fixes: df6706813a698e7a697739b0940bd8f528713aab --- src/settings/nm-settings-connection.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index e655a11937..c00cba4509 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -288,6 +288,7 @@ for_each_secret (NMConnection *self, if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { if (!remove_non_secrets) g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); + g_variant_unref (val); continue; } if (callback (secret_flags, callback_data)) From 5eac241578656148897b5c3dfc76cadb8ca95d4a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 10:02:45 +0100 Subject: [PATCH 06/11] settings: fix for_each_secret() to check variant type of VPN secrets We cannot just blindly assume that the variant is of the right type to iterate over it. --- src/settings/nm-settings-connection.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index c00cba4509..68efe8d31c 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -268,6 +268,13 @@ for_each_secret (NMConnection *self, GVariantIter vpn_secrets_iter; const char *vpn_secret_name, *secret; + if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("a{ss}"))) { + /* invalid type. Silently ignore the secrets as we cannot find out the + * secret-flags. */ + g_variant_unref (val); + continue; + } + /* Iterate through each secret from the VPN dict in the overall secrets dict */ g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}")); g_variant_iter_init (&vpn_secrets_iter, val); From 4ea6c83e9b94dddbf7a86072df4b2c0abb303859 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 10:04:43 +0100 Subject: [PATCH 07/11] settings: fix handling of VPN secrets in for_each_secret() nm_setting_get_secret_flags() looks whether we have a suitable "-flags" data value, or whether we have a secret with that name. In fact, we know this is a valid secret-name. Even if there are no secret-flags and the secret (currently) does not exists. We shall not care about the return value. Note that nm_setting_get_secret_flags() also for non-secrets will set the flags to "NONE", which is just what we need. --- src/settings/nm-settings-connection.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 68efe8d31c..9ea3253733 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -279,11 +279,12 @@ for_each_secret (NMConnection *self, g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}")); g_variant_iter_init (&vpn_secrets_iter, val); while (g_variant_iter_next (&vpn_secrets_iter, "{&s&s}", &vpn_secret_name, &secret)) { - if (!nm_setting_get_secret_flags (setting, vpn_secret_name, &secret_flags, NULL)) { - if (!remove_non_secrets) - g_variant_builder_add (&vpn_secrets_builder, "{ss}", vpn_secret_name, secret); - continue; - } + + /* we ignore the return value of get_secret_flags. The function may determine + * that this is not a secret, based on having not secret-flags and no secrets. + * But we have the secret at hand. We know it would be a valid secret, if we + * only would add it to the VPN settings. */ + nm_setting_get_secret_flags (setting, vpn_secret_name, &secret_flags, NULL); if (callback (secret_flags, callback_data)) g_variant_builder_add (&vpn_secrets_builder, "{ss}", vpn_secret_name, secret); From 79a0238c5eb857fd4da3cc08df42ae9c716e622c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 08:53:32 +0100 Subject: [PATCH 08/11] libnm,core: move _nm_connection_for_each_secret() from core to libnm-core _nm_connection_for_each_secret() (formerly for_each_secret()) and _nm_connection_find_secret() (formerly find_secret()) operate on a GVariant of secrets. For that, they implement certain assumptions of how to handle secrets. For example, it must special-case VPN settings, because there is no generic abstraction to handle regular secret and VPN secrets the same. Such special casing should only be done in libnm-core, at one place. Move the code to libnm-core as internal API. --- libnm-core/nm-connection.c | 141 ++++++++++++++++++++++++ libnm-core/nm-core-internal.h | 20 ++++ src/settings/nm-settings-connection.c | 150 +------------------------- 3 files changed, 164 insertions(+), 147 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index b2c109f874..1e38029dd8 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -1873,6 +1873,147 @@ nm_connection_clear_secrets_with_flags (NMConnection *connection, g_signal_emit (connection, signals[SECRETS_CLEARED], 0); } +/*****************************************************************************/ + +/* Returns always a non-NULL, non-floating variant that must + * be unrefed by the caller. */ +GVariant * +_nm_connection_for_each_secret (NMConnection *self, + GVariant *secrets, + gboolean remove_non_secrets, + NMConnectionForEachSecretFunc callback, + gpointer callback_data) +{ + GVariantBuilder secrets_builder, setting_builder; + GVariantIter secrets_iter, *setting_iter; + const char *setting_name; + + /* This function, given a dict of dicts representing new secrets of + * an NMConnection, walks through each toplevel dict (which represents a + * NMSetting), and for each setting, walks through that setting dict's + * properties. For each property that's a secret, it will check that + * secret's flags in the backing NMConnection object, and call a supplied + * callback. + * + * The one complexity is that the VPN setting's 'secrets' property is + * *also* a dict (since the key/value pairs are arbitrary and known + * only to the VPN plugin itself). That means we have three levels of + * dicts that we potentially have to traverse here. When we hit the + * VPN setting's 'secrets' property, we special-case that and iterate over + * each item in that 'secrets' dict, calling the supplied callback + * each time. + */ + + g_return_val_if_fail (callback, NULL); + + g_variant_iter_init (&secrets_iter, secrets); + g_variant_builder_init (&secrets_builder, NM_VARIANT_TYPE_CONNECTION); + while (g_variant_iter_next (&secrets_iter, "{&sa{sv}}", &setting_name, &setting_iter)) { + NMSetting *setting; + const char *secret_name; + GVariant *val; + + setting = nm_connection_get_setting_by_name (self, setting_name); + if (setting == NULL) { + g_variant_iter_free (setting_iter); + continue; + } + + g_variant_builder_init (&setting_builder, NM_VARIANT_TYPE_SETTING); + while (g_variant_iter_next (setting_iter, "{&sv}", &secret_name, &val)) { + NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + + /* VPN secrets need slightly different treatment here since the + * "secrets" property is actually a hash table of secrets. + */ + if (NM_IS_SETTING_VPN (setting) && !g_strcmp0 (secret_name, NM_SETTING_VPN_SECRETS)) { + GVariantBuilder vpn_secrets_builder; + GVariantIter vpn_secrets_iter; + const char *vpn_secret_name, *secret; + + if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("a{ss}"))) { + /* invalid type. Silently ignore the secrets as we cannot find out the + * secret-flags. */ + g_variant_unref (val); + continue; + } + + /* Iterate through each secret from the VPN dict in the overall secrets dict */ + g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}")); + g_variant_iter_init (&vpn_secrets_iter, val); + while (g_variant_iter_next (&vpn_secrets_iter, "{&s&s}", &vpn_secret_name, &secret)) { + + /* we ignore the return value of get_secret_flags. The function may determine + * that this is not a secret, based on having not secret-flags and no secrets. + * But we have the secret at hand. We know it would be a valid secret, if we + * only would add it to the VPN settings. */ + nm_setting_get_secret_flags (setting, vpn_secret_name, &secret_flags, NULL); + + if (callback (secret_flags, callback_data)) + g_variant_builder_add (&vpn_secrets_builder, "{ss}", vpn_secret_name, secret); + } + + g_variant_builder_add (&setting_builder, "{sv}", + secret_name, g_variant_builder_end (&vpn_secrets_builder)); + } else { + if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { + if (!remove_non_secrets) + g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); + g_variant_unref (val); + continue; + } + if (callback (secret_flags, callback_data)) + g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); + } + g_variant_unref (val); + } + + g_variant_iter_free (setting_iter); + g_variant_builder_add (&secrets_builder, "{sa{sv}}", setting_name, &setting_builder); + } + + return g_variant_ref_sink (g_variant_builder_end (&secrets_builder)); +} + +/*****************************************************************************/ + +typedef struct { + NMConnectionFindSecretFunc find_func; + gpointer find_func_data; + gboolean found; +} FindSecretData; + +static gboolean +find_secret_for_each_func (NMSettingSecretFlags flags, + gpointer user_data) +{ + FindSecretData *data = user_data; + + if (!data->found) + data->found = data->find_func (flags, data->find_func_data); + return FALSE; +} + +gboolean +_nm_connection_find_secret (NMConnection *self, + GVariant *secrets, + NMConnectionFindSecretFunc callback, + gpointer callback_data) +{ + FindSecretData data; + GVariant *dummy; + + data.find_func = callback; + data.find_func_data = callback_data; + data.found = FALSE; + + dummy = _nm_connection_for_each_secret (self, secrets, FALSE, find_secret_for_each_func, &data); + g_variant_unref (dummy); + return data.found; +} + +/*****************************************************************************/ + /** * nm_connection_to_dbus: * @connection: the #NMConnection diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 9385d505e7..870ec78133 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -733,4 +733,24 @@ GBytes *_nm_setting_802_1x_cert_value_to_bytes (NMSetting8021xCKScheme scheme, /*****************************************************************************/ +/* Return TRUE to keep (copy to the result), FALSE to drop. */ +typedef gboolean (*NMConnectionForEachSecretFunc) (NMSettingSecretFlags flags, + gpointer user_data); + +GVariant *_nm_connection_for_each_secret (NMConnection *self, + GVariant *secrets, + gboolean remove_non_secrets, + NMConnectionForEachSecretFunc callback, + gpointer callback_data); + +typedef gboolean (*NMConnectionFindSecretFunc) (NMSettingSecretFlags flags, + gpointer user_data); + +gboolean _nm_connection_find_secret (NMConnection *self, + GVariant *secrets, + NMConnectionFindSecretFunc callback, + gpointer callback_data); + +/*****************************************************************************/ + #endif diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 9ea3253733..aa4ec0a90a 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -208,150 +208,6 @@ nm_settings_connection_get_last_secret_agent_version_id (NMSettingsConnection *s /*****************************************************************************/ -/* Return TRUE to keep, FALSE to drop */ -typedef gboolean (*ForEachSecretFunc) (NMSettingSecretFlags flags, - gpointer user_data); - -/* Returns always a non-NULL, non-floating variant that must - * be unrefed by the caller. */ -static GVariant * -for_each_secret (NMConnection *self, - GVariant *secrets, - gboolean remove_non_secrets, - ForEachSecretFunc callback, - gpointer callback_data) -{ - GVariantBuilder secrets_builder, setting_builder; - GVariantIter secrets_iter, *setting_iter; - const char *setting_name; - - /* This function, given a dict of dicts representing new secrets of - * an NMConnection, walks through each toplevel dict (which represents a - * NMSetting), and for each setting, walks through that setting dict's - * properties. For each property that's a secret, it will check that - * secret's flags in the backing NMConnection object, and call a supplied - * callback. - * - * The one complexity is that the VPN setting's 'secrets' property is - * *also* a dict (since the key/value pairs are arbitrary and known - * only to the VPN plugin itself). That means we have three levels of - * dicts that we potentially have to traverse here. When we hit the - * VPN setting's 'secrets' property, we special-case that and iterate over - * each item in that 'secrets' dict, calling the supplied callback - * each time. - */ - - g_return_val_if_fail (callback, NULL); - - g_variant_iter_init (&secrets_iter, secrets); - g_variant_builder_init (&secrets_builder, NM_VARIANT_TYPE_CONNECTION); - while (g_variant_iter_next (&secrets_iter, "{&sa{sv}}", &setting_name, &setting_iter)) { - NMSetting *setting; - const char *secret_name; - GVariant *val; - - setting = nm_connection_get_setting_by_name (self, setting_name); - if (setting == NULL) { - g_variant_iter_free (setting_iter); - continue; - } - - g_variant_builder_init (&setting_builder, NM_VARIANT_TYPE_SETTING); - while (g_variant_iter_next (setting_iter, "{&sv}", &secret_name, &val)) { - NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; - - /* VPN secrets need slightly different treatment here since the - * "secrets" property is actually a hash table of secrets. - */ - if (NM_IS_SETTING_VPN (setting) && !g_strcmp0 (secret_name, NM_SETTING_VPN_SECRETS)) { - GVariantBuilder vpn_secrets_builder; - GVariantIter vpn_secrets_iter; - const char *vpn_secret_name, *secret; - - if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("a{ss}"))) { - /* invalid type. Silently ignore the secrets as we cannot find out the - * secret-flags. */ - g_variant_unref (val); - continue; - } - - /* Iterate through each secret from the VPN dict in the overall secrets dict */ - g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}")); - g_variant_iter_init (&vpn_secrets_iter, val); - while (g_variant_iter_next (&vpn_secrets_iter, "{&s&s}", &vpn_secret_name, &secret)) { - - /* we ignore the return value of get_secret_flags. The function may determine - * that this is not a secret, based on having not secret-flags and no secrets. - * But we have the secret at hand. We know it would be a valid secret, if we - * only would add it to the VPN settings. */ - nm_setting_get_secret_flags (setting, vpn_secret_name, &secret_flags, NULL); - - if (callback (secret_flags, callback_data)) - g_variant_builder_add (&vpn_secrets_builder, "{ss}", vpn_secret_name, secret); - } - - g_variant_builder_add (&setting_builder, "{sv}", - secret_name, g_variant_builder_end (&vpn_secrets_builder)); - } else { - if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { - if (!remove_non_secrets) - g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); - g_variant_unref (val); - continue; - } - if (callback (secret_flags, callback_data)) - g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); - } - g_variant_unref (val); - } - - g_variant_iter_free (setting_iter); - g_variant_builder_add (&secrets_builder, "{sa{sv}}", setting_name, &setting_builder); - } - - return g_variant_ref_sink (g_variant_builder_end (&secrets_builder)); -} - -typedef gboolean (*FindSecretFunc) (NMSettingSecretFlags flags, - gpointer user_data); - -typedef struct { - FindSecretFunc find_func; - gpointer find_func_data; - gboolean found; -} FindSecretData; - -static gboolean -find_secret_for_each_func (NMSettingSecretFlags flags, - gpointer user_data) -{ - FindSecretData *data = user_data; - - if (!data->found) - data->found = data->find_func (flags, data->find_func_data); - return FALSE; -} - -static gboolean -find_secret (NMConnection *self, - GVariant *secrets, - FindSecretFunc callback, - gpointer callback_data) -{ - FindSecretData data; - GVariant *dummy; - - data.find_func = callback; - data.find_func_data = callback_data; - data.found = FALSE; - - dummy = for_each_secret (self, secrets, FALSE, find_secret_for_each_func, &data); - g_variant_unref (dummy); - return data.found; -} - -/*****************************************************************************/ - static void set_visible (NMSettingsConnection *self, gboolean new_visible) { @@ -1001,7 +857,7 @@ get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */ * save those system-owned secrets. If not, discard them and use the * existing secrets, or fail the connection. */ - *agent_had_system = find_secret (connection, secrets, secret_is_system_owned, NULL); + *agent_had_system = _nm_connection_find_secret (connection, secrets, secret_is_system_owned, NULL); if (*agent_had_system) { if (flags == NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) { /* No user interaction was allowed when requesting secrets; the @@ -1167,7 +1023,7 @@ get_secrets_done_cb (NMAgentManager *manager, * will have been authenticated, so those secrets can replace the existing * system secrets. */ - filtered_secrets = for_each_secret (nm_settings_connection_get_connection (self), secrets, TRUE, validate_secret_flags, &cmp_flags); + filtered_secrets = _nm_connection_for_each_secret (nm_settings_connection_get_connection (self), secrets, TRUE, validate_secret_flags, &cmp_flags); if (nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, filtered_secrets, &local)) { /* Now that all secrets are updated, copy and cache new secrets, * then save them to backing storage. @@ -1229,7 +1085,7 @@ get_secrets_done_cb (NMAgentManager *manager, if (!dict || nm_connection_update_secrets (applied_connection, setting_name, dict, NULL)) { GVariant *filtered_secrets; - filtered_secrets = for_each_secret (applied_connection, secrets, TRUE, validate_secret_flags, &cmp_flags); + filtered_secrets = _nm_connection_for_each_secret (applied_connection, secrets, TRUE, validate_secret_flags, &cmp_flags); nm_connection_update_secrets (applied_connection, setting_name, filtered_secrets, NULL); g_variant_unref (filtered_secrets); } From 866ac505a85e8f3b3e71cde58afb4c9d8674ec0f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 09:04:01 +0100 Subject: [PATCH 09/11] libnm,core: various cleanups of _nm_connection_for_each_secret() - use cleanup attribute to free memory - return floating reference from _nm_connection_for_each_secret(). It's more idiomatic that a function that constructs a variant and returns it, returns a floating variant. --- libnm-core/nm-connection.c | 32 +++++++++++++-------------- src/settings/nm-settings-connection.c | 26 +++++++++++++++------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 1e38029dd8..a555bd6d58 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -1875,7 +1875,7 @@ nm_connection_clear_secrets_with_flags (NMConnection *connection, /*****************************************************************************/ -/* Returns always a non-NULL, non-floating variant that must +/* Returns always a non-NULL, floating variant that must * be unrefed by the caller. */ GVariant * _nm_connection_for_each_secret (NMConnection *self, @@ -1884,8 +1884,10 @@ _nm_connection_for_each_secret (NMConnection *self, NMConnectionForEachSecretFunc callback, gpointer callback_data) { - GVariantBuilder secrets_builder, setting_builder; - GVariantIter secrets_iter, *setting_iter; + GVariantBuilder secrets_builder; + GVariantBuilder setting_builder; + GVariantIter secrets_iter; + GVariantIter *setting_iter; const char *setting_name; /* This function, given a dict of dicts representing new secrets of @@ -1909,18 +1911,18 @@ _nm_connection_for_each_secret (NMConnection *self, g_variant_iter_init (&secrets_iter, secrets); g_variant_builder_init (&secrets_builder, NM_VARIANT_TYPE_CONNECTION); while (g_variant_iter_next (&secrets_iter, "{&sa{sv}}", &setting_name, &setting_iter)) { + _nm_unused nm_auto_free_variant_iter GVariantIter *setting_iter_free = setting_iter; NMSetting *setting; const char *secret_name; GVariant *val; setting = nm_connection_get_setting_by_name (self, setting_name); - if (setting == NULL) { - g_variant_iter_free (setting_iter); + if (!setting) continue; - } g_variant_builder_init (&setting_builder, NM_VARIANT_TYPE_SETTING); while (g_variant_iter_next (setting_iter, "{&sv}", &secret_name, &val)) { + _nm_unused gs_unref_variant GVariant *val_free = val; NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; /* VPN secrets need slightly different treatment here since the @@ -1959,20 +1961,17 @@ _nm_connection_for_each_secret (NMConnection *self, if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { if (!remove_non_secrets) g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); - g_variant_unref (val); continue; } if (callback (secret_flags, callback_data)) g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); } - g_variant_unref (val); } - g_variant_iter_free (setting_iter); g_variant_builder_add (&secrets_builder, "{sa{sv}}", setting_name, &setting_builder); } - return g_variant_ref_sink (g_variant_builder_end (&secrets_builder)); + return g_variant_builder_end (&secrets_builder); } /*****************************************************************************/ @@ -2000,15 +1999,14 @@ _nm_connection_find_secret (NMConnection *self, NMConnectionFindSecretFunc callback, gpointer callback_data) { - FindSecretData data; - GVariant *dummy; - - data.find_func = callback; - data.find_func_data = callback_data; - data.found = FALSE; + gs_unref_variant GVariant *dummy = NULL; + FindSecretData data = { + .find_func = callback, + .find_func_data = callback_data, + .found = FALSE, + }; dummy = _nm_connection_for_each_secret (self, secrets, FALSE, find_secret_for_each_func, &data); - g_variant_unref (dummy); return data.found; } diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index aa4ec0a90a..25eb828bbb 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -803,8 +803,8 @@ typedef struct { } ForEachSecretFlags; static gboolean -validate_secret_flags (NMSettingSecretFlags flags, - gpointer user_data) +validate_secret_flags_cb (NMSettingSecretFlags flags, + gpointer user_data) { ForEachSecretFlags *cmp_flags = user_data; @@ -815,6 +815,18 @@ validate_secret_flags (NMSettingSecretFlags flags, return TRUE; } +static GVariant * +validate_secret_flags (NMConnection *connection, + GVariant *secrets, + ForEachSecretFlags *cmp_flags) +{ + return g_variant_ref_sink (_nm_connection_for_each_secret (connection, + secrets, + TRUE, + validate_secret_flags_cb, + cmp_flags)); +} + static gboolean secret_is_system_owned (NMSettingSecretFlags flags, gpointer user_data) @@ -1016,14 +1028,14 @@ get_secrets_done_cb (NMAgentManager *manager, /* Update the connection with our existing secrets from backing storage */ nm_connection_clear_secrets (nm_settings_connection_get_connection (self)); if (!dict || nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, dict, &local)) { - GVariant *filtered_secrets; + gs_unref_variant GVariant *filtered_secrets = NULL; /* Update the connection with the agent's secrets; by this point if any * system-owned secrets exist in 'secrets' the agent that provided them * will have been authenticated, so those secrets can replace the existing * system secrets. */ - filtered_secrets = _nm_connection_for_each_secret (nm_settings_connection_get_connection (self), secrets, TRUE, validate_secret_flags, &cmp_flags); + filtered_secrets = validate_secret_flags (nm_settings_connection_get_connection (self), secrets, &cmp_flags); if (nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, filtered_secrets, &local)) { /* Now that all secrets are updated, copy and cache new secrets, * then save them to backing storage. @@ -1059,7 +1071,6 @@ get_secrets_done_cb (NMAgentManager *manager, call_id, local->message); } - g_variant_unref (filtered_secrets); } else { _LOGD ("(%s:%p) failed to update with existing secrets: %s", setting_name, @@ -1083,11 +1094,10 @@ get_secrets_done_cb (NMAgentManager *manager, nm_connection_clear_secrets (applied_connection); if (!dict || nm_connection_update_secrets (applied_connection, setting_name, dict, NULL)) { - GVariant *filtered_secrets; + gs_unref_variant GVariant *filtered_secrets = NULL; - filtered_secrets = _nm_connection_for_each_secret (applied_connection, secrets, TRUE, validate_secret_flags, &cmp_flags); + filtered_secrets = validate_secret_flags (applied_connection, secrets, &cmp_flags); nm_connection_update_secrets (applied_connection, setting_name, filtered_secrets, NULL); - g_variant_unref (filtered_secrets); } } From 353e619c9f15c1bfee2ab01000ecc5ae3e28f87f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 09:38:58 +0100 Subject: [PATCH 10/11] libnm,core: make for-each-secret implementation virtual functions of NMSetting We already need to special handle regular settings (with secrets as GObject properties) and VPN secrets. Next, we will also need to special handle WireGuard peers, which can have secrets too. Move the code to a virtual function, so that "nm-connection.c" and "nm-setting.c" does not have explicit per-setting knowledge. --- libnm-core/nm-connection.c | 57 ++++++----------------------------- libnm-core/nm-core-internal.h | 6 +--- libnm-core/nm-setting-vpn.c | 52 ++++++++++++++++++++++++++++++++ libnm-core/nm-setting.c | 21 +++++++++++++ libnm-core/nm-setting.h | 17 +++++++++-- 5 files changed, 99 insertions(+), 54 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index a555bd6d58..5bafe2d7c0 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -1881,7 +1881,7 @@ GVariant * _nm_connection_for_each_secret (NMConnection *self, GVariant *secrets, gboolean remove_non_secrets, - NMConnectionForEachSecretFunc callback, + _NMConnectionForEachSecretFunc callback, gpointer callback_data) { GVariantBuilder secrets_builder; @@ -1900,10 +1900,8 @@ _nm_connection_for_each_secret (NMConnection *self, * The one complexity is that the VPN setting's 'secrets' property is * *also* a dict (since the key/value pairs are arbitrary and known * only to the VPN plugin itself). That means we have three levels of - * dicts that we potentially have to traverse here. When we hit the - * VPN setting's 'secrets' property, we special-case that and iterate over - * each item in that 'secrets' dict, calling the supplied callback - * each time. + * dicts that we potentially have to traverse here. The differences + * are handled by the virtual for_each_secret() function. */ g_return_val_if_fail (callback, NULL); @@ -1923,49 +1921,14 @@ _nm_connection_for_each_secret (NMConnection *self, g_variant_builder_init (&setting_builder, NM_VARIANT_TYPE_SETTING); while (g_variant_iter_next (setting_iter, "{&sv}", &secret_name, &val)) { _nm_unused gs_unref_variant GVariant *val_free = val; - NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; - /* VPN secrets need slightly different treatment here since the - * "secrets" property is actually a hash table of secrets. - */ - if (NM_IS_SETTING_VPN (setting) && !g_strcmp0 (secret_name, NM_SETTING_VPN_SECRETS)) { - GVariantBuilder vpn_secrets_builder; - GVariantIter vpn_secrets_iter; - const char *vpn_secret_name, *secret; - - if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("a{ss}"))) { - /* invalid type. Silently ignore the secrets as we cannot find out the - * secret-flags. */ - g_variant_unref (val); - continue; - } - - /* Iterate through each secret from the VPN dict in the overall secrets dict */ - g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}")); - g_variant_iter_init (&vpn_secrets_iter, val); - while (g_variant_iter_next (&vpn_secrets_iter, "{&s&s}", &vpn_secret_name, &secret)) { - - /* we ignore the return value of get_secret_flags. The function may determine - * that this is not a secret, based on having not secret-flags and no secrets. - * But we have the secret at hand. We know it would be a valid secret, if we - * only would add it to the VPN settings. */ - nm_setting_get_secret_flags (setting, vpn_secret_name, &secret_flags, NULL); - - if (callback (secret_flags, callback_data)) - g_variant_builder_add (&vpn_secrets_builder, "{ss}", vpn_secret_name, secret); - } - - g_variant_builder_add (&setting_builder, "{sv}", - secret_name, g_variant_builder_end (&vpn_secrets_builder)); - } else { - if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { - if (!remove_non_secrets) - g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); - continue; - } - if (callback (secret_flags, callback_data)) - g_variant_builder_add (&setting_builder, "{sv}", secret_name, val); - } + NM_SETTING_GET_CLASS (setting)->for_each_secret (setting, + secret_name, + val, + remove_non_secrets, + callback, + callback_data, + &setting_builder); } g_variant_builder_add (&secrets_builder, "{sa{sv}}", setting_name, &setting_builder); diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 870ec78133..2d66eb83fb 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -733,14 +733,10 @@ GBytes *_nm_setting_802_1x_cert_value_to_bytes (NMSetting8021xCKScheme scheme, /*****************************************************************************/ -/* Return TRUE to keep (copy to the result), FALSE to drop. */ -typedef gboolean (*NMConnectionForEachSecretFunc) (NMSettingSecretFlags flags, - gpointer user_data); - GVariant *_nm_connection_for_each_secret (NMConnection *self, GVariant *secrets, gboolean remove_non_secrets, - NMConnectionForEachSecretFunc callback, + _NMConnectionForEachSecretFunc callback, gpointer callback_data); typedef gboolean (*NMConnectionFindSecretFunc) (NMSettingSecretFlags flags, diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c index 133620b7d4..e3612c13bd 100644 --- a/libnm-core/nm-setting-vpn.c +++ b/libnm-core/nm-setting-vpn.c @@ -684,6 +684,57 @@ update_one_secret (NMSetting *setting, const char *key, GVariant *value, GError return success; } +static void +for_each_secret (NMSetting *setting, + const char *secret_name, + GVariant *val, + gboolean remove_non_secrets, + _NMConnectionForEachSecretFunc callback, + gpointer callback_data, + GVariantBuilder *setting_builder) +{ + GVariantBuilder vpn_secrets_builder; + GVariantIter vpn_secrets_iter; + const char *vpn_secret_name; + const char *secret; + + if (!nm_streq (secret_name, NM_SETTING_VPN_SECRETS)) { + NM_SETTING_CLASS (nm_setting_vpn_parent_class)->for_each_secret (setting, + secret_name, + val, + remove_non_secrets, + callback, + callback_data, + setting_builder); + return; + } + + if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("a{ss}"))) { + /* invalid type. Silently ignore the secrets as we cannot find out the + * secret-flags. */ + return; + } + + /* Iterate through each secret from the VPN dict in the overall secrets dict */ + g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}")); + g_variant_iter_init (&vpn_secrets_iter, val); + while (g_variant_iter_next (&vpn_secrets_iter, "{&s&s}", &vpn_secret_name, &secret)) { + NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + + /* we ignore the return value of get_secret_flags. The function may determine + * that this is not a secret, based on having not secret-flags and no secrets. + * But we have the secret at hand. We know it would be a valid secret, if we + * only add it to the VPN settings. */ + nm_setting_get_secret_flags (setting, vpn_secret_name, &secret_flags, NULL); + + if (callback (secret_flags, callback_data)) + g_variant_builder_add (&vpn_secrets_builder, "{ss}", vpn_secret_name, secret); + } + + g_variant_builder_add (setting_builder, "{sv}", + secret_name, g_variant_builder_end (&vpn_secrets_builder)); +} + static gboolean get_secret_flags (NMSetting *setting, const char *secret_name, @@ -976,6 +1027,7 @@ nm_setting_vpn_class_init (NMSettingVpnClass *klass) setting_class->verify = verify; setting_class->update_one_secret = update_one_secret; + setting_class->for_each_secret = for_each_secret; setting_class->get_secret_flags = get_secret_flags; setting_class->set_secret_flags = set_secret_flags; setting_class->need_secrets = need_secrets; diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index c679e89d39..126c25d8a8 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -2094,6 +2094,26 @@ _nm_setting_update_secrets (NMSetting *setting, GVariant *secrets, GError **erro return result; } +static void +for_each_secret (NMSetting *setting, + const char *secret_name, + GVariant *val, + gboolean remove_non_secrets, + _NMConnectionForEachSecretFunc callback, + gpointer callback_data, + GVariantBuilder *setting_builder) +{ + NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + + if (!nm_setting_get_secret_flags (setting, secret_name, &secret_flags, NULL)) { + if (!remove_non_secrets) + g_variant_builder_add (setting_builder, "{sv}", secret_name, val); + return; + } + if (callback (secret_flags, callback_data)) + g_variant_builder_add (setting_builder, "{sv}", secret_name, val); +} + static void _set_error_secret_property_not_found (GError **error, NMSetting *setting, @@ -2629,6 +2649,7 @@ nm_setting_class_init (NMSettingClass *setting_class) setting_class->set_secret_flags = set_secret_flags; setting_class->compare_property = compare_property; setting_class->clear_secrets = clear_secrets; + setting_class->for_each_secret = for_each_secret; setting_class->duplicate_copy_properties = duplicate_copy_properties; setting_class->enumerate_values = enumerate_values; setting_class->aggregate = aggregate; diff --git a/libnm-core/nm-setting.h b/libnm-core/nm-setting.h index 96868731eb..fdf4a4c516 100644 --- a/libnm-core/nm-setting.h +++ b/libnm-core/nm-setting.h @@ -187,6 +187,10 @@ typedef void (*NMSettingValueIterFn) (NMSetting *setting, GParamFlags flags, gpointer user_data); +/*< private >*/ +typedef gboolean (*_NMConnectionForEachSecretFunc) (NMSettingSecretFlags flags, + gpointer user_data); + typedef struct { GObjectClass parent; @@ -253,10 +257,19 @@ typedef struct { gpointer arg); /*< private >*/ - const struct _NMMetaSettingInfo *setting_info; + void (*for_each_secret) (NMSetting *setting, + const char *secret_name, + GVariant *val, + gboolean remove_non_secrets, + _NMConnectionForEachSecretFunc callback, + gpointer callback_data, + GVariantBuilder *setting_builder); /*< private >*/ - gpointer padding[3]; + gpointer padding[2]; + + /*< private >*/ + const struct _NMMetaSettingInfo *setting_info; } NMSettingClass; GType nm_setting_get_type (void); From b4e9efd724590ed43bc32ed6d5505422dc1af3c2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 10:25:18 +0100 Subject: [PATCH 11/11] libnm-core: drop unused includes from "nm-setting.c" --- libnm-core/nm-setting.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 126c25d8a8..fe6808bcdc 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -32,15 +32,6 @@ #include "nm-utils-private.h" #include "nm-property-compare.h" -#include "nm-setting-connection.h" -#include "nm-setting-bond.h" -#include "nm-setting-bridge.h" -#include "nm-setting-bridge-port.h" -#include "nm-setting-pppoe.h" -#include "nm-setting-team.h" -#include "nm-setting-team-port.h" -#include "nm-setting-vpn.h" - /** * SECTION:nm-setting * @short_description: Describes related configuration information