From 34285fec76591b5c216cf24bdf4765f6210c327b Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Thu, 25 Mar 2021 16:39:35 +0100 Subject: [PATCH 1/4] libnm: Refactor NM_CONNECTION_SERIALIZE_* flags nm-settings-connection.c has code similar to this in two places: /* FIXME: improve NMConnection API so we can avoid the overhead of cloning the connection, * in particular if there are no secrets to begin with. */ connection_cloned = nm_simple_connection_new_clone(new); /* Clear out unwanted secrets */ _nm_connection_clear_secrets_by_secret_flags(connection_cloned, NM_SETTING_SECRET_FLAG_NOT_SAVED | NM_SETTING_SECRET_FLAG_AGENT_OWNED); secrets = nm_g_variant_ref_sink( nm_connection_to_dbus(connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS)); It seems the secrets filtering can be done by nm_connection_to_dbus() if the NM_CONNECTION_SERIALIZE_* flags are extended. The current set of flags contains flags that start with NO, ONLY and WITH prefixes, which makes it useless for combining the flags because most combinations of more than one flag don't have a clear interpretation. So they're mostly useful when used alone, i.e. you'd need to add a new enum value for each new subset of settings to be serialized. To get the most flexibility from a small set of flags they should either all be of the WITH_* type or NO_* type. In the former case they could be combined to extend the subset of properties serialized, in the latter case each flag would reduce the subset. After trying both options I found it's easier to adapt the current set of flags to the WITH_* schema while keeping binary and source compatibility. This commit changes the set of flags in the following way: NM_CONNECTION_SERIALIZE_ALL is kept for compatibility but is equivalent to a combination of other flags. NM_CONNECTION_SERIALIZE_WITH_NON_SECRET is added with the same value as NM_CONNECTION_SERIALIZE_NO_SECRETS, it implies that non-secret properties are included but doesn't prevent including other properties. Since it couldn't be meaningfully combined with any other flag this change shouldn't break compatibility. Similarly NM_CONNECTION_SERIALIZE_WITH_SECRETS is added with the same value as existing NM_CONNECTION_SERIALIZE_ONLY_SECRETS with the same consideration about compatibility. NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED and the new NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED and NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED add only subsets of secrets and can be combined. For backwards compatibility NM_CONNECTION_SERIALIZE_ONLY_SECRETS is basically ignored when either of these three is present, so that the value: ..ONLY_SECRETS | ..AGENT_OWNED works as previously. --- src/libnm-core-impl/nm-setting-ip4-config.c | 6 ++-- src/libnm-core-impl/nm-setting-ip6-config.c | 4 +-- src/libnm-core-impl/nm-setting-vpn.c | 25 +++++++++----- src/libnm-core-impl/nm-setting-wireguard.c | 9 +++-- src/libnm-core-impl/nm-setting-wireless.c | 2 +- src/libnm-core-impl/nm-setting.c | 18 +++++----- src/libnm-core-impl/nm-utils.c | 3 +- src/libnm-core-intern/nm-core-internal.h | 37 ++++++++++++++++++--- src/libnm-core-public/nm-connection.h | 36 ++++++++++++++------ 9 files changed, 96 insertions(+), 44 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index b19d82e618..7c386d1fd4 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -388,7 +388,7 @@ ip4_address_labels_get(const NMSettInfoSetting * sett_info, GVariant * ret; int num_addrs, i; - if (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; num_addrs = nm_setting_ip_config_get_num_addresses(s_ip); @@ -428,7 +428,7 @@ ip4_address_data_get(const NMSettInfoSetting * sett_info, { gs_unref_ptrarray GPtrArray *addrs = NULL; - if (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; g_object_get(setting, NM_SETTING_IP_CONFIG_ADDRESSES, &addrs, NULL); @@ -502,7 +502,7 @@ ip4_route_data_get(const NMSettInfoSetting * sett_info, { gs_unref_ptrarray GPtrArray *routes = NULL; - if (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; g_object_get(setting, NM_SETTING_IP_CONFIG_ROUTES, &routes, NULL); diff --git a/src/libnm-core-impl/nm-setting-ip6-config.c b/src/libnm-core-impl/nm-setting-ip6-config.c index 4477436edb..815a118db8 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -410,7 +410,7 @@ ip6_address_data_get(const NMSettInfoSetting * sett_info, { gs_unref_ptrarray GPtrArray *addrs = NULL; - if (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; g_object_get(setting, NM_SETTING_IP_CONFIG_ADDRESSES, &addrs, NULL); @@ -484,7 +484,7 @@ ip6_route_data_get(const NMSettInfoSetting * sett_info, { gs_unref_ptrarray GPtrArray *routes = NULL; - if (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; g_object_get(setting, NM_SETTING_IP_CONFIG_ROUTES, &routes, NULL); diff --git a/src/libnm-core-impl/nm-setting-vpn.c b/src/libnm-core-impl/nm-setting-vpn.c index ce6e73f5b4..a70fa9b939 100644 --- a/src/libnm-core-impl/nm-setting-vpn.c +++ b/src/libnm-core-impl/nm-setting-vpn.c @@ -937,21 +937,30 @@ vpn_secrets_to_dbus(const NMSettInfoSetting * sett_info, gs_free const char **keys = NULL; guint i, len; - if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_NO_SECRETS)) + if (flags != NM_CONNECTION_SERIALIZE_ALL + && !NM_FLAGS_ANY(flags, + NM_CONNECTION_SERIALIZE_WITH_SECRETS + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED)) return NULL; g_variant_builder_init(&builder, G_VARIANT_TYPE("a{ss}")); keys = nm_utils_strdict_get_keys(priv->secrets, TRUE, &len); for (i = 0; i < len; i++) { - const char * key = keys[i]; - NMSettingSecretFlags secret_flags; + const char * key = keys[i]; + NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; + + if (NM_FLAGS_ANY(flags, + NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED)) + nm_setting_get_secret_flags(setting, key, &secret_flags, NULL); + + if (!_nm_connection_serialize_secrets(flags, secret_flags)) + continue; - if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED)) { - if (!nm_setting_get_secret_flags(setting, key, &secret_flags, NULL) - || !NM_FLAGS_HAS(secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) - continue; - } g_variant_builder_add(&builder, "{ss}", key, g_hash_table_lookup(priv->secrets, key)); } diff --git a/src/libnm-core-impl/nm-setting-wireguard.c b/src/libnm-core-impl/nm-setting-wireguard.c index 5c96fc6dfa..70862f6147 100644 --- a/src/libnm-core-impl/nm-setting-wireguard.c +++ b/src/libnm-core-impl/nm-setting-wireguard.c @@ -1494,7 +1494,7 @@ _peers_dbus_only_synth(const NMSettInfoSetting * sett_info, NM_WIREGUARD_PEER_ATTR_PUBLIC_KEY, g_variant_new_string(peer->public_key)); - if (!NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_ONLY_SECRETS) && peer->endpoint) + if (_nm_connection_serialize_non_secret(flags) && peer->endpoint) g_variant_builder_add( &builder, "{sv}", @@ -1508,21 +1508,20 @@ _peers_dbus_only_synth(const NMSettInfoSetting * sett_info, NM_WIREGUARD_PEER_ATTR_PRESHARED_KEY, g_variant_new_string(peer->preshared_key)); - if (!NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (_nm_connection_serialize_non_secret(flags) && peer->preshared_key_flags != NM_SETTING_SECRET_FLAG_NOT_REQUIRED) g_variant_builder_add(&builder, "{sv}", NM_WIREGUARD_PEER_ATTR_PRESHARED_KEY_FLAGS, g_variant_new_uint32(peer->preshared_key_flags)); - if (!NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_ONLY_SECRETS) - && peer->persistent_keepalive != 0) + if (_nm_connection_serialize_non_secret(flags) && peer->persistent_keepalive != 0) g_variant_builder_add(&builder, "{sv}", NM_WIREGUARD_PEER_ATTR_PERSISTENT_KEEPALIVE, g_variant_new_uint32(peer->persistent_keepalive)); - if (!NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_ONLY_SECRETS) && peer->allowed_ips + if (_nm_connection_serialize_non_secret(flags) && peer->allowed_ips && peer->allowed_ips->len > 0) { const char *const * strv = (const char *const *) peer->allowed_ips->pdata; gs_free const char **strv_fixed = NULL; diff --git a/src/libnm-core-impl/nm-setting-wireless.c b/src/libnm-core-impl/nm-setting-wireless.c index 54c12c818a..95041aeaa5 100644 --- a/src/libnm-core-impl/nm-setting-wireless.c +++ b/src/libnm-core-impl/nm-setting-wireless.c @@ -1080,7 +1080,7 @@ nm_setting_wireless_get_security(const NMSettInfoSetting * sett_in NMConnectionSerializationFlags flags, const NMConnectionSerializationOptions *options) { - if (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; if (!connection) diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index 22ec0f6d34..2e373a5ab0 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -582,20 +582,20 @@ property_to_dbus(const NMSettInfoSetting * sett_info, return NULL; if (NM_FLAGS_HAS(property->param_spec->flags, NM_SETTING_PARAM_SECRET)) { - if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_NO_SECRETS)) - return NULL; + NMSettingSecretFlags f = NM_SETTING_SECRET_FLAG_NONE; - if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED)) { - NMSettingSecretFlags f; - - /* see also _nm_connection_serialize_secrets() */ + if (NM_FLAGS_ANY(flags, + NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED)) { if (!nm_setting_get_secret_flags(setting, property->param_spec->name, &f, NULL)) return NULL; - if (!NM_FLAGS_HAS(f, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) - return NULL; } + + if (!_nm_connection_serialize_secrets(flags, f)) + return NULL; } else { - if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_ONLY_SECRETS)) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; } } diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index badae39a1b..8f6a858530 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -4306,7 +4306,8 @@ _nm_utils_hwaddr_cloned_data_synth(const NMSettInfoSetting * sett_ { gs_free char *addr = NULL; - if (flags & NM_CONNECTION_SERIALIZE_ONLY_SECRETS) + if (flags != NM_CONNECTION_SERIALIZE_ALL + && !NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET)) return NULL; nm_assert(nm_streq0(sett_info->property_infos[property_idx].name, "assigned-mac-address")); diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index bd607b45b6..aa24c3644c 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -801,16 +801,43 @@ GBytes *_nm_setting_802_1x_cert_value_to_bytes(NMSetting8021xCKScheme scheme, /*****************************************************************************/ +static inline gboolean +_nm_connection_serialize_non_secret(NMConnectionSerializationFlags flags) +{ + if (flags == NM_CONNECTION_SERIALIZE_ALL) + return TRUE; + + return NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET); +} + static inline gboolean _nm_connection_serialize_secrets(NMConnectionSerializationFlags flags, NMSettingSecretFlags secret_flags) { - if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_NO_SECRETS)) - return FALSE; + if (flags == NM_CONNECTION_SERIALIZE_ALL) + return TRUE; + + if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS) + && !NM_FLAGS_ANY(flags, + NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED)) + return TRUE; + if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED) - && !NM_FLAGS_HAS(secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) - return FALSE; - return TRUE; + && NM_FLAGS_HAS(secret_flags, NM_SETTING_SECRET_FLAG_AGENT_OWNED)) + return TRUE; + + if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED) + && !NM_FLAGS_ANY(secret_flags, + NM_SETTING_SECRET_FLAG_AGENT_OWNED | NM_SETTING_SECRET_FLAG_NOT_SAVED)) + return TRUE; + + if (NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED) + && NM_FLAGS_HAS(secret_flags, NM_SETTING_SECRET_FLAG_NOT_SAVED)) + return TRUE; + + return FALSE; } void _nm_connection_clear_secrets_by_secret_flags(NMConnection * self, diff --git a/src/libnm-core-public/nm-connection.h b/src/libnm-core-public/nm-connection.h index 19034e790a..44d1568d7a 100644 --- a/src/libnm-core-public/nm-connection.h +++ b/src/libnm-core-public/nm-connection.h @@ -91,19 +91,35 @@ NMSetting *nm_connection_get_setting_by_name(NMConnection *connection, const cha /** * NMConnectionSerializationFlags: * @NM_CONNECTION_SERIALIZE_ALL: serialize all properties (including secrets) - * @NM_CONNECTION_SERIALIZE_NO_SECRETS: do not include secrets - * @NM_CONNECTION_SERIALIZE_ONLY_SECRETS: only serialize secrets - * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED: if set, only secrets that - * are agent owned will be serialized. Since: 1.20. + * @NM_CONNECTION_SERIALIZE_WITH_NON_SECRET: serialize properties that are + * not secrets. Since 1.32. + * @NM_CONNECTION_SERIALIZE_NO_SECRETS: this is a deprecated alias for + * @NM_CONNECTION_SERIALIZE_WITH_NON_SECRET. + * @NM_CONNECTION_SERIALIZE_WITH_SECRETS: serialize all secrets. This flag is + * ignored if any of @NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED, + * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED or + * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED is set. Since 1.32. + * @NM_CONNECTION_SERIALIZE_ONLY_SECRETS: a deprecated alias for + * @NM_CONNECTION_SERIALIZE_WITH_SECRETS. + * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED: serialize agent-owned + * secrets. Since: 1.20. + * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED: serialize system-owned + * secrets. Since: 1.32. + * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED: serialize secrets that + * are marked as never saved. Since: 1.32. * - * These flags determine which properties are serialized when calling when - * calling nm_connection_to_dbus(). + * These flags determine which properties are serialized when calling + * nm_connection_to_dbus(). **/ typedef enum { /*< flags >*/ - NM_CONNECTION_SERIALIZE_ALL = 0x00000000, - NM_CONNECTION_SERIALIZE_NO_SECRETS = 0x00000001, - NM_CONNECTION_SERIALIZE_ONLY_SECRETS = 0x00000002, - NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED = 0x00000004, + NM_CONNECTION_SERIALIZE_ALL = 0x00000000, + NM_CONNECTION_SERIALIZE_WITH_NON_SECRET = 0x00000001, + NM_CONNECTION_SERIALIZE_NO_SECRETS = 0x00000001, + NM_CONNECTION_SERIALIZE_WITH_SECRETS = 0x00000002, + NM_CONNECTION_SERIALIZE_ONLY_SECRETS = 0x00000002, + NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED = 0x00000004, + NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED = 0x00000008, + NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED = 0x00000010, } NMConnectionSerializationFlags; GVariant *nm_connection_to_dbus(NMConnection *connection, NMConnectionSerializationFlags flags); From 2a1b65ce12ee8063910a88674b53b03e1de3cad1 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Thu, 25 Mar 2021 18:06:27 +0100 Subject: [PATCH 2/4] settings: Don't clone connections to serialize secrets Use the new nm_connection_to_dbus() flags to filter secrets instead of cloning connections and using _nm_connection_clear_secrets_by_secret_flags() and then serializing all secrets in NMSettingsConnection. Fix a related comment. --- src/core/settings/nm-settings-connection.c | 52 +++++++--------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 6f400f7e85..83ca261060 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -436,27 +436,16 @@ nm_settings_connection_check_permission(NMSettingsConnection *self, const char * static void update_system_secrets_cache(NMSettingsConnection *self, NMConnection *new) { - NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self); - gs_unref_object NMConnection *connection_cloned = NULL; - gs_unref_variant GVariant *old_secrets = NULL; + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self); + gs_unref_variant GVariant *old_secrets = NULL; old_secrets = g_steal_pointer(&priv->system_secrets); - if (!new) - goto out; + if (new) { + priv->system_secrets = nm_g_variant_ref_sink( + nm_connection_to_dbus(new, NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED)); + } - /* FIXME: improve NMConnection API so we can avoid the overhead of cloning the connection, - * in particular if there are no secrets to begin with. */ - - connection_cloned = nm_simple_connection_new_clone(new); - - /* Clear out non-system-owned and not-saved secrets */ - _nm_connection_clear_secrets_by_secret_flags(connection_cloned, NM_SETTING_SECRET_FLAG_NONE); - - priv->system_secrets = nm_g_variant_ref_sink( - nm_connection_to_dbus(connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS)); - -out: if (_LOGT_ENABLED()) { if ((!!old_secrets) != (!!priv->system_secrets)) { _LOGT("update system secrets: secrets %s", old_secrets ? "cleared" : "set"); @@ -468,29 +457,18 @@ out: static void update_agent_secrets_cache(NMSettingsConnection *self, NMConnection *new) { - NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self); - gs_unref_object NMConnection *connection_cloned = NULL; - gs_unref_variant GVariant *old_secrets = NULL; + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self); + gs_unref_variant GVariant *old_secrets = NULL; old_secrets = g_steal_pointer(&priv->agent_secrets); - if (!new) - goto out; + if (new) { + priv->agent_secrets = nm_g_variant_ref_sink( + nm_connection_to_dbus(new, + NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED)); + } - /* FIXME: improve NMConnection API so we can avoid the overhead of cloning the connection, - * in particular if there are no secrets to begin with. */ - - connection_cloned = nm_simple_connection_new_clone(new); - - /* Clear out non-system-owned secrets */ - _nm_connection_clear_secrets_by_secret_flags(connection_cloned, - NM_SETTING_SECRET_FLAG_NOT_SAVED - | NM_SETTING_SECRET_FLAG_AGENT_OWNED); - - priv->agent_secrets = nm_g_variant_ref_sink( - nm_connection_to_dbus(connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS)); - -out: if (_LOGT_ENABLED()) { if ((!!old_secrets) != (!!priv->agent_secrets)) { _LOGT("update agent secrets: secrets %s", old_secrets ? "cleared" : "set"); @@ -1568,7 +1546,7 @@ update_auth_cb(NMSettingsConnection * self, gs_unref_object NMConnection *for_agent = NULL; /* Dupe the connection so we can clear out non-agent-owned secrets, - * as agent-owned secrets are the only ones we send back be saved. + * as agent-owned secrets are the only ones we send back to be saved. * Only send secrets to agents of the same UID that called update too. */ for_agent = nm_simple_connection_new_clone(nm_settings_connection_get_connection(self)); From f0fe7384e1d059f6a7d80e2e08188583f02711d9 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Fri, 26 Mar 2021 22:56:46 +0100 Subject: [PATCH 3/4] all: Replace deprecated NM_CONNECTION_SERIALIZE_* flags Review and replace usages of the two nm_connection_to_dbus() flags marked deprecated in commit 84648e562c98 ('libnm: Refactor NM_CONNECTION_SERIALIZE_* flags'): NM_CONNECTION_SERIALIZE_NO_SECRETS and NM_CONNECTION_SERIALIZE_ONLY_SECRETS. --- src/core/devices/nm-device.c | 3 ++- src/core/nm-dispatcher.c | 2 +- src/core/settings/nm-secret-agent.c | 2 +- src/core/settings/nm-settings-connection.c | 4 ++-- src/core/settings/nm-settings.c | 8 ++------ src/libnm-core-impl/tests/test-general.c | 7 +++++-- src/libnm-core-impl/tests/test-secrets.c | 2 +- src/libnm-core-impl/tests/test-setting.c | 6 +++--- src/nmcli/connections.c | 2 +- src/nmtui/nmt-editor.c | 2 +- 10 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index e93a32d323..7caaa50016 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -12926,7 +12926,8 @@ impl_device_get_applied_connection(NMDBusObject * obj, return; } - var_settings = nm_connection_to_dbus(applied_connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); + var_settings = + nm_connection_to_dbus(applied_connection, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET); if (!var_settings) var_settings = g_variant_new_array(G_VARIANT_TYPE("{sa{sv}}"), NULL, 0); diff --git a/src/core/nm-dispatcher.c b/src/core/nm-dispatcher.c index 4fdb905dcf..5862dd0a82 100644 --- a/src/core/nm-dispatcher.c +++ b/src/core/nm-dispatcher.c @@ -550,7 +550,7 @@ _dispatcher_call(NMDispatcherAction action, if (applied_connection) connection_dict = - nm_connection_to_dbus(applied_connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); + nm_connection_to_dbus(applied_connection, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET); else connection_dict = g_variant_new_array(G_VARIANT_TYPE("{sa{sv}}"), NULL, 0); diff --git a/src/core/settings/nm-secret-agent.c b/src/core/settings/nm-secret-agent.c index 5493984b7b..45152c2778 100644 --- a/src/core/settings/nm-secret-agent.c +++ b/src/core/settings/nm-secret-agent.c @@ -583,7 +583,7 @@ nm_secret_agent_delete_secrets(NMSecretAgent * self, priv = NM_SECRET_AGENT_GET_PRIVATE(self); /* No secrets sent; agents must be smart enough to track secrets using the UUID or something */ - dict = nm_connection_to_dbus(connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); + dict = nm_connection_to_dbus(connection, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET); call_id = _call_id_new(self, METHOD_DELETE_SECRETS, path, NULL, callback, callback_data); diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 83ca261060..294cb04705 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -1333,7 +1333,7 @@ get_settings_auth_cb(NMSettingsConnection * self, * protected against leakage of secrets to unprivileged callers. */ settings = nm_connection_to_dbus_full(nm_settings_connection_get_connection(self), - NM_CONNECTION_SERIALIZE_NO_SECRETS, + NM_CONNECTION_SERIALIZE_WITH_NON_SECRET, &options); g_dbus_method_invocation_return_value(context, g_variant_new("(@a{sa{sv}})", settings)); } @@ -1866,7 +1866,7 @@ dbus_get_agent_secrets_cb(NMSettingsConnection * self, * by the time we get here. */ dict = nm_connection_to_dbus(nm_settings_connection_get_connection(self), - NM_CONNECTION_SERIALIZE_ONLY_SECRETS); + NM_CONNECTION_SERIALIZE_WITH_SECRETS); if (!dict) dict = g_variant_new_array(G_VARIANT_TYPE("{sa{sv}}"), NULL, 0); g_dbus_method_invocation_return_value(context, g_variant_new("(@a{sa{sv}})", dict)); diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c index 92733f5dbf..4c43484d39 100644 --- a/src/core/settings/nm-settings.c +++ b/src/core/settings/nm-settings.c @@ -1541,9 +1541,7 @@ _add_connection_to_first_plugin(NMSettings * self, } agent_owned_secrets = - nm_connection_to_dbus(new_connection, - NM_CONNECTION_SERIALIZE_ONLY_SECRETS - | NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED); + nm_connection_to_dbus(new_connection, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED); connection_to_add_real = _connection_changed_normalize_connection(storage, connection_to_add, @@ -2218,9 +2216,7 @@ nm_settings_update_connection(NMSettings * self, nm_assert(nm_streq(uuid, nm_settings_storage_get_uuid(new_storage))); agent_owned_secrets = - nm_connection_to_dbus(connection, - NM_CONNECTION_SERIALIZE_ONLY_SECRETS - | NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED); + nm_connection_to_dbus(connection, NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED); new_connection_real = _connection_changed_normalize_connection(new_storage, new_connection, agent_owned_secrets, diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c index f75401693e..146eeee20f 100644 --- a/src/libnm-core-impl/tests/test-general.c +++ b/src/libnm-core-impl/tests/test-general.c @@ -2583,7 +2583,10 @@ test_setting_to_dbus_no_secrets(void) s_wsec = make_test_wsec_setting("setting-to-dbus-no-secrets"); - dict = _nm_setting_to_dbus(NM_SETTING(s_wsec), NULL, NM_CONNECTION_SERIALIZE_NO_SECRETS, NULL); + dict = _nm_setting_to_dbus(NM_SETTING(s_wsec), + NULL, + NM_CONNECTION_SERIALIZE_WITH_NON_SECRET, + NULL); /* Make sure non-secret keys are there */ g_assert(_variant_contains(dict, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT)); @@ -2606,7 +2609,7 @@ test_setting_to_dbus_only_secrets(void) s_wsec = make_test_wsec_setting("setting-to-dbus-only-secrets"); dict = - _nm_setting_to_dbus(NM_SETTING(s_wsec), NULL, NM_CONNECTION_SERIALIZE_ONLY_SECRETS, NULL); + _nm_setting_to_dbus(NM_SETTING(s_wsec), NULL, NM_CONNECTION_SERIALIZE_WITH_SECRETS, NULL); /* Make sure non-secret keys are not there */ g_assert(!_variant_contains(dict, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT)); diff --git a/src/libnm-core-impl/tests/test-secrets.c b/src/libnm-core-impl/tests/test-secrets.c index e371795c74..0d33df6e97 100644 --- a/src/libnm-core-impl/tests/test-secrets.c +++ b/src/libnm-core-impl/tests/test-secrets.c @@ -605,7 +605,7 @@ test_update_secrets_whole_connection_empty_base_setting(void) */ connection = wifi_connection_new(); - secrets = nm_connection_to_dbus(connection, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); + secrets = nm_connection_to_dbus(connection, NM_CONNECTION_SERIALIZE_WITH_SECRETS); g_assert_cmpint(g_variant_n_children(secrets), ==, 3); setting = g_variant_lookup_value(secrets, NM_SETTING_WIRELESS_SETTING_NAME, NULL); diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index 0f80826bcb..7d8d078b8b 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -3245,8 +3245,8 @@ test_roundtrip_conversion(gconstpointer test_data) gs_unref_ptrarray GPtrArray * wg_peers = NULL; const NMConnectionSerializationFlags dbus_serialization_flags[] = { NM_CONNECTION_SERIALIZE_ALL, - NM_CONNECTION_SERIALIZE_NO_SECRETS, - NM_CONNECTION_SERIALIZE_ONLY_SECRETS, + NM_CONNECTION_SERIALIZE_WITH_NON_SECRET, + NM_CONNECTION_SERIALIZE_WITH_SECRETS, }; guint dbus_serialization_flags_idx; gs_unref_object NMConnection *con = NULL; @@ -3643,7 +3643,7 @@ test_roundtrip_conversion(gconstpointer test_data) if (flag == NM_CONNECTION_SERIALIZE_ALL) _rndt_wg_peers_assert_equal(s_wg2, wg_peers, TRUE, TRUE, FALSE); - else if (flag == NM_CONNECTION_SERIALIZE_NO_SECRETS) + else if (flag == NM_CONNECTION_SERIALIZE_WITH_NON_SECRET) _rndt_wg_peers_assert_equal(s_wg2, wg_peers, FALSE, FALSE, TRUE); else g_assert_not_reached(); diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 74a5b6e260..50f684c0e6 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -5821,7 +5821,7 @@ gen_cmd_print0(const char *text, int state) int i = 0; settings = nm_connection_to_dbus(nmc_tab_completion.connection, - NM_CONNECTION_SERIALIZE_NO_SECRETS); + NM_CONNECTION_SERIALIZE_WITH_NON_SECRET); words = g_new(char *, g_variant_n_children(settings) + 2); g_variant_iter_init(&iter, settings); while (g_variant_iter_next(&iter, "{&s@a{sv}}", &setting_name, NULL)) diff --git a/src/nmtui/nmt-editor.c b/src/nmtui/nmt-editor.c index 40cee1b0f8..0ef236938f 100644 --- a/src/nmtui/nmt-editor.c +++ b/src/nmtui/nmt-editor.c @@ -216,7 +216,7 @@ build_edit_connection(NMConnection *orig_connection) if (!NM_IS_REMOTE_CONNECTION(orig_connection)) return edit_connection; - settings = nm_connection_to_dbus(orig_connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); + settings = nm_connection_to_dbus(orig_connection, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET); g_variant_iter_init(&iter, settings); while (g_variant_iter_next(&iter, "{&s@a{sv}}", &setting_name, NULL)) { if (!nm_meta_setting_info_editor_has_secrets( From 15fe7841809cdedb54ddb4d6971926d391f39ef7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Apr 2021 17:28:53 +0200 Subject: [PATCH 4/4] libnm-core: use _nm_connection_serialize_non_secret() in _nm_utils_hwaddr_cloned_data_synth() --- src/libnm-core-impl/nm-utils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index 8f6a858530..84565e6503 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -4306,8 +4306,7 @@ _nm_utils_hwaddr_cloned_data_synth(const NMSettInfoSetting * sett_ { gs_free char *addr = NULL; - if (flags != NM_CONNECTION_SERIALIZE_ALL - && !NM_FLAGS_HAS(flags, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET)) + if (!_nm_connection_serialize_non_secret(flags)) return NULL; nm_assert(nm_streq0(sett_info->property_infos[property_idx].name, "assigned-mac-address"));