Commit fb62f395 authored by Dan Williams's avatar Dan Williams

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.
parent 2d461942
......@@ -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 */
......
......@@ -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,6 +1059,7 @@ nm_agent_manager_get_secrets (NMAgentManager *self,
g_hash_table_insert (priv->requests, GUINT_TO_POINTER (req->reqid), req);
/* Kick off the request */
if (!(req->flags & NM_SETTINGS_GET_SECRETS_FLAG_ONLY_SYSTEM))
request_add_agents (self, req);
req->idle_id = g_idle_add (get_start, req);
......
......@@ -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",
......
......@@ -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,79 +806,45 @@ 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)
{
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,
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);
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",
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_log_dbg (LOGD_VPN, "(%s/%s) service indicated no additional secrets required",
nm_connection_get_uuid (priv->connection),
nm_connection_get_id (priv->connection),
setting_name);
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,
get_secrets_cb (NMSettingsConnection *connection,
guint32 call_id,
const char *agent_username,
const char *setting_name,
......@@ -881,9 +861,8 @@ 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",
......@@ -891,47 +870,68 @@ existing_secrets_cb (NMSettingsConnection *connection,
nm_connection_get_id (priv->connection));
/* Cache the username for later */
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 */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment