core: fix race of blocking autoconnect for no-secrets when a new secret-agent registers

When activation of the connection fails with no-secrets, we block
autoconnect due to that. However, NMPolicy also unblocks such
autoconnect, whenever a new secret-agent registers. The reason
is obviously, that the new secret-agent might be able to provide
the previously missing secrets.

However, there is a race between
  - making the secret request, failing activation and blocking autoconnect
  - new secret-agent registers

If the secret-agent registers after making the request, but before we
block autoconnect, then autoconnect stays blocked.

  [1511468634.5759] device (wlp4s0): state change: config -> need-auth (reason 'none', sys-iface-state: 'managed')
  [1511468634.5772] device (wlp4s0): No agents were available for this request.
  [1511468638.4082] agent-manager: req[0x55ea7e58a5d0, :1.32/org.kde.plasma.networkmanagement/1000]: agent registered
  [1511468638.4082] policy: re-enabling autoconnect for all connections with failed secrets
  [1511468664.6280] device (wlp4s0): state change: need-auth -> failed (reason 'no-secrets', sys-iface-state: 'managed')
  [1511468664.6287] policy: connection 'tuxmobil' now blocked from autoconnect due to no secrets

Note the long timing between making the secret request and the
activation failure. This race already existed before, but now with
WPS push-button method enabled by default, the duraction of the
activation is much longer and the race is easy to hit.

https://bugzilla.gnome.org/show_bug.cgi?id=790571
This commit is contained in:
Thomas Haller 2017-11-27 09:33:32 +01:00
parent 46af70b508
commit e2c8ef45ac
6 changed files with 63 additions and 11 deletions

View file

@ -1750,20 +1750,35 @@ device_state_changed (NMDevice *device,
if ( connection
&& old_state >= NM_DEVICE_STATE_PREPARE
&& old_state <= NM_DEVICE_STATE_ACTIVATED) {
gboolean block_no_secrets = FALSE;
int tries;
guint64 con_v;
tries = nm_settings_connection_autoconnect_retries_get (connection);
if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_NO_SECRETS) {
/* we want to block the connection from auto-connect if it failed due to no-secrets.
* However, if a secret-agent registered, since the connection made the last
* secret-request, we do not block it. The new secret-agent might not yet
* been consulted, and it may be able to provide the secrets.
*
* We detect this by using a version-id of the agent-manager, which increments
* whenever new agents register. */
con_v = nm_settings_connection_get_last_secret_agent_version_id (connection);
if ( con_v == 0
|| con_v != nm_agent_manager_get_agent_version_id (priv->agent_mgr))
block_no_secrets = TRUE;
}
if (block_no_secrets) {
_LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to no secrets",
nm_settings_connection_get_id (connection));
nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS, TRUE);
} else if (tries != 0) {
} else {
tries = nm_settings_connection_autoconnect_retries_get (connection);
if (tries > 0) {
_LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; %d tries left",
nm_settings_connection_get_id (connection), tries - 1);
_connection_autoconnect_retries_set (self, connection, tries - 1);
} else {
} else if (tries != 0) {
_LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; infinite tries left",
nm_settings_connection_get_id (connection));
}

View file

