wifi/iwd: merge branch 'balrog-kun/iwd-fixes' (pr#206)

https://github.com/NetworkManager/NetworkManager/pull/206

src/devices/wifi/nm-device-iwd.c

@@ -229,7 +229,7 @@ vardict_from_network_type (const char *type)
 }
 
 static void
-insert_ap_from_network (GHashTable *aps, GDBusProxy *proxy, const char *path, int16_t signal, uint32_t ap_id)
+insert_ap_from_network (GHashTable *aps, const char *path, int16_t signal, uint32_t ap_id)
 {
 	gs_unref_object GDBusProxy *network_proxy = NULL;
 	gs_unref_variant GVariant *name_value = NULL, *type_value = NULL;
@@ -239,19 +239,12 @@ insert_ap_from_network (GHashTable *aps, GDBusProxy *proxy, const char *path, in
 	GVariant *rsn;
 	uint8_t bssid[6];
 	NMWifiAP *ap;
-	GError *error;
 
-	network_proxy = g_dbus_proxy_new_sync (g_dbus_proxy_get_connection (proxy),
+	network_proxy = nm_iwd_manager_get_dbus_interface (nm_iwd_manager_get (),
-	                                       G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS,
+	                                                   path,
-	                                       NULL,
+	                                                   NM_IWD_NETWORK_INTERFACE);
-	                                       NM_IWD_SERVICE,
+	if (!network_proxy)
-	                                       path,
-	                                       NM_IWD_NETWORK_INTERFACE,
-	                                       NULL, &error);
-	if (!network_proxy) {
-		g_clear_error (&error);
 		return;
-	}
 
 	name_value = g_dbus_proxy_get_cached_property (network_proxy, "Name");
 	type_value = g_dbus_proxy_get_cached_property (network_proxy, "Type");
@@ -344,10 +337,10 @@ get_ordered_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data)
 
 	if (compat) {
 		while (g_variant_iter_next (networks, "(&o&sn&s)", &path, &name, &signal, &type))
-			insert_ap_from_network (new_aps, priv->dbus_station_proxy, path, signal, ap_id++);
+			insert_ap_from_network (new_aps, path, signal, ap_id++);
 	} else {
 		while (g_variant_iter_next (networks, "(&on)", &path, &signal))
-			insert_ap_from_network (new_aps, priv->dbus_station_proxy, path, signal, ap_id++);
+			insert_ap_from_network (new_aps, path, signal, ap_id++);
 	}
 
 	g_variant_iter_free (networks);
@@ -1255,12 +1248,7 @@ secrets_error:
 	g_dbus_method_invocation_return_error_literal (invocation, NM_DEVICE_ERROR,
 	                                               NM_DEVICE_ERROR_INVALID_CONNECTION,
 	                                               "NM secrets request failed");
-
+	/* Now wait for the Connect callback to update device state */
-	nm_device_state_changed (device,
-	                         NM_DEVICE_STATE_FAILED,
-	                         NM_DEVICE_STATE_REASON_NO_SECRETS);
-
-	cleanup_association_attempt (self, TRUE);
 }
 
 static void
@@ -1292,11 +1280,14 @@ network_connect_cb (GObject *source, GAsyncResult *res, gpointer user_data)
 {
 	NMDeviceIwd *self = user_data;
 	NMDevice *device = NM_DEVICE (self);
+	NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self);
 	gs_free_error GError *error = NULL;
 	NMConnection *connection;
 	NMSettingWireless *s_wifi;
 	GBytes *ssid;
 	gs_free char *ssid_utf8 = NULL;
+	NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED;
+	GVariant *value;
 
 	if (!_nm_dbus_proxy_call_finish (G_DBUS_PROXY (source), res,
 	                                 G_VARIANT_TYPE ("()"),
@@ -1311,6 +1302,12 @@ network_connect_cb (GObject *source, GAsyncResult *res, gpointer user_data)
 		       "Activation: (wifi) Network.Connect failed: %s",
 		       error->message);
 
+		if (nm_utils_error_is_cancelled (error, TRUE))
+			return;
+
+		if (!NM_IN_SET (nm_device_get_state (device), NM_DEVICE_STATE_CONFIG, NM_DEVICE_STATE_NEED_AUTH))
+			return;
+
 		connection = nm_device_get_applied_connection (device);
 		if (!connection)
 			goto failed;
@@ -1318,20 +1315,17 @@ network_connect_cb (GObject *source, GAsyncResult *res, gpointer user_data)
 		if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_DBUS_ERROR))
 			dbus_error = g_dbus_error_get_remote_error (error);
 
