diff options
author | Thomas Haller <thaller@redhat.com> | 2015-09-18 14:28:08 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2015-09-18 14:31:31 +0200 |
commit | 6c989bb68f3d182ec6e97e66c2f3faa25a0429b0 (patch) | |
tree | 0682ac5f5b914a9263466cf087e8c7c018e62eb2 | |
parent | d7614d0bda0622fa41a301054c34502453a263f6 (diff) | |
parent | 1b5664fed4ee40f9b7da7da88fd6b58c6b8a9bd0 (diff) | |
download | NetworkManager-6c989bb68f3d182ec6e97e66c2f3faa25a0429b0.tar.gz |
core: merge branch 'th/secret-requests-bgo754508'
https://bugzilla.gnome.org/show_bug.cgi?id=754508
-rw-r--r-- | src/NetworkManagerUtils.c | 6 | ||||
-rw-r--r-- | src/devices/nm-device-ethernet.c | 6 | ||||
-rw-r--r-- | src/devices/wifi/nm-device-wifi.c | 6 | ||||
-rw-r--r-- | src/devices/wwan/nm-modem.c | 40 | ||||
-rw-r--r-- | src/devices/wwan/nm-modem.h | 8 | ||||
-rw-r--r-- | src/nm-activation-request.c | 96 | ||||
-rw-r--r-- | src/nm-activation-request.h | 19 | ||||
-rw-r--r-- | src/nm-auth-utils.c | 124 | ||||
-rw-r--r-- | src/ppp-manager/nm-ppp-manager.c | 18 | ||||
-rw-r--r-- | src/settings/nm-agent-manager.c | 969 | ||||
-rw-r--r-- | src/settings/nm-agent-manager.h | 39 | ||||
-rw-r--r-- | src/settings/nm-secret-agent.c | 11 | ||||
-rw-r--r-- | src/settings/nm-settings-connection.c | 185 | ||||
-rw-r--r-- | src/settings/nm-settings-connection.h | 23 | ||||
-rw-r--r-- | src/vpn-manager/nm-vpn-connection.c | 14 |
15 files changed, 809 insertions, 755 deletions
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 2e732c52a5..d78313df50 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1540,6 +1540,12 @@ nm_match_spec_join (GSList *specs) return g_string_free (str, FALSE); } +/** + * nm_utils_get_shared_wifi_permission: + * @connection: the NMConnection to lookup the permission. + * + * Returns: a static string of the wifi-permission (if any) or %NULL. + */ const char * nm_utils_get_shared_wifi_permission (NMConnection *connection) { diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 03c1d5109a..3de26b48c1 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -480,7 +480,7 @@ supplicant_interface_release (NMDeviceEthernet *self) static void wired_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) @@ -488,7 +488,9 @@ wired_secrets_cb (NMActRequest *req, NMDeviceEthernet *self = NM_DEVICE_ETHERNET (user_data); NMDevice *dev = NM_DEVICE (self); - g_return_if_fail (req == nm_device_get_act_request (dev)); + if (req != nm_device_get_act_request (dev)) + return; + g_return_if_fail (nm_device_get_state (dev) == NM_DEVICE_STATE_NEED_AUTH); g_return_if_fail (nm_act_request_get_connection (req) == connection); diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 807f7b9cca..c668e1c70f 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1674,7 +1674,7 @@ cleanup_association_attempt (NMDeviceWifi *self, gboolean disconnect) static void wifi_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) @@ -1682,7 +1682,9 @@ wifi_secrets_cb (NMActRequest *req, NMDevice *device = NM_DEVICE (user_data); NMDeviceWifi *self = NM_DEVICE_WIFI (device); - g_return_if_fail (req == nm_device_get_act_request (device)); + if (req != nm_device_get_act_request (device)) + return; + g_return_if_fail (nm_device_get_state (device) == NM_DEVICE_STATE_NEED_AUTH); g_return_if_fail (nm_act_request_get_connection (req) == connection); diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index d6433a085d..71c2323983 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -73,7 +73,7 @@ typedef struct { NMActRequest *act_request; guint32 secrets_tries; - guint32 secrets_id; + NMActRequestGetSecretsCallId secrets_id; guint32 mm_ip_timeout; @@ -722,15 +722,12 @@ cancel_get_secrets (NMModem *self) { NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self); - if (priv->secrets_id) { - nm_act_request_cancel_secrets (priv->act_request, priv->secrets_id); - priv->secrets_id = 0; - } + nm_act_request_cancel_secrets (priv->act_request, priv->secrets_id); } static void modem_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) @@ -740,7 +737,10 @@ modem_secrets_cb (NMActRequest *req, g_return_if_fail (call_id == priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; + + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; if (error) nm_log_warn (LOGD_MB, "(%s): %s", nm_modem_get_uid (self), error->message); @@ -748,7 +748,7 @@ modem_secrets_cb (NMActRequest *req, g_signal_emit (self, signals[AUTH_RESULT], 0, error); } -gboolean +void nm_modem_get_secrets (NMModem *self, const char *setting_name, gboolean request_new, @@ -767,10 +767,8 @@ nm_modem_get_secrets (NMModem *self, hint, modem_secrets_cb, self); - if (priv->secrets_id) - g_signal_emit (self, signals[AUTH_REQUESTED], 0); - - return !!(priv->secrets_id); + g_return_if_fail (priv->secrets_id); + g_signal_emit (self, signals[AUTH_REQUESTED], 0); } /*****************************************************************************/ @@ -790,8 +788,7 @@ nm_modem_act_stage1_prepare (NMModem *self, NMDeviceStateReason *reason) { NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self); - NMActStageReturn ret; - GPtrArray *hints = NULL; + gs_unref_ptrarray GPtrArray *hints = NULL; const char *setting_name = NULL; NMSecretAgentGetSecretsFlags flags = NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION; NMConnection *connection; @@ -820,18 +817,9 @@ nm_modem_act_stage1_prepare (NMModem *self, hints ? g_ptr_array_index (hints, 0) : NULL, modem_secrets_cb, self); - if (priv->secrets_id) { - g_signal_emit (self, signals[AUTH_REQUESTED], 0); - ret = NM_ACT_STAGE_RETURN_POSTPONE; - } else { - *reason = NM_DEVICE_STATE_REASON_NO_SECRETS; - ret = NM_ACT_STAGE_RETURN_FAILURE; - } - - if (hints) - g_ptr_array_free (hints, TRUE); - - return ret; + g_return_val_if_fail (priv->secrets_id, NM_ACT_STAGE_RETURN_FAILURE); + g_signal_emit (self, signals[AUTH_REQUESTED], 0); + return NM_ACT_STAGE_RETURN_POSTPONE; } /*****************************************************************************/ diff --git a/src/devices/wwan/nm-modem.h b/src/devices/wwan/nm-modem.h index 35b80ba15b..fffbeb3213 100644 --- a/src/devices/wwan/nm-modem.h +++ b/src/devices/wwan/nm-modem.h @@ -218,10 +218,10 @@ NMActStageReturn nm_modem_stage3_ip6_config_start (NMModem *modem, void nm_modem_ip4_pre_commit (NMModem *modem, NMDevice *device, NMIP4Config *config); -gboolean nm_modem_get_secrets (NMModem *modem, - const char *setting_name, - gboolean request_new, - const char *hint); +void nm_modem_get_secrets (NMModem *modem, + const char *setting_name, + gboolean request_new, + const char *hint); void nm_modem_deactivate (NMModem *modem, NMDevice *device); diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c index 3ced2a3dcf..90590838c0 100644 --- a/src/nm-activation-request.c +++ b/src/nm-activation-request.c @@ -75,16 +75,18 @@ nm_act_request_get_connection (NMActRequest *req) /*******************************************************************/ -typedef struct { +struct _NMActRequestGetSecretsCallId { NMActRequest *self; - guint32 call_id; + NMSettingsConnectionCallId call_id_s; NMActRequestSecretsFunc callback; gpointer callback_data; -} GetSecretsInfo; +}; + +typedef struct _NMActRequestGetSecretsCallId GetSecretsInfo; static void get_secrets_cb (NMSettingsConnection *connection, - guint32 call_id, + NMSettingsConnectionCallId call_id_s, const char *agent_username, const char *setting_name, GError *error, @@ -93,14 +95,36 @@ get_secrets_cb (NMSettingsConnection *connection, GetSecretsInfo *info = user_data; NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (info->self); - g_return_if_fail (info->call_id == call_id); + g_return_if_fail (info->call_id_s == call_id_s); + g_return_if_fail (g_slist_find (priv->secrets_calls, info)); + priv->secrets_calls = g_slist_remove (priv->secrets_calls, info); - info->callback (info->self, call_id, NM_CONNECTION (connection), error, info->callback_data); + if (info->callback) + info->callback (info->self, info, NM_CONNECTION (connection), error, info->callback_data); g_free (info); } -guint32 +/** + * nm_act_request_get_secrets: + * @self: + * @setting_name: + * @flags: + * @hint: + * @callback: + * @callback_data: + * + * Asnychronously starts the request for secrets. This function cannot + * fail. + * + * The return call-id can be used to cancel the request. You are + * only allowed to cancel a still pending operation (once). + * The callback will always be invoked once, even for canceling + * or disposing of NMActRequest. + * + * Returns: a call-id. + */ +NMActRequestGetSecretsCallId nm_act_request_get_secrets (NMActRequest *self, const char *setting_name, NMSecretAgentGetSecretsFlags flags, @@ -110,7 +134,7 @@ nm_act_request_get_secrets (NMActRequest *self, { NMActRequestPrivate *priv; GetSecretsInfo *info; - guint32 call_id; + NMSettingsConnectionCallId call_id_s; NMConnection *connection; const char *hints[2] = { hint, NULL }; @@ -128,50 +152,40 @@ nm_act_request_get_secrets (NMActRequest *self, flags |= NM_SECRET_AGENT_GET_SECRETS_FLAG_USER_REQUESTED; connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); - call_id = nm_settings_connection_get_secrets (NM_SETTINGS_CONNECTION (connection), - nm_active_connection_get_subject (NM_ACTIVE_CONNECTION (self)), - setting_name, - flags, - hints, - get_secrets_cb, - info, - NULL); - if (call_id > 0) { - info->call_id = call_id; + call_id_s = nm_settings_connection_get_secrets (NM_SETTINGS_CONNECTION (connection), + nm_active_connection_get_subject (NM_ACTIVE_CONNECTION (self)), + setting_name, + flags, + hints, + get_secrets_cb, + info, + NULL); + if (call_id_s) { + info->call_id_s = call_id_s; priv->secrets_calls = g_slist_append (priv->secrets_calls, info); } else g_free (info); - return call_id; + return info; } void -nm_act_request_cancel_secrets (NMActRequest *self, guint32 call_id) +nm_act_request_cancel_secrets (NMActRequest *self, NMActRequestGetSecretsCallId call_id) { NMActRequestPrivate *priv; NMConnection *connection; - GSList *iter; g_return_if_fail (self); g_return_if_fail (NM_IS_ACT_REQUEST (self)); - g_return_if_fail (call_id > 0); + g_return_if_fail (call_id); priv = NM_ACT_REQUEST_GET_PRIVATE (self); - connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); - for (iter = priv->secrets_calls; iter; iter = g_slist_next (iter)) { - GetSecretsInfo *info = iter->data; - - /* Remove the matching info */ - if (info->call_id == call_id) { - priv->secrets_calls = g_slist_remove_link (priv->secrets_calls, iter); - g_slist_free (iter); + if (g_slist_find (priv->secrets_calls, call_id)) + g_return_if_reached (); - nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), call_id); - g_free (info); - break; - } - } + connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self)); + nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), call_id->call_id_s); } /********************************************************************/ @@ -431,7 +445,6 @@ dispose (GObject *object) { NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (object); NMConnection *connection; - GSList *iter; /* Clear any share rules */ if (priv->share_rules) { @@ -441,14 +454,13 @@ dispose (GObject *object) /* Kill any in-progress secrets requests */ connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (object)); - for (iter = priv->secrets_calls; connection && iter; iter = g_slist_next (iter)) { - GetSecretsInfo *info = iter->data; + while (priv->secrets_calls) { + GetSecretsInfo *info = priv->secrets_calls->data; - nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id); - g_free (info); + nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id_s); + + g_return_if_fail (!priv->secrets_calls || info != priv->secrets_calls->data); } - g_slist_free (priv->secrets_calls); - priv->secrets_calls = NULL; G_OBJECT_CLASS (nm_act_request_parent_class)->dispose (object); } diff --git a/src/nm-activation-request.h b/src/nm-activation-request.h index b93fff5b72..8c990d520f 100644 --- a/src/nm-activation-request.h +++ b/src/nm-activation-request.h @@ -33,6 +33,9 @@ #define NM_IS_ACT_REQUEST_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_ACT_REQUEST)) #define NM_ACT_REQUEST_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_ACT_REQUEST, NMActRequestClass)) +struct _NMActRequestGetSecretsCallId; +typedef struct _NMActRequestGetSecretsCallId *NMActRequestGetSecretsCallId; + struct _NMActRequest { NMActiveConnection parent; }; @@ -62,19 +65,19 @@ void nm_act_request_add_share_rule (NMActRequest *req, /* Secrets handling */ typedef void (*NMActRequestSecretsFunc) (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data); -guint32 nm_act_request_get_secrets (NMActRequest *req, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - const char *hint, - NMActRequestSecretsFunc callback, - gpointer callback_data); +NMActRequestGetSecretsCallId nm_act_request_get_secrets (NMActRequest *req, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char *hint, + NMActRequestSecretsFunc callback, + gpointer callback_data); -void nm_act_request_cancel_secrets (NMActRequest *req, guint32 call_id); +void nm_act_request_cancel_secrets (NMActRequest *req, NMActRequestGetSecretsCallId call_id); #endif /* __NETWORKMANAGER_ACTIVATION_REQUEST_H__ */ diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c index 1ed01b6c8d..8c1c4dac4a 100644 --- a/src/nm-auth-utils.c +++ b/src/nm-auth-utils.c @@ -39,6 +39,7 @@ struct NMAuthChain { GError *error; guint idle_id; + gboolean done; NMAuthChainResultFunc done_func; gpointer user_data; @@ -56,15 +57,26 @@ typedef struct { GDestroyNotify destroy; } ChainData; +static ChainData * +chain_data_new (gpointer data, GDestroyNotify destroy) +{ + ChainData *tmp; + + tmp = g_slice_new (ChainData); + tmp->data = data; + tmp->destroy = destroy; + return tmp; +} + static void -free_data (gpointer data) +chain_data_free (gpointer data) { ChainData *tmp = data; if (tmp->destroy) tmp->destroy (tmp->data); memset (tmp, 0, sizeof (ChainData)); - g_free (tmp); + g_slice_free (ChainData, tmp); } static gboolean @@ -73,8 +85,9 @@ auth_chain_finish (gpointer user_data) NMAuthChain *self = user_data; self->idle_id = 0; + self->done = TRUE; - /* Ensure we say alive across the callback */ + /* Ensure we stay alive across the callback */ self->refcount++; self->done_func (self, self->error, self->context, self->user_data); nm_auth_chain_unref (self); @@ -116,27 +129,33 @@ nm_auth_chain_new_subject (NMAuthSubject *subject, g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL); g_return_val_if_fail (nm_auth_subject_is_unix_process (subject) || nm_auth_subject_is_internal (subject), NULL); - self = g_malloc0 (sizeof (NMAuthChain)); + self = g_slice_new0 (NMAuthChain); self->refcount = 1; - self->data = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, free_data); + self->data = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, chain_data_free); self->done_func = done_func; self->user_data = user_data; - self->context = context; + self->context = context ? g_object_ref (context) : NULL; self->subject = g_object_ref (subject); return self; } -gpointer -nm_auth_chain_get_data (NMAuthChain *self, const char *tag) +static gpointer +_get_data (NMAuthChain *self, const char *tag) { ChainData *tmp; + tmp = g_hash_table_lookup (self->data, tag); + return tmp ? tmp->data : NULL; +} + +gpointer +nm_auth_chain_get_data (NMAuthChain *self, const char *tag) +{ g_return_val_if_fail (self != NULL, NULL); g_return_val_if_fail (tag != NULL, NULL); - tmp = g_hash_table_lookup (self->data, tag); - return tmp ? tmp->data : NULL; + return _get_data (self, tag); } /** @@ -165,7 +184,7 @@ nm_auth_chain_steal_data (NMAuthChain *self, const char *tag) value = tmp->data; /* Make sure the destroy handler isn't called when freeing */ tmp->destroy = NULL; - free_data (tmp); + chain_data_free (tmp); g_free (orig_key); } return value; @@ -177,35 +196,30 @@ nm_auth_chain_set_data (NMAuthChain *self, gpointer data, GDestroyNotify data_destroy) { - ChainData *tmp; - g_return_if_fail (self != NULL); g_return_if_fail (tag != NULL); if (data == NULL) g_hash_table_remove (self->data, tag); else { - tmp = g_malloc0 (sizeof (ChainData)); - tmp->data = data; - tmp->destroy = data_destroy; - - g_hash_table_insert (self->data, g_strdup (tag), tmp); + g_hash_table_insert (self->data, + g_strdup (tag), + chain_data_new (data, data_destroy)); } } gulong nm_auth_chain_get_data_ulong (NMAuthChain *self, const char *tag) { - gulong *ptr; + gulong *data; g_return_val_if_fail (self != NULL, 0); g_return_val_if_fail (tag != NULL, 0); - ptr = nm_auth_chain_get_data (self, tag); - return *ptr; + data = _get_data (self, tag); + return data ? *data : 0ul; } - void nm_auth_chain_set_data_ulong (NMAuthChain *self, const char *tag, @@ -232,30 +246,13 @@ nm_auth_chain_get_subject (NMAuthChain *self) NMAuthCallResult nm_auth_chain_get_result (NMAuthChain *self, const char *permission) { + gpointer data; + g_return_val_if_fail (self != NULL, NM_AUTH_CALL_RESULT_UNKNOWN); g_return_val_if_fail (permission != NULL, NM_AUTH_CALL_RESULT_UNKNOWN); - return GPOINTER_TO_UINT (nm_auth_chain_get_data (self, permission)); -} - -static void -nm_auth_chain_check_done (NMAuthChain *self) -{ - g_return_if_fail (self != NULL); - - if (g_slist_length (self->calls) == 0) { - g_assert (self->idle_id == 0); - self->idle_id = g_idle_add (auth_chain_finish, self); - } -} - -static void -nm_auth_chain_remove_call (NMAuthChain *self, AuthCall *call) -{ - g_return_if_fail (self != NULL); - g_return_if_fail (call != NULL); - - self->calls = g_slist_remove (self->calls, call); + data = _get_data (self, permission); + return data ? GPOINTER_TO_UINT (data) : NM_AUTH_CALL_RESULT_UNKNOWN; } static AuthCall * @@ -263,10 +260,9 @@ auth_call_new (NMAuthChain *chain, const char *permission) { AuthCall *call; - call = g_malloc0 (sizeof (AuthCall)); + call = g_slice_new0 (AuthCall); call->chain = chain; call->permission = g_strdup (permission); - chain->calls = g_slist_append (chain->calls, call); return call; } @@ -275,15 +271,27 @@ auth_call_free (AuthCall *call) { g_free (call->permission); g_clear_object (&call->cancellable); - g_free (call); + g_slice_free (AuthCall, call); } -/* This can get used from scheduled idles, hence the boolean return */ static gboolean auth_call_complete (AuthCall *call) { - nm_auth_chain_remove_call (call->chain, call); - nm_auth_chain_check_done (call->chain); + NMAuthChain *self; + + g_return_val_if_fail (call, G_SOURCE_REMOVE); + + self = call->chain; + + g_return_val_if_fail (self, G_SOURCE_REMOVE); + g_return_val_if_fail (g_slist_find (self->calls, call), G_SOURCE_REMOVE); + + self->calls = g_slist_remove (self->calls, call); + + if (!self->calls) { + g_assert (!self->idle_id && !self->done); + self->idle_id = g_idle_add (auth_chain_finish, self); + } auth_call_free (call); return FALSE; } @@ -367,8 +375,10 @@ nm_auth_chain_add_call (NMAuthChain *self, g_return_if_fail (permission && *permission); g_return_if_fail (self->subject); g_return_if_fail (nm_auth_subject_is_unix_process (self->subject) || nm_auth_subject_is_internal (self->subject)); + g_return_if_fail (!self->idle_id && !self->done); call = auth_call_new (self, permission); + self->calls = g_slist_append (self->calls, call); if ( nm_auth_subject_is_internal (self->subject) || nm_auth_subject_get_unix_process_uid (self->subject) == 0 @@ -398,10 +408,21 @@ nm_auth_chain_add_call (NMAuthChain *self, } } +/** + * nm_auth_chain_unref: + * @self: the auth-chain + * + * Unrefs the auth-chain. By unrefing the auth-chain, you also cancel + * the receipt of the done-callback. IOW, the callback will not be invoked. + * + * The only exception is, if you call nm_auth_chain_unref() from inside + * the callback. In this case, @self stays alive until the callback returns. + */ void nm_auth_chain_unref (NMAuthChain *self) { g_return_if_fail (self != NULL); + g_return_if_fail (self->refcount > 0); self->refcount--; if (self->refcount > 0) @@ -412,13 +433,16 @@ nm_auth_chain_unref (NMAuthChain *self) g_object_unref (self->subject); + if (self->context) + g_object_unref (self->context); + g_slist_free_full (self->calls, auth_call_cancel); g_clear_error (&self->error); g_hash_table_destroy (self->data); memset (self, 0, sizeof (NMAuthChain)); - g_free (self); + g_slice_free (NMAuthChain, self); } /************ utils **************/ diff --git a/src/ppp-manager/nm-ppp-manager.c b/src/ppp-manager/nm-ppp-manager.c index dcc916902a..b5bb22c11d 100644 --- a/src/ppp-manager/nm-ppp-manager.c +++ b/src/ppp-manager/nm-ppp-manager.c @@ -63,7 +63,7 @@ typedef struct { NMActRequest *act_req; GDBusMethodInvocation *pending_secrets_context; - guint32 secrets_id; + NMActRequestGetSecretsCallId secrets_id; const char *secrets_setting_name; guint32 ppp_watch_id; @@ -229,7 +229,7 @@ static void remove_timeout_handler (NMPPPManager *manager) { NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager); - + if (priv->ppp_timeout_handler) { g_source_remove (priv->ppp_timeout_handler); priv->ppp_timeout_handler = 0; @@ -241,11 +241,10 @@ cancel_get_secrets (NMPPPManager *self) { NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (self); - if (priv->secrets_id) { + if (priv->secrets_id) nm_act_request_cancel_secrets (priv->act_req, priv->secrets_id); - priv->secrets_id = 0; - } - priv->secrets_setting_name = NULL; + + g_return_if_fail (!priv->secrets_id && !priv->secrets_setting_name); } static gboolean @@ -309,7 +308,7 @@ extract_details_from_connection (NMConnection *connection, static void ppp_secrets_cb (NMActRequest *req, - guint32 call_id, + NMActRequestGetSecretsCallId call_id, NMConnection *connection, GError *error, gpointer user_data) @@ -324,6 +323,9 @@ ppp_secrets_cb (NMActRequest *req, g_return_if_fail (req == priv->act_req); g_return_if_fail (call_id == priv->secrets_id); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + goto out; + if (error) { nm_log_warn (LOGD_PPP, "%s", error->message); g_dbus_method_invocation_return_gerror (priv->pending_secrets_context, error); @@ -348,7 +350,7 @@ ppp_secrets_cb (NMActRequest *req, out: priv->pending_secrets_context = NULL; - priv->secrets_id = 0; + priv->secrets_id = NULL; priv->secrets_setting_name = NULL; } diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index be59b18358..8685f3b21f 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -70,11 +70,31 @@ NM_DEFINE_SINGLETON_INSTANCE (NMAgentManager); } \ } G_STMT_END -#define LOG_REQ_FMT "[%p/%s%s%s]" -#define LOG_REQ_ARG(req) (req), NM_PRINT_FMT_QUOTE_STRING ((req)->detail) - -#define LOG_REQ2_FMT "[%p/%s%s%s/%s%s%s]" -#define LOG_REQ2_ARG(req) (req), NM_PRINT_FMT_QUOTE_STRING ((req)->parent.detail), NM_PRINT_FMT_QUOTE_STRING ((req)->setting_name) +#define LOG_REQ_FMT "[%p/%s%s%s%s%s%s]" +#define LOG_REQ_ARG(req) \ + (req), \ + NM_PRINT_FMT_QUOTE_STRING ((req)->detail), \ + NM_PRINT_FMT_QUOTED (((req)->request_type == REQUEST_TYPE_CON_GET) && (req)->con.get.setting_name, \ + "/\"", (req)->con.get.setting_name, "\"", \ + ((req)->request_type == REQUEST_TYPE_CON_GET ? "/(none)" : _request_type_to_string ((req)->request_type, FALSE))) + +typedef enum { + REQUEST_TYPE_INVALID, + REQUEST_TYPE_CON_GET, + REQUEST_TYPE_CON_SAVE, + REQUEST_TYPE_CON_DEL, +} RequestType; + +static const char * +_request_type_to_string (RequestType request_type, gboolean verbose) +{ + switch (request_type) { + case REQUEST_TYPE_CON_GET: return verbose ? "getting" : "get"; + case REQUEST_TYPE_CON_SAVE: return verbose ? "saving" : "sav"; + case REQUEST_TYPE_CON_DEL: return verbose ? "deleting" : "del"; + default: return "??"; + } +} G_DEFINE_TYPE (NMAgentManager, nm_agent_manager, NM_TYPE_EXPORTED_OBJECT) @@ -104,7 +124,7 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; -typedef struct _Request Request; +typedef struct _NMAgentManagerCallId Request; static void request_add_agent (Request *req, NMSecretAgent *agent); @@ -112,6 +132,12 @@ static void request_remove_agent (Request *req, NMSecretAgent *agent, GSList **p static void request_next_agent (Request *req); +static void _con_get_request_start (Request *req); +static void _con_save_request_start (Request *req); +static void _con_del_request_start (Request *req); + +static gboolean _con_get_try_complete_early (Request *req); + /*************************************************************/ static gboolean @@ -134,7 +160,7 @@ remove_agent (NMAgentManager *self, const char *owner) /* Remove this agent from any in-progress secrets requests */ g_hash_table_iter_init (&iter, priv->requests); - while (g_hash_table_iter_next (&iter, NULL, &data)) + while (g_hash_table_iter_next (&iter, &data, NULL)) request_remove_agent ((Request *) data, agent, &pending_reqs); /* We cannot call request_next_agent() from from within hash iterating loop, @@ -264,7 +290,7 @@ agent_register_permissions_done (NMAuthChain *chain, /* Add this agent to any in-progress secrets requests */ g_hash_table_iter_init (&iter, priv->requests); - while (g_hash_table_iter_next (&iter, NULL, (gpointer) &req)) + while (g_hash_table_iter_next (&iter, (gpointer) &req, NULL)) request_add_agent (req, agent); } @@ -409,23 +435,12 @@ done: /*************************************************************/ -typedef void (*RequestCompleteFunc) (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data); -typedef gboolean (*RequestAddAgentFunc) (Request *req, NMSecretAgent *agent); -typedef void (*RequestNextFunc) (Request *req); -typedef void (*RequestCancelFunc) (Request *req); - -/* Basic secrets request structure */ -struct _Request { +struct _NMAgentManagerCallId { NMAgentManager *self; - guint32 reqid; + RequestType request_type; + char *detail; - char *verb; NMAuthSubject *subject; @@ -436,66 +451,87 @@ struct _Request { /* Stores the sorted list of NMSecretAgents which will be asked for secrets */ GSList *pending; - guint32 idle_id; + guint idle_id; - RequestAddAgentFunc add_agent_callback; - RequestCancelFunc cancel_callback; - RequestNextFunc next_callback; - RequestCompleteFunc complete_callback; - gpointer complete_callback_data; - gboolean completed; + union { + struct { + NMConnection *connection; - GDestroyNotify free_func; -}; + NMAuthChain *chain; -static guint32 next_req_id = 1; + /* Whether the agent currently being asked for secrets + * has the system.modify privilege. + */ + gboolean current_has_modify; + + union { + struct { + NMSecretAgentGetSecretsFlags flags; + char *setting_name; + char **hints; + + GVariant *existing_secrets; + + NMAgentSecretsResultFunc callback; + gpointer callback_data; + gpointer other_data2; + gpointer other_data3; + } get; + }; + } con; + }; +}; static Request * -request_new (gsize struct_size, - NMAgentManager *self, +request_new (NMAgentManager *self, + RequestType request_type, const char *detail, - const char *verb, - NMAuthSubject *subject, - RequestCompleteFunc complete_callback, - gpointer complete_callback_data, - RequestAddAgentFunc add_agent_callback, - RequestNextFunc next_callback, - RequestCancelFunc cancel_callback, - GDestroyNotify free_func) + NMAuthSubject *subject) { Request *req; - req = g_malloc0 (struct_size); + req = g_slice_new0 (Request); req->self = g_object_ref (self); - req->reqid = next_req_id++; + req->request_type = request_type; req->detail = g_strdup (detail); - req->verb = g_strdup (verb); req->subject = g_object_ref (subject); - req->complete_callback = complete_callback; - req->complete_callback_data = complete_callback_data; - req->add_agent_callback = add_agent_callback, - req->next_callback = next_callback; - req->cancel_callback = cancel_callback; - req->free_func = free_func; return req; } static void request_free (Request *req) { - if (req->free_func) - req->free_func ((gpointer) req); + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + case REQUEST_TYPE_CON_SAVE: + case REQUEST_TYPE_CON_DEL: + g_object_unref (req->con.connection); + if (req->con.chain) + nm_auth_chain_unref (req->con.chain); + if (req->request_type == REQUEST_TYPE_CON_GET) { + g_free (req->con.get.setting_name); + g_strfreev (req->con.get.hints); + if (req->con.get.existing_secrets) + g_variant_unref (req->con.get.existing_secrets); + } + break; + default: + g_assert_not_reached (); + } if (req->idle_id) g_source_remove (req->idle_id); - if (!req->completed && req->cancel_callback) - req->cancel_callback (req); + if (req->current && req->current_call_id) { + /* cancel-secrets invokes the done-callback synchronously -- in which case + * the handler just return. + * Hence, we can proceed to free @req... */ + nm_secret_agent_cancel_secrets (req->current, req->current_call_id); + } g_object_unref (req->subject); g_free (req->detail); - g_free (req->verb); g_slist_free_full (req->pending, g_object_unref); g_object_unref (req->self); @@ -504,29 +540,79 @@ request_free (Request *req) g_object_unref (req->current); memset (req, 0, sizeof (Request)); - g_free (req); + g_slice_free (Request, req); } static void -req_complete_success (Request *req, +req_complete_release (Request *req, GVariant *secrets, const char *agent_dbus_owner, - const char *agent_uname) + const char *agent_username, + GError *error) { - req->completed = TRUE; - req->complete_callback (req, - secrets, - agent_dbus_owner, - agent_uname, - NULL, - req->complete_callback_data); + NMAgentManager *self = req->self; + + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + req->con.get.callback (self, + req, + agent_dbus_owner, + agent_username, + req->con.current_has_modify, + req->con.get.setting_name, + req->con.get.flags, + error ? NULL : secrets, + error, + req->con.get.callback_data, + req->con.get.other_data2, + req->con.get.other_data3); + + break; + case REQUEST_TYPE_CON_SAVE: + case REQUEST_TYPE_CON_DEL: + break; + default: + g_return_if_reached (); + } + + request_free (req); +} + +static void +req_complete_cancel (Request *req, + GQuark domain, + guint code, + const char *message) +{ + GError *error = NULL; + + nm_assert (req && req->self); + nm_assert (!g_hash_table_contains (NM_AGENT_MANAGER_GET_PRIVATE (req->self)->requests, req)); + + g_set_error_literal (&error, domain, code, message); + req_complete_release (req, NULL, NULL, NULL, error); + g_error_free (error); +} + +static void +req_complete (Request *req, + GVariant *secrets, + const char *agent_dbus_owner, + const char *agent_username, + GError *error) +{ + NMAgentManager *self = req->self; + NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); + + if (!g_hash_table_remove (priv->requests, req)) + g_return_if_reached (); + req_complete_release (req, secrets, agent_dbus_owner, agent_username, error); } static void req_complete_error (Request *req, GError *error) { - req->completed = TRUE; - req->complete_callback (req, NULL, NULL, NULL, error, req->complete_callback_data); + req_complete (req, NULL, NULL, NULL, error); } static gint @@ -575,8 +661,19 @@ request_add_agent (Request *req, NMSecretAgent *agent) self = req->self; - if (req->add_agent_callback && !req->add_agent_callback (req, agent)) - return; + if (req->request_type == REQUEST_TYPE_CON_GET) { + NMAuthSubject *subject = nm_secret_agent_get_subject (agent); + + /* Ensure the caller's username exists in the connection's permissions, + * or that the permissions is empty (ie, visible by everyone). + */ + if (!nm_auth_is_subject_in_acl (req->con.connection, subject, NULL)) { + _LOGD (agent, "agent ignored for secrets request "LOG_REQ_FMT" (not in ACL)", + LOG_REQ_ARG (req)); + /* Connection not visible to this agent's user */ + return; + } + } /* If the request should filter agents by UID, do that now */ if (nm_auth_subject_is_unix_process (req->subject)) { @@ -636,9 +733,22 @@ request_next_agent (Request *req) req->pending = g_slist_remove (req->pending, req->current); _LOGD (req->current, "agent %s secrets for request "LOG_REQ_FMT, - req->verb, LOG_REQ_ARG (req)); + _request_type_to_string (req->request_type, TRUE), + LOG_REQ_ARG (req)); - req->next_callback (req); + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + _con_get_request_start (req); + break; + case REQUEST_TYPE_CON_SAVE: + _con_save_request_start (req); + break; + case REQUEST_TYPE_CON_DEL: + _con_del_request_start (req); + break; + default: + g_assert_not_reached (); + } } else { /* No more secret agents are available to fulfill this secrets request */ error = g_error_new_literal (NM_AGENT_MANAGER_ERROR, @@ -681,204 +791,82 @@ request_start (gpointer user_data) Request *req = user_data; req->idle_id = 0; - request_next_agent (req); - return FALSE; -} - -/*************************************************************/ - -/* Request subclass for connection secrets */ -typedef struct { - Request parent; - - NMSecretAgentGetSecretsFlags flags; - NMConnection *connection; - char *setting_name; - char **hints; - - GVariant *existing_secrets; - - NMAgentSecretsResultFunc callback; - gpointer callback_data; - gpointer other_data2; - gpointer other_data3; - NMAuthChain *chain; - - /* Whether the agent currently being asked for secrets - * has the system.modify privilege. - */ - gboolean current_has_modify; -} ConnectionRequest; - -static void -connection_request_free (gpointer data) -{ - ConnectionRequest *req = data; - - g_object_unref (req->connection); - g_free (req->setting_name); - g_strfreev (req->hints); - if (req->existing_secrets) - g_variant_unref (req->existing_secrets); - if (req->chain) - nm_auth_chain_unref (req->chain); -} - -static gboolean -connection_request_add_agent (Request *parent, NMSecretAgent *agent) -{ - NMAgentManager *self; - ConnectionRequest *req = (ConnectionRequest *) parent; - NMAuthSubject *subject = nm_secret_agent_get_subject(agent); - - self = parent->self; - - /* Ensure the caller's username exists in the connection's permissions, - * or that the permissions is empty (ie, visible by everyone). - */ - if (!nm_auth_is_subject_in_acl (req->connection, subject, NULL)) { - _LOGD (agent, "agent ignored for secrets request "LOG_REQ_FMT" (not in ACL)", - LOG_REQ_ARG (parent)); - /* Connection not visible to this agent's user */ - return FALSE; + switch (req->request_type) { + case REQUEST_TYPE_CON_GET: + if (_con_get_try_complete_early (req)) + goto out; + break; + default: + break; } + request_next_agent (req); - return TRUE; -} - -static ConnectionRequest * -connection_request_new_get (NMAgentManager *self, - NMConnection *connection, - NMAuthSubject *subject, - GVariant *existing_secrets, - const char *setting_name, - const char *verb, - NMSecretAgentGetSecretsFlags flags, - const char **hints, - NMAgentSecretsResultFunc callback, - gpointer callback_data, - gpointer other_data2, - gpointer other_data3, - RequestCompleteFunc complete_callback, - gpointer complete_callback_data, - RequestNextFunc next_callback, - RequestCancelFunc cancel_callback) -{ - ConnectionRequest *req; - - req = (ConnectionRequest *) request_new (sizeof (ConnectionRequest), - self, - nm_connection_get_id (connection), - verb, - subject, - complete_callback, - complete_callback_data, - connection_request_add_agent, - next_callback, - cancel_callback, - connection_request_free); - g_assert (req); - - req->connection = g_object_ref (connection); - if (existing_secrets) - req->existing_secrets = g_variant_ref (existing_secrets); - req->setting_name = g_strdup (setting_name); - req->hints = g_strdupv ((char **) hints); - req->flags = flags; - req->callback = callback; - req->callback_data = callback_data; - req->other_data2 = other_data2; - req->other_data3 = other_data3; - return req; +out: + return FALSE; } -static ConnectionRequest * -connection_request_new_other (NMAgentManager *self, - NMConnection *connection, - NMAuthSubject *subject, - const char *verb, - RequestCompleteFunc complete_callback, - gpointer complete_callback_data, - RequestNextFunc next_callback) -{ - ConnectionRequest *req; - - req = (ConnectionRequest *) request_new (sizeof (ConnectionRequest), - self, - nm_connection_get_id (connection), - verb, - subject, - complete_callback, - complete_callback_data, - NULL, - next_callback, - NULL, - connection_request_free); - g_assert (req); - req->connection = g_object_ref (connection); - return req; -} +/*************************************************************/ static void -get_done_cb (NMSecretAgent *agent, - NMSecretAgentCallId call_id, - GVariant *secrets, - GError *error, - gpointer user_data) +_con_get_request_done (NMSecretAgent *agent, + NMSecretAgentCallId call_id, + GVariant *secrets, + GError *error, + gpointer user_data) { NMAgentManager *self; - Request *parent = user_data; - ConnectionRequest *req = user_data; + Request *req = user_data; GVariant *setting_secrets; const char *agent_dbus_owner; struct passwd *pw; char *agent_uname = NULL; - g_return_if_fail (call_id == parent->current_call_id); - g_return_if_fail (agent == parent->current); + g_return_if_fail (call_id == req->current_call_id); + g_return_if_fail (agent == req->current); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); - self = parent->self; + self = req->self; - parent->current_call_id = NULL; + req->current_call_id = NULL; if (error) { if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { - _LOGD (agent, "get secrets request cancelled: "LOG_REQ2_FMT, - LOG_REQ2_ARG (req)); + _LOGD (agent, "get secrets request cancelled: "LOG_REQ_FMT, + LOG_REQ_ARG (req)); return; } - _LOGD (agent, "agent failed secrets request "LOG_REQ2_FMT": %s", - LOG_REQ2_ARG (req), + _LOGD (agent, "agent failed secrets request "LOG_REQ_FMT": %s", + LOG_REQ_ARG (req), error->message); if (g_error_matches (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_USER_CANCELED)) { error = g_error_new_literal (NM_AGENT_MANAGER_ERROR, NM_AGENT_MANAGER_ERROR_USER_CANCELED, "User canceled the secrets request."); - req_complete_error (parent, error); + req_complete_error (req, error); g_error_free (error); } else { /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); maybe_remove_agent_on_error (agent, error); } return; } /* Ensure the setting we wanted secrets for got returned and has something in it */ - setting_secrets = g_variant_lookup_value (secrets, req->setting_name, NM_VARIANT_TYPE_SETTING); + setting_secrets = g_variant_lookup_value (secrets, req->con.get.setting_name, NM_VARIANT_TYPE_SETTING); if (!setting_secrets || !g_variant_n_children (setting_secrets)) { - _LOGD (agent, "agent returned no secrets for request "LOG_REQ2_FMT, - LOG_REQ2_ARG (req)); + _LOGD (agent, "agent returned no secrets for request "LOG_REQ_FMT, + LOG_REQ_ARG (req)); /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); return; } - _LOGD (agent, "agent returned secrets for request "LOG_REQ2_FMT, - LOG_REQ2_ARG (req)); + _LOGD (agent, "agent returned secrets for request "LOG_REQ_FMT, + LOG_REQ_ARG (req)); /* Get the agent's username */ pw = getpwuid (nm_secret_agent_get_owner_uid (agent)); @@ -889,7 +877,7 @@ get_done_cb (NMSecretAgent *agent, } agent_dbus_owner = nm_secret_agent_get_dbus_owner (agent); - req_complete_success (parent, secrets, agent_dbus_owner, agent_uname); + req_complete (req, secrets, agent_dbus_owner, agent_uname, NULL); g_free (agent_uname); } @@ -934,61 +922,63 @@ set_secrets_not_required (NMConnection *connection, GVariant *dict) } static void -get_agent_request_secrets (ConnectionRequest *req, gboolean include_system_secrets) +_con_get_request_start_proceed (Request *req, gboolean include_system_secrets) { - Request *parent = (Request *) req; NMConnection *tmp; - tmp = nm_simple_connection_new_clone (req->connection); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); + + tmp = nm_simple_connection_new_clone (req->con.connection); nm_connection_clear_secrets (tmp); if (include_system_secrets) { - if (req->existing_secrets) - (void) nm_connection_update_secrets (tmp, req->setting_name, req->existing_secrets, NULL); + if (req->con.get.existing_secrets) + (void) nm_connection_update_secrets (tmp, req->con.get.setting_name, req->con.get.existing_secrets, NULL); } else { /* Update secret flags in the temporary connection to indicate that * the system secrets we're not sending to the agent aren't required, * so the agent can properly validate UI controls and such. */ - if (req->existing_secrets) - set_secrets_not_required (tmp, req->existing_secrets); + if (req->con.get.existing_secrets) + set_secrets_not_required (tmp, req->con.get.existing_secrets); } - parent->current_call_id = nm_secret_agent_get_secrets (parent->current, - tmp, - req->setting_name, - (const char **) req->hints, - req->flags, - get_done_cb, - req); - if (!parent->current_call_id) { + req->current_call_id = nm_secret_agent_get_secrets (req->current, + tmp, + req->con.get.setting_name, + (const char **) req->con.get.hints, + req->con.get.flags, + _con_get_request_done, + req); + if (!req->current_call_id) { g_warn_if_reached (); - request_next_agent (parent); + request_next_agent (req); } g_object_unref (tmp); } static void -get_agent_modify_auth_cb (NMAuthChain *chain, - GError *error, - GDBusMethodInvocation *context, - gpointer user_data) +_con_get_request_start_validated (NMAuthChain *chain, + GError *error, + GDBusMethodInvocation *context, + gpointer user_data) { NMAgentManager *self; - Request *parent = user_data; - ConnectionRequest *req = user_data; + Request *req = user_data; const char *perm; - self = parent->self; + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_GET); + + self = req->self; - req->chain = NULL; + req->con.chain = NULL; if (error) { - _LOGD (parent->current, "agent "LOG_REQ2_FMT" MODIFY check error: (%d) %s", - LOG_REQ2_ARG (req), + _LOGD (req->current, "agent "LOG_REQ_FMT" MODIFY check error: (%d) %s", + LOG_REQ_ARG (req), error->code, error->message ? error->message : "(unknown)"); /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); } else { /* If the agent obtained the 'modify' permission, we send all system secrets * to it. If it didn't, we still ask it for secrets, but we don't send @@ -997,24 +987,24 @@ get_agent_modify_auth_cb (NMAuthChain *chain, perm = nm_auth_chain_get_data (chain, "perm"); g_assert (perm); if (nm_auth_chain_get_result (chain, perm) == NM_AUTH_CALL_RESULT_YES) - req->current_has_modify = TRUE; + req->con.current_has_modify = TRUE; - _LOGD (parent->current, "agent "LOG_REQ2_FMT" MODIFY check result %s", - LOG_REQ2_ARG (req), - req->current_has_modify ? "YES" : "NO"); + _LOGD (req->current, "agent "LOG_REQ_FMT" MODIFY check result %s", + LOG_REQ_ARG (req), + req->con.current_has_modify ? "YES" : "NO"); - get_agent_request_secrets (req, req->current_has_modify); + _con_get_request_start_proceed (req, req->con.current_has_modify); } nm_auth_chain_unref (chain); } static void -check_system_secrets_cb (NMSetting *setting, - const char *key, - const GValue *value, - GParamFlags flags, - gpointer user_data) +has_system_secrets_check (NMSetting *setting, + const char *key, + const GValue *value, + GParamFlags flags, + gpointer user_data) { NMSettingSecretFlags secret_flags = NM_SETTING_SECRET_FLAG_NONE; gboolean *has_system = user_data; @@ -1048,23 +1038,22 @@ has_system_secrets (NMConnection *connection) { gboolean has_system = FALSE; - nm_connection_for_each_setting_value (connection, check_system_secrets_cb, &has_system); + nm_connection_for_each_setting_value (connection, has_system_secrets_check, &has_system); return has_system; } static void -get_next_cb (Request *parent) +_con_get_request_start (Request *req) { NMAgentManager *self; - ConnectionRequest *req = (ConnectionRequest *) parent; NMSettingConnection *s_con; const char *agent_dbus_owner, *perm; - self = parent->self; + self = req->self; - req->current_has_modify = FALSE; + req->con.current_has_modify = FALSE; - agent_dbus_owner = nm_secret_agent_get_dbus_owner (parent->current); + agent_dbus_owner = nm_secret_agent_get_dbus_owner (req->current); /* If the request flags allow user interaction, and there are existing * system secrets (or blank secrets that are supposed to be system-owned), @@ -1072,148 +1061,123 @@ get_next_cb (Request *parent) * secrets to the agent. We shouldn't leak system-owned secrets to * unprivileged users. */ - if ( (req->flags != NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) - && (req->existing_secrets || has_system_secrets (req->connection))) { - _LOGD (NULL, "("LOG_REQ2_FMT") request has system secrets; checking agent %s for MODIFY", - LOG_REQ2_ARG (req), agent_dbus_owner); + if ( (req->con.get.flags != NM_SECRET_AGENT_GET_SECRETS_FLAG_NONE) + && (req->con.get.existing_secrets || has_system_secrets (req->con.connection))) { + _LOGD (NULL, "("LOG_REQ_FMT") request has system secrets; checking agent %s for MODIFY", + LOG_REQ_ARG (req), agent_dbus_owner); - req->chain = nm_auth_chain_new_subject (nm_secret_agent_get_subject (parent->current), - NULL, - get_agent_modify_auth_cb, - req); - g_assert (req->chain); + req->con.chain = nm_auth_chain_new_subject (nm_secret_agent_get_subject (req->current), + NULL, + _con_get_request_start_validated, + req); + g_assert (req->con.chain); /* If the caller is the only user in the connection's permissions, then * we use the 'modify.own' permission instead of 'modify.system'. If the * request affects more than just the caller, require 'modify.system'. */ - s_con = nm_connection_get_setting_connection (req->connection); + s_con = nm_connection_get_setting_connection (req->con.connection); g_assert (s_con); if (nm_setting_connection_get_num_permissions (s_con) == 1) perm = NM_AUTH_PERMISSION_SETTINGS_MODIFY_OWN; else perm = NM_AUTH_PERMISSION_SETTINGS_MODIFY_SYSTEM; - nm_auth_chain_set_data (req->chain, "perm", (gpointer) perm, NULL); + nm_auth_chain_set_data (req->con.chain, "perm", (gpointer) perm, NULL); - nm_auth_chain_add_call (req->chain, perm, TRUE); + nm_auth_chain_add_call (req->con.chain, perm, TRUE); } else { - _LOGD (NULL, "("LOG_REQ2_FMT") requesting user-owned secrets from agent %s", - LOG_REQ2_ARG (req), agent_dbus_owner); + _LOGD (NULL, "("LOG_REQ_FMT") requesting user-owned secrets from agent %s", + LOG_REQ_ARG (req), agent_dbus_owner); - get_agent_request_secrets (req, FALSE); + _con_get_request_start_proceed (req, FALSE); } } static gboolean -get_start (gpointer user_data) +_con_get_try_complete_early (Request *req) { NMAgentManager *self; - Request *parent = user_data; - ConnectionRequest *req = user_data; - GVariant *setting_secrets = NULL; - - self = parent->self; + gs_unref_variant GVariant *setting_secrets = NULL; + gs_unref_object NMConnection *tmp = NULL; + GError *error = NULL; - parent->idle_id = 0; + self = req->self; /* Check if there are any existing secrets */ - if (req->existing_secrets) - setting_secrets = g_variant_lookup_value (req->existing_secrets, req->setting_name, NM_VARIANT_TYPE_SETTING); - - if (setting_secrets && g_variant_n_children (setting_secrets)) { - NMConnection *tmp; - GError *error = NULL; - gboolean new_secrets = (req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW); - - /* The connection already had secrets; check if any more are required. - * If no more are required, we're done. If secrets are still needed, - * ask a secret agent for more. This allows admins to provide generic - * secrets but allow additional user-specific ones as well. - */ - tmp = nm_simple_connection_new_clone (req->connection); - g_assert (tmp); + if (req->con.get.existing_secrets) + setting_secrets = g_variant_lookup_value (req->con.get.existing_secrets, req->con.get.setting_name, NM_VARIANT_TYPE_SETTING); - if (!nm_connection_update_secrets (tmp, req->setting_name, req->existing_secrets, &error)) { - req_complete_error (parent, error); - g_clear_error (&error); - } else { - /* Do we have everything we need? */ - if ( (req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM) - || ((nm_connection_need_secrets (tmp, NULL) == NULL) && (new_secrets == FALSE))) { - _LOGD (NULL, "("LOG_REQ2_FMT") system settings secrets sufficient", - LOG_REQ2_ARG (req)); - - /* Got everything, we're done */ - req_complete_success (parent, req->existing_secrets, NULL, NULL); - } else { - _LOGD (NULL, "("LOG_REQ2_FMT") system settings secrets insufficient, asking agents", - LOG_REQ2_ARG (req)); - - /* We don't, so ask some agents for additional secrets */ - if ( req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS - && !parent->pending) { - /* The request initiated from GetSecrets() via DBus, - * don't error out if any secrets are missing. */ - req_complete_success (parent, req->existing_secrets, NULL, NULL); - } else - request_next_agent (parent); - } - } - g_object_unref (tmp); - } else { - /* Couldn't get secrets from system settings, so now we ask the - * agents for secrets. Let the Agent Manager handle which agents - * we'll ask and in which order. - */ - request_next_agent (parent); - } + if (!setting_secrets || !g_variant_n_children (setting_secrets)) + return FALSE; - if (setting_secrets) - g_variant_unref (setting_secrets); + /* The connection already had secrets; check if any more are required. + * If no more are required, we're done. If secrets are still needed, + * ask a secret agent for more. This allows admins to provide generic + * secrets but allow additional user-specific ones as well. + */ + tmp = nm_simple_connection_new_clone (req->con.connection); + g_assert (tmp); - return FALSE; -} + if (!nm_connection_update_secrets (tmp, req->con.get.setting_name, req->con.get.existing_secrets, &error)) { + req_complete_error (req, error); + g_clear_error (&error); + return TRUE; + } + /* Do we have everything we need? */ + if ( NM_FLAGS_HAS (req->con.get.flags, NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM) + || ( (nm_connection_need_secrets (tmp, NULL) == NULL) + && !NM_FLAGS_HAS(req->con.get.flags, NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW))) { + _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets sufficient", + LOG_REQ_ARG (req)); -static void -get_complete_cb (Request *parent, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data) -{ - NMAgentManager *self = NM_AGENT_MANAGER (user_data); - NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - ConnectionRequest *req = (ConnectionRequest *) parent; - - /* Send secrets back to the requesting object */ - req->callback (self, - parent->reqid, - agent_dbus_owner, - agent_username, - req->current_has_modify, - req->setting_name, - req->flags, - error ? NULL : secrets, - error, - req->callback_data, - req->other_data2, - req->other_data3); - - g_hash_table_remove (priv->requests, GUINT_TO_POINTER (parent->reqid)); -} + /* Got everything, we're done */ + req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); + return TRUE; + } -static void -get_cancel_cb (Request *parent) -{ - ConnectionRequest *req = (ConnectionRequest *) parent; + _LOGD (NULL, "("LOG_REQ_FMT") system settings secrets insufficient, asking agents", + LOG_REQ_ARG (req)); + + /* We don't, so ask some agents for additional secrets */ + if ( req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS + && !req->pending) { + /* The request initiated from GetSecrets() via DBus, + * don't error out if any secrets are missing. */ + req_complete (req, req->con.get.existing_secrets, NULL, NULL, NULL); + return TRUE; + } - req->current_has_modify = FALSE; - if (parent->current && parent->current_call_id) - nm_secret_agent_cancel_secrets (parent->current, parent->current_call_id); + /* Couldn't get secrets from system settings, so now we ask the + * agents for secrets. Let the Agent Manager handle which agents + * we'll ask and in which order. + */ + return FALSE; } -guint32 +/** + * nm_agent_manager_get_secrets: + * @self: + * @connection: + * @subject: + * @existing_secrets: + * @flags: + * @hints: + * @callback: + * @callback_data: + * @other_data2: + * @other_data3: + * + * Requests secrets for a connection. + * + * This function cannot fail. The callback will be invoked + * asynchrnously, but it will always be invoked exactly once. + * Even for cancellation and disposing of @self. In those latter + * cases, the callback is invoked synchrnously during the cancellation/ + * disposal. + * + * Returns: a call-id to cancel the call. + */ +NMAgentManagerCallId nm_agent_manager_get_secrets (NMAgentManager *self, NMConnection *connection, NMAuthSubject *subject, @@ -1227,8 +1191,7 @@ nm_agent_manager_get_secrets (NMAgentManager *self, gpointer other_data3) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - Request *parent; - ConnectionRequest *req; + Request *req; g_return_val_if_fail (self != NULL, 0); g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); @@ -1245,161 +1208,147 @@ nm_agent_manager_get_secrets (NMAgentManager *self, * This in turn depends on nm_connection_to_dbus() and nm_setting_to_hash() * both returning NULL if they didn't hash anything. */ + req = request_new (self, + REQUEST_TYPE_CON_GET, + nm_connection_get_id (connection), + subject); - req = connection_request_new_get (self, - connection, - subject, - existing_secrets, - setting_name, - "getting", - flags, - hints, - callback, - callback_data, - other_data2, - other_data3, - get_complete_cb, - self, - get_next_cb, - get_cancel_cb); - parent = (Request *) req; - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (parent->reqid), req); + req->con.connection = g_object_ref (connection); + if (existing_secrets) + req->con.get.existing_secrets = g_variant_ref (existing_secrets); + req->con.get.setting_name = g_strdup (setting_name); + req->con.get.hints = g_strdupv ((char **) hints); + req->con.get.flags = flags; + req->con.get.callback = callback; + req->con.get.callback_data = callback_data; + req->con.get.other_data2 = other_data2; + req->con.get.other_data3 = other_data3; + + if (!g_hash_table_add (priv->requests, req)) + g_assert_not_reached (); /* Kick off the request */ - if (!(req->flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)) - request_add_agents (self, parent); - parent->idle_id = g_idle_add (get_start, req); - return parent->reqid; + if (!(req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM)) + request_add_agents (self, req); + req->idle_id = g_idle_add (request_start, req); + return req; } void nm_agent_manager_cancel_secrets (NMAgentManager *self, - guint32 request_id) + NMAgentManagerCallId request_id) { g_return_if_fail (self != NULL); - g_return_if_fail (request_id > 0); + g_return_if_fail (request_id); + g_return_if_fail (request_id->request_type == REQUEST_TYPE_CON_GET); + + if (!g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, + request_id)) + g_return_if_reached (); - g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (self)->requests, - GUINT_TO_POINTER (request_id)); + req_complete_cancel (request_id, G_IO_ERROR, G_IO_ERROR_CANCELLED, "Request cancelled"); } /*************************************************************/ static void -save_done_cb (NMSecretAgent *agent, - NMSecretAgentCallId call_id, - GVariant *secrets, - GError *error, - gpointer user_data) +_con_save_request_done (NMSecretAgent *agent, + NMSecretAgentCallId call_id, + GVariant *secrets, + GError *error, + gpointer user_data) { NMAgentManager *self; - ConnectionRequest *req = user_data; - Request *parent = (Request *) req; + Request *req = user_data; const char *agent_dbus_owner; - g_return_if_fail (call_id == parent->current_call_id); - g_return_if_fail (agent == parent->current); + g_return_if_fail (call_id == req->current_call_id); + g_return_if_fail (agent == req->current); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_SAVE); - self = parent->self; + self = req->self; - parent->current_call_id = NULL; + req->current_call_id = NULL; if (error) { if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { _LOGD (agent, "save secrets request cancelled: "LOG_REQ_FMT, - LOG_REQ_ARG (parent)); + LOG_REQ_ARG (req)); return; } _LOGD (agent, "agent failed save secrets request "LOG_REQ_FMT": %s", - LOG_REQ_ARG (parent), error->message); + LOG_REQ_ARG (req), error->message); /* Try the next agent */ - request_next_agent (parent); + request_next_agent (req); maybe_remove_agent_on_error (agent, error); return; } _LOGD (agent, "agent saved secrets for request "LOG_REQ_FMT, - LOG_REQ_ARG (parent)); + LOG_REQ_ARG (req)); agent_dbus_owner = nm_secret_agent_get_dbus_owner (agent); - req_complete_success (parent, NULL, NULL, agent_dbus_owner); + req_complete (req, NULL, NULL, agent_dbus_owner, NULL); } static void -save_next_cb (Request *parent) +_con_save_request_start (Request *req) { - ConnectionRequest *req = (ConnectionRequest *) parent; - - parent->current_call_id = nm_secret_agent_save_secrets (parent->current, - req->connection, - save_done_cb, - req); - if (!parent->current_call_id) { + req->current_call_id = nm_secret_agent_save_secrets (req->current, + req->con.connection, + _con_save_request_done, + req); + if (!req->current_call_id) { g_warn_if_reached (); - request_next_agent (parent); + request_next_agent (req); } } -static void -save_complete_cb (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data) -{ - g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (user_data)->requests, - GUINT_TO_POINTER (req->reqid)); -} - -guint32 +void nm_agent_manager_save_secrets (NMAgentManager *self, NMConnection *connection, NMAuthSubject *subject) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); - ConnectionRequest *req; - Request *parent; + Request *req; - g_return_val_if_fail (self != NULL, 0); - g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); + g_return_if_fail (self); + g_return_if_fail (NM_IS_CONNECTION (connection)); nm_log_dbg (LOGD_SETTINGS, "Saving secrets for connection %s (%s)", nm_connection_get_path (connection), nm_connection_get_id (connection)); - req = connection_request_new_other (self, - connection, - subject, - "saving", - save_complete_cb, - self, - save_next_cb); - parent = (Request *) req; - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (parent->reqid), req); + req = request_new (self, + REQUEST_TYPE_CON_SAVE, + nm_connection_get_id (connection), + subject); + req->con.connection = g_object_ref (connection); + if (!g_hash_table_add (priv->requests, req)) + g_assert_not_reached (); /* Kick off the request */ - request_add_agents (self, parent); - parent->idle_id = g_idle_add (request_start, req); - return parent->reqid; + request_add_agents (self, req); + req->idle_id = g_idle_add (request_start, req); } /*************************************************************/ static void -delete_done_cb (NMSecretAgent *agent, - NMSecretAgentCallId call_id, - GVariant *secrets, - GError *error, - gpointer user_data) +_con_del_request_done (NMSecretAgent *agent, + NMSecretAgentCallId call_id, + GVariant *secrets, + GError *error, + gpointer user_data) { NMAgentManager *self; Request *req = user_data; g_return_if_fail (call_id == req->current_call_id); g_return_if_fail (agent == req->current); + g_return_if_fail (req->request_type == REQUEST_TYPE_CON_DEL); self = req->self; @@ -1426,43 +1375,28 @@ delete_done_cb (NMSecretAgent *agent, } static void -delete_next_cb (Request *parent) +_con_del_request_start (Request *req) { - ConnectionRequest *req = (ConnectionRequest *) parent; - - parent->current_call_id = nm_secret_agent_delete_secrets (parent->current, - req->connection, - delete_done_cb, - req); - if (!parent->current_call_id) { + req->current_call_id = nm_secret_agent_delete_secrets (req->current, + req->con.connection, + _con_del_request_done, + req); + if (!req->current_call_id) { g_warn_if_reached (); - request_next_agent (parent); + request_next_agent (req); } } -static void -delete_complete_cb (Request *req, - GVariant *secrets, - const char *agent_dbus_owner, - const char *agent_username, - GError *error, - gpointer user_data) -{ - g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (user_data)->requests, - GUINT_TO_POINTER (req->reqid)); -} - -guint32 +void nm_agent_manager_delete_secrets (NMAgentManager *self, NMConnection *connection) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); NMAuthSubject *subject; - ConnectionRequest *req; - Request *parent; + Request *req; - g_return_val_if_fail (self != NULL, 0); - g_return_val_if_fail (NM_IS_CONNECTION (connection), 0); + g_return_if_fail (self != NULL); + g_return_if_fail (NM_IS_CONNECTION (connection)); nm_log_dbg (LOGD_SETTINGS, "Deleting secrets for connection %s (%s)", @@ -1470,21 +1404,18 @@ nm_agent_manager_delete_secrets (NMAgentManager *self, nm_connection_get_id (connection)); subject = nm_auth_subject_new_internal (); - req = connection_request_new_other (self, - connection, - subject, - "deleting", - delete_complete_cb, - self, - delete_next_cb); + req = request_new (self, + REQUEST_TYPE_CON_DEL, + nm_connection_get_id (connection), + subject); + req->con.connection = g_object_ref (connection); g_object_unref (subject); - parent = (Request *) req; - g_hash_table_insert (priv->requests, GUINT_TO_POINTER (parent->reqid), req); + if (!g_hash_table_add (priv->requests, req)) + g_assert_not_reached (); /* Kick off the request */ - request_add_agents (self, parent); - parent->idle_id = g_idle_add (request_start, req); - return parent->reqid; + request_add_agents (self, req); + req->idle_id = g_idle_add (request_start, req); } /*************************************************************/ @@ -1605,10 +1536,7 @@ nm_agent_manager_init (NMAgentManager *self) NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); priv->agents = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); - priv->requests = g_hash_table_new_full (g_direct_hash, - g_direct_equal, - NULL, - (GDestroyNotify) request_free); + priv->requests = g_hash_table_new (g_direct_hash, g_direct_equal); } static void @@ -1635,6 +1563,19 @@ dispose (GObject *object) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (object); + if (priv->requests) { + GHashTableIter iter; + Request *req; + + g_hash_table_iter_init (&iter, priv->requests); + while (g_hash_table_iter_next (&iter, (gpointer *) &req, NULL)) { + g_hash_table_iter_remove (&iter); + req_complete_cancel (req, G_IO_ERROR, G_IO_ERROR_FAILED, "Disposing NMAgentManagerClass instance"); + } + g_hash_table_destroy (priv->requests); + priv->requests = NULL; + } + g_slist_free_full (priv->chains, (GDestroyNotify) nm_auth_chain_unref); priv->chains = NULL; @@ -1642,10 +1583,6 @@ dispose (GObject *object) g_hash_table_destroy (priv->agents); priv->agents = NULL; } - if (priv->requests) { - g_hash_table_destroy (priv->requests); - priv->requests = NULL; - } if (priv->auth_mgr) { g_signal_handlers_disconnect_by_func (priv->auth_mgr, diff --git a/src/settings/nm-agent-manager.h b/src/settings/nm-agent-manager.h index 792ee572d3..35564aa103 100644 --- a/src/settings/nm-agent-manager.h +++ b/src/settings/nm-agent-manager.h @@ -33,6 +33,9 @@ #define NM_IS_AGENT_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_AGENT_MANAGER)) #define NM_AGENT_MANAGER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_AGENT_MANAGER, NMAgentManagerClass)) +struct _NMAgentManagerCallId; +typedef struct _NMAgentManagerCallId *NMAgentManagerCallId; + struct _NMAgentManager { NMExportedObject parent; }; @@ -50,7 +53,7 @@ NMAgentManager *nm_agent_manager_get (void); /* If no agent fulfilled the secrets request, agent_dbus_owner will be NULL */ typedef void (*NMAgentSecretsResultFunc) (NMAgentManager *manager, - guint32 call_id, + NMAgentManagerCallId call_id, const char *agent_dbus_owner, const char *agent_uname, gboolean agent_has_modify, @@ -62,27 +65,27 @@ typedef void (*NMAgentSecretsResultFunc) (NMAgentManager *manager, gpointer other_data2, gpointer other_data3); -guint32 nm_agent_manager_get_secrets (NMAgentManager *manager, - NMConnection *connection, - NMAuthSubject *subject, - GVariant *existing_secrets, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - const char **hints, - NMAgentSecretsResultFunc callback, - gpointer callback_data, - gpointer other_data2, - gpointer other_data3); +NMAgentManagerCallId nm_agent_manager_get_secrets (NMAgentManager *manager, + NMConnection *connection, + NMAuthSubject *subject, + GVariant *existing_secrets, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char **hints, + NMAgentSecretsResultFunc callback, + gpointer callback_data, + gpointer other_data2, + gpointer other_data3); void nm_agent_manager_cancel_secrets (NMAgentManager *manager, - guint32 request_id); + NMAgentManagerCallId request_id); -guint32 nm_agent_manager_save_secrets (NMAgentManager *manager, - NMConnection *connection, - NMAuthSubject *subject); +void nm_agent_manager_save_secrets (NMAgentManager *manager, + NMConnection *connection, + NMAuthSubject *subject); -guint32 nm_agent_manager_delete_secrets (NMAgentManager *manager, - NMConnection *connection); +void nm_agent_manager_delete_secrets (NMAgentManager *manager, + NMConnection *connection); NMSecretAgent *nm_agent_manager_get_agent_by_user (NMAgentManager *manager, const char *username); diff --git a/src/settings/nm-secret-agent.c b/src/settings/nm-secret-agent.c index d77f5e0e29..ca30d46211 100644 --- a/src/settings/nm-secret-agent.c +++ b/src/settings/nm-secret-agent.c @@ -446,6 +446,17 @@ do_cancel_secrets (NMSecretAgent *self, Request *r, gboolean disposing) } } +/** + * nm_secret_agent_cancel_secrets: + * @self: #NMSecretAgent instance + * @call_id: the call id to cancel + * + * It is an error to pass an invalid @call_id or a @call_id for an operation + * that already completed. NMSecretAgent will always invoke the callback, + * also for cancel() and dispose(). + * In case of nm_secret_agent_cancel_secrets() this will synchronously invoke the + * callback before nm_secret_agent_cancel_secrets() returns. + */ void nm_secret_agent_cancel_secrets (NMSecretAgent *self, NMSecretAgentCallId call_id) { diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 8e0c2a7276..918b03ecfa 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -72,6 +72,18 @@ G_DEFINE_TYPE_WITH_CODE (NMSettingsConnection, nm_settings_connection, NM_TYPE_E NM_TYPE_SETTINGS_CONNECTION, \ NMSettingsConnectionPrivate)) +static inline NMAgentManagerCallId +NM_AGENT_MANAGER_CALL_ID (NMSettingsConnectionCallId call_id) +{ + return (NMAgentManagerCallId) call_id; +} + +static inline NMSettingsConnectionCallId +NM_SETTINGS_CONNECTION_CALL_ID (NMAgentManagerCallId call_id_a) +{ + return (NMSettingsConnectionCallId) call_id_a; +} + enum { PROP_0 = 0, PROP_VISIBLE, @@ -103,7 +115,14 @@ typedef struct { GSList *pending_auths; /* List of pending authentication requests */ gboolean visible; /* Is this connection is visible by some session? */ - GSList *reqs; /* in-progress secrets requests */ + + GSList *reqs_int; /* in-progress internal secrets requests */ + + GSList *reqs_ext; /* in-progress external secrets requests (D-Bus). Note that + this list contains NMSettingsConnectionCallId, vs. NMAgentManagerCallId. + So, we should for example cancel the reqs_ext with nm_settings_connection_cancel_secrets() + instead of nm_agent_manager_cancel_secrets(). + Actually, the difference doesn't matter, because both are indeed equivalent. */ /* Caches secrets from on-disk connections; were they not cached any * call to nm_connection_clear_secrets() wipes them out and we'd have @@ -793,8 +812,18 @@ new_secrets_commit_cb (NMSettingsConnection *self, } static void +_callback_nop (NMSettingsConnection *self, + NMSettingsConnectionCallId call_id, + const char *agent_username, + const char *setting_name, + GError *error, + gpointer user_data) +{ +} + +static void agent_secrets_done_cb (NMAgentManager *manager, - guint32 call_id, + NMAgentManagerCallId call_id_a, const char *agent_dbus_owner, const char *agent_username, gboolean agent_has_modify, @@ -806,6 +835,7 @@ agent_secrets_done_cb (NMAgentManager *manager, gpointer other_data2, gpointer other_data3) { + NMSettingsConnectionCallId call_id = NM_SETTINGS_CONNECTION_CALL_ID (call_id_a); NMSettingsConnection *self = NM_SETTINGS_CONNECTION (user_data); NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); NMSettingsConnectionSecretsFunc callback = other_data2; @@ -815,12 +845,16 @@ agent_secrets_done_cb (NMAgentManager *manager, gboolean agent_had_system = FALSE; ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE }; + if (!callback) + callback = _callback_nop; + + g_return_if_fail (g_slist_find (priv->reqs_int, call_id)); + + priv->reqs_int = g_slist_remove (priv->reqs_int, call_id); + if (error) { - _LOGD ("(%s:%u) secrets request error: (%d) %s", - setting_name, - call_id, - error->code, - error->message ? error->message : "(unknown)"); + _LOGD ("(%s:%p) secrets request error: %s", + setting_name, call_id, error->message); callback (self, call_id, NULL, setting_name, error, callback_data); return; @@ -837,7 +871,7 @@ agent_secrets_done_cb (NMAgentManager *manager, g_assert (secrets); if (agent_dbus_owner) { - _LOGD ("(%s:%u) secrets returned from agent %s", + _LOGD ("(%s:%p) secrets returned from agent %s", setting_name, call_id, agent_dbus_owner); @@ -854,7 +888,7 @@ agent_secrets_done_cb (NMAgentManager *manager, /* No user interaction was allowed when requesting secrets; the * agent is being bad. Remove system-owned secrets. */ - _LOGD ("(%s:%u) interaction forbidden but agent %s returned system secrets", + _LOGD ("(%s:%p) interaction forbidden but agent %s returned system secrets", setting_name, call_id, agent_dbus_owner); @@ -864,7 +898,7 @@ agent_secrets_done_cb (NMAgentManager *manager, /* Agent didn't successfully authenticate; clear system-owned secrets * from the secrets the agent returned. */ - _LOGD ("(%s:%u) agent failed to authenticate but provided system secrets", + _LOGD ("(%s:%p) agent failed to authenticate but provided system secrets", setting_name, call_id); @@ -872,12 +906,12 @@ agent_secrets_done_cb (NMAgentManager *manager, } } } else { - _LOGD ("(%s:%u) existing secrets returned", + _LOGD ("(%s:%p) existing secrets returned", setting_name, call_id); } - _LOGD ("(%s:%u) secrets request completed", + _LOGD ("(%s:%p) secrets request completed", setting_name, call_id); @@ -914,19 +948,19 @@ agent_secrets_done_cb (NMAgentManager *manager, * nothing has changed, since agent-owned secrets don't get saved here. */ if (agent_had_system) { - _LOGD ("(%s:%u) saving new secrets to backing storage", + _LOGD ("(%s:%p) saving new secrets to backing storage", setting_name, call_id); nm_settings_connection_commit_changes (self, NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, new_secrets_commit_cb, NULL); } else { - _LOGD ("(%s:%u) new agent secrets processed", + _LOGD ("(%s:%p) new agent secrets processed", setting_name, call_id); } } else { - _LOGD ("(%s:%u) failed to update with agent secrets: (%d) %s", + _LOGD ("(%s:%p) failed to update with agent secrets: (%d) %s", setting_name, call_id, local ? local->code : -1, @@ -934,7 +968,7 @@ agent_secrets_done_cb (NMAgentManager *manager, } g_variant_unref (filtered_secrets); } else { - _LOGD ("(%s:%u) failed to update with existing secrets: (%d) %s", + _LOGD ("(%s:%p) failed to update with existing secrets: (%d) %s", setting_name, call_id, local ? local->code : -1, @@ -961,9 +995,15 @@ agent_secrets_done_cb (NMAgentManager *manager, * Retrieves secrets from persistent storage and queries any secret agents for * additional secrets. * - * Returns: a call ID which may be used to cancel the ongoing secrets request + * With the returned call-id, the call can be cancelled. It is an error + * to cancel a call more then once or a call that already completed. + * The callback will always be invoked exactly once, also for cancellation + * and disposing of @self. In those latter cases, the callback will be invoked + * synchronously during cancellation/disposing. + * + * Returns: a call ID which may be used to cancel the ongoing secrets request. **/ -guint32 +NMSettingsConnectionCallId nm_settings_connection_get_secrets (NMSettingsConnection *self, NMAuthSubject *subject, const char *setting_name, @@ -975,7 +1015,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); GVariant *existing_secrets; - guint32 call_id = 0; + NMAgentManagerCallId call_id_a; gs_free char *joined_hints = NULL; /* Use priv->secrets to work around the fact that nm_connection_clear_secrets() @@ -985,7 +1025,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "%s.%d - Internal error; secrets cache invalid.", __FILE__, __LINE__); - return 0; + return NULL; } /* Make sure the request actually requests something we can return */ @@ -993,46 +1033,49 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND, "%s.%d - Connection didn't have requested setting '%s'.", __FILE__, __LINE__, setting_name); - return 0; + return NULL; } existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); if (existing_secrets) g_variant_ref_sink (existing_secrets); - call_id = nm_agent_manager_get_secrets (priv->agent_mgr, - NM_CONNECTION (self), - subject, - existing_secrets, - setting_name, - flags, - hints, - agent_secrets_done_cb, - self, - callback, - callback_data); + call_id_a = nm_agent_manager_get_secrets (priv->agent_mgr, + NM_CONNECTION (self), + subject, + existing_secrets, + setting_name, + flags, + hints, + agent_secrets_done_cb, + self, + callback, + callback_data); if (existing_secrets) g_variant_unref (existing_secrets); - _LOGD ("(%s:%u) secrets requested flags 0x%X hints '%s'", + _LOGD ("(%s:%p) secrets requested flags 0x%X hints '%s'", setting_name, - call_id, + call_id_a, flags, (hints && hints[0]) ? (joined_hints = g_strjoinv (",", (char **) hints)) : "(none)"); - return call_id; + priv->reqs_int = g_slist_append (priv->reqs_int, call_id_a); + + return NM_SETTINGS_CONNECTION_CALL_ID (call_id_a); } void nm_settings_connection_cancel_secrets (NMSettingsConnection *self, - guint32 call_id) + NMSettingsConnectionCallId call_id) { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - _LOGD ("(%u) secrets canceled", - call_id); + if (!g_slist_find (priv->reqs_int, call_id)) + g_return_if_reached (); + + _LOGD ("(%p) secrets canceled", call_id); - priv->reqs = g_slist_remove (priv->reqs, GUINT_TO_POINTER (call_id)); - nm_agent_manager_cancel_secrets (priv->agent_mgr, call_id); + nm_agent_manager_cancel_secrets (priv->agent_mgr, NM_AGENT_MANAGER_CALL_ID (call_id)); } /**** User authorization **************************************/ @@ -1606,7 +1649,7 @@ impl_settings_connection_delete (NMSettingsConnection *self, { NMAuthSubject *subject = NULL; GError *error = NULL; - + if (!check_writable (NM_CONNECTION (self), &error)) goto out_err; @@ -1627,7 +1670,7 @@ out_err: static void dbus_get_agent_secrets_cb (NMSettingsConnection *self, - guint32 call_id, + NMSettingsConnectionCallId call_id, const char *agent_username, const char *setting_name, GError *error, @@ -1637,7 +1680,9 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self, GDBusMethodInvocation *context = user_data; GVariant *dict; - priv->reqs = g_slist_remove (priv->reqs, GUINT_TO_POINTER (call_id)); + g_return_if_fail (g_slist_find (priv->reqs_ext, call_id)); + + priv->reqs_ext = g_slist_remove (priv->reqs_ext, call_id); if (error) g_dbus_method_invocation_return_gerror (context, error); @@ -1655,7 +1700,7 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self, } static void -dbus_get_secrets_auth_cb (NMSettingsConnection *self, +dbus_get_secrets_auth_cb (NMSettingsConnection *self, GDBusMethodInvocation *context, NMAuthSubject *subject, GError *error, @@ -1663,22 +1708,22 @@ dbus_get_secrets_auth_cb (NMSettingsConnection *self, { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); char *setting_name = user_data; - guint32 call_id = 0; + NMSettingsConnectionCallId call_id; GError *local = NULL; if (!error) { call_id = nm_settings_connection_get_secrets (self, - subject, - setting_name, - NM_SECRET_AGENT_GET_SECRETS_FLAG_USER_REQUESTED - | NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS, - NULL, - dbus_get_agent_secrets_cb, - context, - &local); - if (call_id > 0) { + subject, + setting_name, + NM_SECRET_AGENT_GET_SECRETS_FLAG_USER_REQUESTED + | NM_SECRET_AGENT_GET_SECRETS_FLAG_NO_ERRORS, + NULL, + dbus_get_agent_secrets_cb, + context, + &local); + if (call_id) { /* track the request and wait for the callback */ - priv->reqs = g_slist_append (priv->reqs, GUINT_TO_POINTER (call_id)); + priv->reqs_ext = g_slist_append (priv->reqs_ext, call_id); } } @@ -2335,14 +2380,36 @@ constructed (GObject *object) } static void +_cancel_all (NMAgentManager *agent_mgr, GSList **preqs) +{ + while (*preqs) { + NMAgentManagerCallId call_id_a = (*preqs)->data; + + /* Cancel will invoke the complete callback, which in + * turn deletes the current call-id from the list. */ + nm_agent_manager_cancel_secrets (agent_mgr, call_id_a); + + g_return_if_fail (!*preqs || (call_id_a != (*preqs)->data)); + } +} + +static void dispose (GObject *object) { NMSettingsConnection *self = NM_SETTINGS_CONNECTION (object); NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - GSList *iter; _LOGD ("disposing"); + /* Cancel in-progress secrets requests */ + if (priv->agent_mgr) { + /* although @reqs_ext theoretically contains NMSettingsConnectionCallId, + * we still cancel it with nm_agent_manager_cancel_secrets() -- it is + * actually correct. */ + _cancel_all (priv->agent_mgr, &priv->reqs_ext); + _cancel_all (priv->agent_mgr, &priv->reqs_int); + } + if (priv->updated_idle_id) { g_source_remove (priv->updated_idle_id); priv->updated_idle_id = 0; @@ -2363,12 +2430,6 @@ dispose (GObject *object) g_slist_free_full (priv->pending_auths, (GDestroyNotify) nm_auth_chain_unref); priv->pending_auths = NULL; - /* Cancel in-progress secrets requests */ - for (iter = priv->reqs; iter; iter = g_slist_next (iter)) - nm_agent_manager_cancel_secrets (priv->agent_mgr, GPOINTER_TO_UINT (iter->data)); - g_slist_free (priv->reqs); - priv->reqs = NULL; - g_clear_pointer (&priv->seen_bssids, (GDestroyNotify) g_hash_table_destroy); set_visible (self, FALSE); diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index b4b48271ba..588a99af53 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -85,6 +85,9 @@ typedef enum { /*< skip >*/ NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED = (1LL << 1), } NMSettingsConnectionCommitReason; +struct _NMSettingsConnectionCallId; +typedef struct _NMSettingsConnectionCallId *NMSettingsConnectionCallId; + typedef struct _NMSettingsConnectionClass NMSettingsConnectionClass; typedef void (*NMSettingsConnectionCommitFunc) (NMSettingsConnection *self, @@ -144,23 +147,23 @@ void nm_settings_connection_delete (NMSettingsConnection *self, gpointer user_data); typedef void (*NMSettingsConnectionSecretsFunc) (NMSettingsConnection *self, - guint32 call_id, + NMSettingsConnectionCallId call_id, const char *agent_username, const char *setting_name, GError *error, gpointer user_data); -guint32 nm_settings_connection_get_secrets (NMSettingsConnection *self, - NMAuthSubject *subject, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - const char **hints, - NMSettingsConnectionSecretsFunc callback, - gpointer callback_data, - GError **error); +NMSettingsConnectionCallId nm_settings_connection_get_secrets (NMSettingsConnection *self, + NMAuthSubject *subject, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char **hints, + NMSettingsConnectionSecretsFunc callback, + gpointer callback_data, + GError **error); void nm_settings_connection_cancel_secrets (NMSettingsConnection *self, - guint32 call_id); + NMSettingsConnectionCallId call_id); gboolean nm_settings_connection_is_visible (NMSettingsConnection *self); diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 4f84e18531..2bb4fa4678 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -80,7 +80,7 @@ typedef struct { gboolean service_can_persist; gboolean connection_can_persist; - guint32 secrets_id; + NMSettingsConnectionCallId secrets_id; SecretsReq secrets_idx; char *username; @@ -332,10 +332,8 @@ _set_vpn_state (NMVpnConnection *connection, _state_to_ac_state (vpn_state)); /* Clear any in-progress secrets request */ - if (priv->secrets_id) { + if (priv->secrets_id) nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id); - priv->secrets_id = 0; - } dispatcher_cleanup (connection); @@ -2009,7 +2007,7 @@ plugin_new_secrets_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_da static void get_secrets_cb (NMSettingsConnection *connection, - guint32 call_id, + NMSettingsConnectionCallId call_id, const char *agent_username, const char *setting_name, GError *error, @@ -2022,7 +2020,10 @@ get_secrets_cb (NMSettingsConnection *connection, g_return_if_fail (NM_CONNECTION (connection) == priv->connection); g_return_if_fail (call_id == priv->secrets_id); - priv->secrets_id = 0; + priv->secrets_id = NULL; + + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; if (error && priv->secrets_idx >= SECRETS_REQ_NEW) { nm_log_err (LOGD_VPN, "Failed to request VPN secrets #%d: (%d) %s", @@ -2227,7 +2228,6 @@ dispose (GObject *object) if (priv->secrets_id) { nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id); - priv->secrets_id = 0; } if (priv->cancellable) { |