From fb62f395eac076393c1ebe4764ddac197ac80907 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 15 Jun 2011 12:15:52 -0500 Subject: [PATCH] vpn: fix handling of connections with only system secrets The core problem was the nm_connection_need_secrets() call in nm-agent-manager.c's get_start() function; for VPN settings this always returns TRUE. Thus if a VPN connection had only system secrets, when the agent manager checked if additional secrets were required, they would be, and agents would be asked for secrets they didn't have and couldn't provide. Thus the connection would fail. nm_connection_need_secrets() simply can't know if VPN secrets are really required because it doesn't know anything about the internal VPN private data; only the plugin itself can tell us if secrets are required. If the system secrets are sufficient we shouldn't be asking any agents for secrets at all. So implement a three-step secrets path for VPN connections. First we retrieve existing system secrets, and ask the plugin if these are sufficient. Second we request both existing system secrets and existing agent secrets and again ask the plugin if these are sufficient. If both those fail, we ask agents for new secrets. --- include/nm-settings-flags.h | 8 +- src/settings/nm-agent-manager.c | 7 +- src/settings/nm-secret-agent.c | 3 + src/vpn-manager/nm-vpn-connection.c | 170 ++++++++++++++-------------- 4 files changed, 101 insertions(+), 87 deletions(-) diff --git a/include/nm-settings-flags.h b/include/nm-settings-flags.h index 5f008f5c87..4bf59f2f1b 100644 --- a/include/nm-settings-flags.h +++ b/include/nm-settings-flags.h @@ -22,12 +22,16 @@ #define NM_SETTINGS_FLAGS_H /* NOTE: these values should match the NM_SECRET_AGENT_GET_SECRETS_FLAGS in - * the nm-secret-agent.xml introspection file. + * the nm-secret-agent.xml introspection file; except ONLY_SYSTEM which is + * internal to NM. */ typedef enum { NM_SETTINGS_GET_SECRETS_FLAG_NONE = 0x0, NM_SETTINGS_GET_SECRETS_FLAG_ALLOW_INTERACTION = 0x1, - NM_SETTINGS_GET_SECRETS_FLAG_REQUEST_NEW = 0x2 + NM_SETTINGS_GET_SECRETS_FLAG_REQUEST_NEW = 0x2, + + /* Internal only to NM */ + NM_SETTINGS_GET_SECRETS_FLAG_ONLY_SYSTEM = 0x80000000 } NMSettingsGetSecretsFlags; #endif /* NM_SETTINGS_FLAGS_H */ diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 8a5ea1068a..9b9e1897b5 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -945,8 +945,8 @@ get_start (gpointer user_data) g_clear_error (&error); } else { /* Do we have everything we need? */ - /* FIXME: handle second check for VPN connections */ - if ((nm_connection_need_secrets (tmp, NULL) == NULL) && (request_new == FALSE)) { + if ( (req->flags & NM_SETTINGS_GET_SECRETS_FLAG_ONLY_SYSTEM) + || ((nm_connection_need_secrets (tmp, NULL) == NULL) && (request_new == FALSE))) { nm_log_dbg (LOGD_AGENTS, "(%p/%s) system settings secrets sufficient", req, req->setting_name); @@ -1059,7 +1059,8 @@ nm_agent_manager_get_secrets (NMAgentManager *self, g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req); /* Kick off the request */ - request_add_agents (self, req); + if (!(req->flags & NM_SETTINGS_GET_SECRETS_FLAG_ONLY_SYSTEM)) + request_add_agents (self, req); req->idle_id = g_idle_add (get_start, req); return req->reqid; diff --git a/src/settings/nm-secret-agent.c b/src/settings/nm-secret-agent.c index 2b1156cbc8..346c7310a0 100644 --- a/src/settings/nm-secret-agent.c +++ b/src/settings/nm-secret-agent.c @@ -189,6 +189,9 @@ nm_secret_agent_get_secrets (NMSecretAgent *self, hash = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL); + /* Mask off the private ONLY_SYSTEM flag if present */ + flags &= ~NM_SETTINGS_GET_SECRETS_FLAG_ONLY_SYSTEM; + r = request_new (self, nm_connection_get_path (connection), setting_name, callback, callback_data); r->call = dbus_g_proxy_begin_call_with_timeout (priv->proxy, "GetSecrets", diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index cf6135a684..067b2dce3e 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -53,6 +53,17 @@ G_DEFINE_TYPE (NMVPNConnection, nm_vpn_connection, NM_TYPE_VPN_CONNECTION_BASE) +typedef enum { + /* Only system secrets */ + SECRETS_REQ_SYSTEM = 0, + /* All existing secrets including agent secrets */ + SECRETS_REQ_EXISTING = 1, + /* New secrets required; ask an agent */ + SECRETS_REQ_NEW = 2, + /* Placeholder for bounds checking */ + SECRETS_REQ_LAST +} SecretsReq; + typedef struct { gboolean disposed; @@ -61,6 +72,7 @@ typedef struct { gboolean user_requested; gulong user_uid; guint32 secrets_id; + SecretsReq secrets_idx; char *username; NMDevice *parent_dev; @@ -98,6 +110,8 @@ enum { LAST_PROP }; +static void get_secrets (NMVPNConnection *self, SecretsReq secrets_idx); + static void nm_vpn_connection_set_vpn_state (NMVPNConnection *connection, NMVPNConnectionState vpn_state, @@ -792,84 +806,50 @@ nm_vpn_connection_disconnect (NMVPNConnection *connection, /******************************************************************************/ static void -vpn_secrets_cb (NMSettingsConnection *connection, - guint32 call_id, - const char *agent_username, - const char *setting_name, - GError *error, - gpointer user_data) +plugin_need_secrets_cb (DBusGProxy *proxy, + char *setting_name, + GError *error, + gpointer user_data) { NMVPNConnection *self = NM_VPN_CONNECTION (user_data); NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); - g_return_if_fail (NM_CONNECTION (connection) == priv->connection); - g_return_if_fail (call_id == priv->secrets_id); - - priv->secrets_id = 0; - - if (error) - nm_vpn_connection_fail (self, NM_VPN_CONNECTION_STATE_REASON_NO_SECRETS); - else - really_activate (self, agent_username); -} - -static void -connection_need_secrets_cb (DBusGProxy *proxy, - char *setting_name, - GError *error, - gpointer user_data) -{ - NMVPNConnection *self = NM_VPN_CONNECTION (user_data); - NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); - GError *local = NULL; - if (error) { - nm_log_err (LOGD_VPN, "NeedSecrets failed: %s %s", + nm_log_err (LOGD_VPN, "(%s/%s) plugin NeedSecrets request #%d failed: %s %s", + nm_connection_get_uuid (priv->connection), + nm_connection_get_id (priv->connection), + priv->secrets_idx + 1, g_quark_to_string (error->domain), error->message); nm_vpn_connection_fail (self, NM_VPN_CONNECTION_STATE_REASON_NO_SECRETS); return; } - if (!setting_name || !strlen (setting_name)) { - nm_log_dbg (LOGD_VPN, "(%s/%s) service indicated no additional secrets required", - nm_connection_get_uuid (priv->connection), - nm_connection_get_id (priv->connection)); + if (setting_name && strlen (setting_name)) { + /* More secrets required */ + nm_log_dbg (LOGD_VPN, "(%s/%s) service indicated additional secrets required", + nm_connection_get_uuid (priv->connection), + nm_connection_get_id (priv->connection)); - /* No secrets required */ - really_activate (self, priv->username); + get_secrets (self, priv->secrets_idx + 1); return; } - nm_log_dbg (LOGD_VPN, "(%s/%s) service indicated additional '%s' secrets required", - nm_connection_get_uuid (priv->connection), - nm_connection_get_id (priv->connection), - setting_name); + nm_log_dbg (LOGD_VPN, "(%s/%s) service indicated no additional secrets required", + nm_connection_get_uuid (priv->connection), + nm_connection_get_id (priv->connection)); - priv->secrets_id = nm_settings_connection_get_secrets (NM_SETTINGS_CONNECTION (priv->connection), - priv->user_requested, - priv->user_uid, - setting_name, - NM_SETTINGS_GET_SECRETS_FLAG_ALLOW_INTERACTION, - NULL, - vpn_secrets_cb, - self, - &local); - if (!priv->secrets_id) { - if (local) - nm_log_err (LOGD_VPN, "failed to get secrets: (%d) %s", local->code, local->message); - nm_vpn_connection_fail (self, NM_VPN_CONNECTION_STATE_REASON_NO_SECRETS); - g_clear_error (&local); - } + /* No secrets required; we can start the VPN */ + really_activate (self, priv->username); } static void -existing_secrets_cb (NMSettingsConnection *connection, - guint32 call_id, - const char *agent_username, - const char *setting_name, - GError *error, - gpointer user_data) +get_secrets_cb (NMSettingsConnection *connection, + guint32 call_id, + const char *agent_username, + const char *setting_name, + GError *error, + gpointer user_data) { NMVPNConnection *self = NM_VPN_CONNECTION (user_data); NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); @@ -881,57 +861,77 @@ existing_secrets_cb (NMSettingsConnection *connection, priv->secrets_id = 0; if (error) { - nm_log_err (LOGD_VPN, "Failed to request existing VPN secrets #2: (%s) %s", - g_quark_to_string (error->domain), - error->message); + nm_log_err (LOGD_VPN, "Failed to request VPN secrets #%d: (%d) %s", + priv->secrets_idx + 1, error->code, error->message); nm_vpn_connection_fail (self, NM_VPN_CONNECTION_STATE_REASON_NO_SECRETS); } else { nm_log_dbg (LOGD_VPN, "(%s/%s) asking service if additional secrets are required", - nm_connection_get_uuid (priv->connection), - nm_connection_get_id (priv->connection)); + nm_connection_get_uuid (priv->connection), + nm_connection_get_id (priv->connection)); /* Cache the username for later */ - g_free (priv->username); - priv->username = g_strdup (agent_username); + if (agent_username) { + g_free (priv->username); + priv->username = g_strdup (agent_username); + } /* Ask the VPN service if more secrets are required */ hash = _hash_with_username (priv->connection, priv->username); org_freedesktop_NetworkManager_VPN_Plugin_need_secrets_async (priv->proxy, hash, - connection_need_secrets_cb, + plugin_need_secrets_cb, self); g_hash_table_destroy (hash); } } static void -get_existing_secrets (NMVPNConnection *self) +get_secrets (NMVPNConnection *self, SecretsReq secrets_idx) { NMVPNConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); + NMSettingsGetSecretsFlags flags = NM_SETTINGS_GET_SECRETS_FLAG_NONE; GError *error = NULL; + gboolean filter_by_uid = priv->user_requested; - nm_log_dbg (LOGD_VPN, "(%s/%s) requesting existing VPN secrets", + g_return_if_fail (secrets_idx < SECRETS_REQ_LAST); + priv->secrets_idx = secrets_idx; + + nm_log_dbg (LOGD_VPN, "(%s/%s) requesting VPN secrets pass #%d", nm_connection_get_uuid (priv->connection), - nm_connection_get_id (priv->connection)); + nm_connection_get_id (priv->connection), + priv->secrets_idx + 1); + + switch (priv->secrets_idx) { + case SECRETS_REQ_SYSTEM: + flags = NM_SETTINGS_GET_SECRETS_FLAG_ONLY_SYSTEM; + filter_by_uid = FALSE; + break; + case SECRETS_REQ_EXISTING: + flags = NM_SETTINGS_GET_SECRETS_FLAG_NONE; + break; + case SECRETS_REQ_NEW: + flags = NM_SETTINGS_GET_SECRETS_FLAG_ALLOW_INTERACTION; + break; + default: + g_assert_not_reached (); + } - /* Just get existing secrets if any so we can ask the VPN service if - * any more are required. - */ priv->secrets_id = nm_settings_connection_get_secrets (NM_SETTINGS_CONNECTION (priv->connection), - priv->user_requested, + filter_by_uid, priv->user_uid, NM_SETTING_VPN_SETTING_NAME, - NM_SETTINGS_GET_SECRETS_FLAG_NONE, + flags, NULL, - existing_secrets_cb, + get_secrets_cb, self, &error); - if (priv->secrets_id == 0) { - nm_log_err (LOGD_VPN, "Failed to request existing VPN secrets #1: (%s) %s", - g_quark_to_string (error->domain), - error->message); - g_error_free (error); + if (!priv->secrets_id) { + if (error) { + nm_log_err (LOGD_VPN, "failed to request VPN secrets #%d: (%d) %s", + priv->secrets_idx + 1, error->code, error->message); + } nm_vpn_connection_fail (self, NM_VPN_CONNECTION_STATE_REASON_NO_SECRETS); + g_clear_error (&error); } } @@ -1002,10 +1002,16 @@ connection_state_changed (NMVPNConnection *self, nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id); priv->secrets_id = 0; } + priv->secrets_idx = SECRETS_REQ_SYSTEM; switch (state) { case NM_VPN_CONNECTION_STATE_NEED_AUTH: - get_existing_secrets (self); + /* Kick off the secrets requests; first we get existing system secrets + * and ask the plugin if these are sufficient, next we get all existing + * secrets from system and from user agents and ask the plugin again, + * and last we ask the user for new secrets if required. + */ + get_secrets (self, SECRETS_REQ_SYSTEM); break; case NM_VPN_CONNECTION_STATE_ACTIVATED: /* Secrets no longer needed now that we're connected */