-		/* If secrets were wrong, we'd be getting a net.connman.iwd.Failed */
 		if (nm_streq0 (dbus_error, "net.connman.iwd.Failed")) {
 			nm_connection_clear_secrets (connection);
 
-			nm_device_state_changed (device, NM_DEVICE_STATE_FAILED,
+			/* If secrets were wrong, we'd be getting a net.connman.iwd.Failed */
-			                         NM_DEVICE_STATE_REASON_NO_SECRETS);
+			reason = NM_DEVICE_STATE_REASON_NO_SECRETS;
-		} else if (   !nm_utils_error_is_cancelled (error, TRUE)
+		} else if (nm_streq0 (dbus_error, "net.connman.iwd.Aborted")) {
-		           && nm_device_is_activating (device))
+			/* If agent call was cancelled we'd be getting a net.connman.iwd.Aborted */
-			goto failed;
+			reason = NM_DEVICE_STATE_REASON_NO_SECRETS;
+		}
 
-		/* Call Disconnect to make sure IWD's autoconnect is disabled */
+		goto failed;
-		cleanup_association_attempt (self, TRUE);
-
-		return;
 	}
 
 	nm_assert (nm_device_get_state (device) == NM_DEVICE_STATE_CONFIG);
@@ -1358,9 +1352,17 @@ network_connect_cb (GObject *source, GAsyncResult *res, gpointer user_data)
 	return;
 
 failed:
-	cleanup_association_attempt (self, FALSE);
+	/* Call Disconnect to make sure IWD's autoconnect is disabled */
-	nm_device_queue_state (device, NM_DEVICE_STATE_FAILED,
+	cleanup_association_attempt (self, TRUE);
-	                       NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED);
+
+	nm_device_queue_state (device, NM_DEVICE_STATE_FAILED, reason);
+
+	value = g_dbus_proxy_get_cached_property (priv->dbus_station_proxy, "State");
+	if (!priv->can_connect && nm_streq0 (get_variant_state (value), "disconnected")) {
+		priv->can_connect = true;
+		nm_device_emit_recheck_auto_activate (device);
+	}
+	g_variant_unref (value);
 }
 
 static void
@@ -1430,7 +1432,6 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason)
 	NMActRequest *req;
 	NMWifiAP *ap;
 	NMConnection *connection;
-	GError *error = NULL;
 	GDBusProxy *network_proxy;
 
 	req = nm_device_get_act_request (device);
@@ -1460,21 +1461,13 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason)
 		goto out;
 	}
 
-	/* Locate the IWD Network object */
+	network_proxy = nm_iwd_manager_get_dbus_interface (nm_iwd_manager_get (),
-	network_proxy = g_dbus_proxy_new_for_bus_sync (NM_IWD_BUS_TYPE,
+	                                                   nm_wifi_ap_get_supplicant_path (ap),
-	                                               G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES |
+	                                                   NM_IWD_NETWORK_INTERFACE);
-	                                               G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS,
-	                                               NULL,
-	                                               NM_IWD_SERVICE,
-	                                               nm_wifi_ap_get_supplicant_path (ap),
-	                                               NM_IWD_NETWORK_INTERFACE,
-	                                               NULL, &error);
 	if (!network_proxy) {
 		_LOGE (LOGD_DEVICE | LOGD_WIFI,
-		       "Activation: (wifi) could not get Network interface proxy for %s: %s",
+		       "Activation: (wifi) could not get Network interface proxy for %s",
-		       nm_wifi_ap_get_supplicant_path (ap),
+		       nm_wifi_ap_get_supplicant_path (ap));
-		       error->message);
-		g_clear_error (&error);
 		NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED);
 		goto out;
 	}
@@ -1753,13 +1746,6 @@ state_changed (NMDeviceIwd *self, const char *new_state)
 	/* Don't allow scanning while connecting, disconnecting or roaming */
 	priv->can_scan = NM_IN_STRSET (new_state, "connected", "disconnected");
 