@ -62,6 +62,8 @@ typedef struct {
GHashTable *agents;
CList requests;
guint64 agent_version_id;
} NMAgentManagerPrivate;
struct _NMAgentManager {
@ -137,6 +139,16 @@ static gboolean _con_get_try_complete_early (Request *req);
/*****************************************************************************/
guint64
nm_agent_manager_get_agent_version_id (NMAgentManager *self)
{
g_return_val_if_fail (NM_IS_AGENT_MANAGER (self), 0);
return NM_AGENT_MANAGER_GET_PRIVATE (self)->agent_version_id;
}
/*****************************************************************************/
typedef enum {
REQUEST_TYPE_INVALID,
REQUEST_TYPE_CON_GET,
@ -336,6 +348,7 @@ agent_register_permissions_done (NMAuthChain *chain,
if (result == NM_AUTH_CALL_RESULT_YES)
nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, TRUE);
priv->agent_version_id += 1;
sender = nm_secret_agent_get_dbus_owner (agent);
g_hash_table_insert (priv->agents, g_strdup (sender), agent);
_LOGD (agent, "agent registered");
@ -1558,6 +1571,7 @@ nm_agent_manager_init (NMAgentManager *self)
{
NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
priv->agent_version_id = 1;
c_list_init (&priv->requests);
priv->agents = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref);
}

View file

@ -43,6 +43,8 @@ GType nm_agent_manager_get_type (void);
NMAgentManager *nm_agent_manager_get (void);
guint64 nm_agent_manager_get_agent_version_id (NMAgentManager *self);
/* If no agent fulfilled the secrets request, agent_dbus_owner will be NULL */
typedef void (*NMAgentSecretsResultFunc) (NMAgentManager *manager,
NMAgentManagerCallId call_id,

View file

@ -50,16 +50,13 @@ typedef struct {
char *owner_username;
char *dbus_owner;
NMSecretAgentCapabilities capabilities;
GSList *permissions;
NMDBusSecretAgent *proxy;
NMBusManager *bus_mgr;
GDBusConnection *connection;
gboolean connection_is_private;
gulong on_disconnected_id;
CList requests;
gulong on_disconnected_id;
bool connection_is_private:1;
} NMSecretAgentPrivate;
struct _NMSecretAgent {

View file

@ -106,13 +106,17 @@ typedef struct _NMSettingsConnectionPrivate {
*/
NMConnection *agent_secrets;
guint64 timestamp; /* Up-to-date timestamp of connection use */
char *filename;
GHashTable *seen_bssids; /* Up-to-date BSSIDs that's been seen for the connection */
guint64 timestamp; /* Up-to-date timestamp of connection use */
guint64 last_secret_agent_version_id;
int autoconnect_retries;
gint32 autoconnect_retries_blocked_until;
char *filename;
} NMSettingsConnectionPrivate;
G_DEFINE_TYPE_WITH_CODE (NMSettingsConnection, nm_settings_connection, NM_TYPE_EXPORTED_OBJECT,
@ -171,6 +175,16 @@ nm_settings_connection_has_unmodified_applied_connection (NMSettingsConnection *
/*****************************************************************************/
guint64
nm_settings_connection_get_last_secret_agent_version_id (NMSettingsConnection *self)
{
g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), 0);
return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->last_secret_agent_version_id;
}
/*****************************************************************************/
/* Return TRUE to keep, FALSE to drop */
typedef gboolean (*ForEachSecretFunc) (NMSettingSecretFlags flags,
gpointer user_data);
@ -1261,6 +1275,14 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
if (existing_secrets)
g_variant_ref_sink (existing_secrets);
/* we remember the current version-id of the secret-agents. The version-id is strictly increasing,
* as new agents register the number. We know hence, that this request was made against a certain
* set of secret-agents.
* If after making this request a new secret-agent registeres, the version-id increases.
* Then we know that the this request probably did not yet include the latest secret-agent. */
priv->last_secret_agent_version_id = nm_agent_manager_get_agent_version_id (priv->agent_mgr);
call_id_a = nm_agent_manager_get_secrets (priv->agent_mgr,
nm_connection_get_path (NM_CONNECTION (self)),
NM_CONNECTION (self),

View file

@ -125,6 +125,8 @@ struct _NMSettingsConnectionClass {
GType nm_settings_connection_get_type (void);
guint64 nm_settings_connection_get_last_secret_agent_version_id (NMSettingsConnection *self);
gboolean nm_settings_connection_has_unmodified_applied_connection (NMSettingsConnection *self,
NMConnection *applied_connection,
NMSettingCompareFlags compare_flage);