-	/* Don't allow new connection until iwd exits disconnecting */
-	can_connect = NM_IN_STRSET (new_state, "disconnected");
-	if (can_connect != priv->can_connect) {
-		priv->can_connect = can_connect;
-		nm_device_emit_recheck_auto_activate (device);
-	}
-
 	if (NM_IN_STRSET (new_state, "connecting", "connected", "roaming")) {
 		/* If we were connecting, do nothing, the confirmation of
 		 * a connection success is handled in the Device.Connect
@@ -1773,40 +1759,39 @@ state_changed (NMDeviceIwd *self, const char *new_state)
 		_LOGW (LOGD_DEVICE | LOGD_WIFI,
 		       "Unsolicited connection success, asking IWD to disconnect");
 		send_disconnect (self);
-
-		return;
 	} else if (NM_IN_STRSET (new_state, "disconnecting", "disconnected")) {
-		if (!iwd_connection)
-			return;
-
 		/* Call Disconnect on the IWD device object to make sure it
 		 * disables its own autoconnect.
-		 *
-		 * Note we could instead call net.connman.iwd.KnownNetworks.ForgetNetwork
-		 * and leave the device in autoconnect.  This way if NetworkManager
-		 * changes any settings for this connection, they'd be taken into
-		 * account on the next connection attempt.  But both methods are
-		 * a hack, we'll perhaps need an IWD API to "connect once" without
-		 * storing anything.
 		 */
 		send_disconnect (self);
 
 		/*
-		 * If IWD is still handling the Connect call, let our callback
+		 * If IWD is still handling the Connect call, let our Connect
-		 * for the dbus method handle the failure.
+		 * callback for the dbus method handle the failure.  The main
+		 * reason we can't handle the failure here is because the method
+		 * callback will have more information on the specific failure
+		 * reason.
 		 */
 		if (dev_state == NM_DEVICE_STATE_CONFIG)
 			return;
 
-		nm_device_state_changed (device,
+		if (iwd_connection)
-		                         NM_DEVICE_STATE_FAILED,
+			nm_device_state_changed (device,
-	                                 NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT);
+			                         NM_DEVICE_STATE_FAILED,
-
+			                         NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT);
-		return;
+	} else if (!nm_streq (new_state, "unknown")) {
-	} else if (nm_streq (new_state, "unknown"))
+		_LOGE (LOGD_WIFI, "State %s unknown", new_state);
 		return;
+	}
 
-	_LOGE (LOGD_WIFI, "State %s unknown", new_state);
+	/* Don't allow new connection until iwd exits disconnecting and no
+	 * Connect callback is pending.
+	 */
+	can_connect = NM_IN_STRSET (new_state, "disconnected");
+	if (can_connect != priv->can_connect) {
+		priv->can_connect = can_connect;
+		nm_device_emit_recheck_auto_activate (device);
+	}
 }
 
 static void

src/devices/wifi/nm-iwd-manager.c

@@ -430,7 +430,16 @@ mirror_8021x_connection (NMIwdManager *self,
 	                                    NULL));
 	nm_connection_add_setting (connection, setting);
 
-	setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_802_1X, NULL));
+	/* "password" and "private-key-password" may be requested by the IWD agent
+	 * from NM and IWD will implement a specific secret cache policy so by
+	 * default respect that policy and don't save copies of those secrets in
+	 * NM settings.  The saved values can not be used anyway because of our
+	 * use of NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW.
+	 */
+	setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_802_1X,
+	                                    NM_SETTING_802_1X_PASSWORD_FLAGS, NM_SETTING_SECRET_FLAG_NOT_SAVED,
+	                                    NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD_FLAGS, NM_SETTING_SECRET_FLAG_NOT_SAVED,
+	                                    NULL));
 	nm_setting_802_1x_add_eap_method (NM_SETTING_802_1X (setting), "external");
 	nm_connection_add_setting (connection, setting);
 
@@ -786,6 +795,21 @@ nm_iwd_manager_is_known_network (NMIwdManager *self, const char *name,
 	return g_hash_table_contains (priv->known_networks, &kn_id);
 }
 
+GDBusProxy *
+nm_iwd_manager_get_dbus_interface (NMIwdManager *self, const char *path,
+                                   const char *name)
+{
+	NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self);
+	GDBusInterface *interface;
+
+	if (!priv->object_manager)
+		return NULL;
+
+	interface = g_dbus_object_manager_get_interface (priv->object_manager, path, name);
+
+	return interface ? G_DBUS_PROXY (interface) : NULL;
+}
+
 /*****************************************************************************/
 
 NM_DEFINE_SINGLETON_GETTER (NMIwdManager, nm_iwd_manager_get,

src/devices/wifi/nm-iwd-manager.h

@@ -57,4 +57,7 @@ NMIwdManager *nm_iwd_manager_get (void);
 gboolean nm_iwd_manager_is_known_network (NMIwdManager *self, const char *name,
                                           NMIwdNetworkSecurity security);
 
+GDBusProxy *nm_iwd_manager_get_dbus_interface (NMIwdManager *self, const char *path,
+                                               const char *name);
+
 #endif /* __NETWORKMANAGER_IWD_MANAGER_H__ */