[PATCH 1/3] iwd: Scan hidden networks

[PATCH 1/3] iwd: Scan hidden networks

[VAUTRIN Emmanuel (Canal Plus Prestataire)]

Only visible networks were retrieved by a scan, via GetOrderedNetworks()
iwd function. A call to GetHiddenAccessPoints() is necessary to
retrieve the hidden ones.
---
 plugins/iwd.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 1 deletion(-)

diff --git a/plugins/iwd.c b/plugins/iwd.c
index ec9a4574cf2d..00578f4d4ab3 100644
--- a/plugins/iwd.c
+++ b/plugins/iwd.c
@@ -113,12 +113,22 @@ struct iwd_known_network {
 	bool autoconnect;
 };
 
+struct iwd_hidden_ap {
+	char *path;
+	char *device;
+	char *address;
+	char *type;
+	int16_t signal_strength;
+};
+
 struct iwd_station {
 	GDBusProxy *proxy;
 	char *path;
 	char *state;
 	char *connected_network;
 	bool scanning;
+
+	GHashTable *hidden_aps;
 };
 
 struct iwd_ap {
@@ -926,7 +936,8 @@ static void add_network(const char *path, struct iwd_network *iwdn)
 	connman_network_set_data(iwdn->network, iwdn);
 
 	connman_network_set_name(iwdn->network, iwdn->name);
-	connman_network_set_blob(iwdn->network, "WiFi.SSID", iwdn->name,
+	if (iwdn->name)
+		connman_network_set_blob(iwdn->network, "WiFi.SSID", iwdn->name,
 					strlen(iwdn->name));
 	connman_network_set_string(iwdn->network, "WiFi.Security",
 					security_remap(iwdn->type));
@@ -1189,6 +1200,113 @@ static void update_signal_strength(struct iwd_station *iwds)
 		DBG("GetOrderedNetworks() failed");
 }
 
+static void create_hidden_network(struct iwd_hidden_ap *iwdh)
+{
+	struct iwd_network *iwdn;
+
+	iwdn = g_try_new0(struct iwd_network, 1);
+	if (!iwdn) {
+		connman_error("Out of memory creating IWD network");
+		return;
+	}
+
+	iwdn->proxy = NULL;
+	iwdn->path = g_strdup(iwdh->path);
+	iwdn->device = g_strdup(iwdh->device);
+	iwdn->name = NULL;
+	iwdn->type = g_strdup(iwdh->type);
+	iwdn->connected = false;
+	iwdn->known_network = NULL;
+
+	DBG("device %s type %s",
+		iwdn->device, iwdn->type);
+
+	g_hash_table_replace(networks, iwdn->path, iwdn);
+
+	add_network(iwdn->path, iwdn);
+
+	_update_signal_strength(iwdn->path, iwdh->signal_strength);
+}
+
+static void create_hidden_ap(char *path, DBusMessageIter *array)
+{
+	struct iwd_hidden_ap *iwdh;
+	const char *address, *type;
+	struct iwd_station *iwds;
+	int16_t signal_strength;
+	DBusMessageIter value;
+
+	dbus_message_iter_recurse(array, &value);
+
+	dbus_message_iter_get_basic(&value, &address);
+
+	dbus_message_iter_next(&value);
+	dbus_message_iter_get_basic(&value, &signal_strength);
+
+	dbus_message_iter_next(&value);
+	dbus_message_iter_get_basic(&value, &type);
+
+	dbus_message_iter_next(array);
+
+	DBG("device '%s' address '%s' type %s signal_strength %d",
+		path, address, type, signal_strength);
+
+	iwds = g_hash_table_lookup(stations, path);
+	if (!iwds) {
+		connman_error("Related IWD station not found %s", path);
+		return;
+	}
+
+	iwdh = g_try_new0(struct iwd_hidden_ap, 1);
+	if (!iwdh) {
+		connman_error("Out of memory creating IWD hidden ap");
+		return;
+	}
+
+	iwdh->device = g_strdup(path);
+	iwdh->type = g_strdup(type);
+	iwdh->path = g_strdup_printf("%s/hidden_%s", iwdh->device, iwdh->type);
+	iwdh->address = g_strdup(address);
+	iwdh->signal_strength = signal_strength;
+
+	g_hash_table_replace(iwds->hidden_aps, iwdh->path, iwdh);
+
+	create_hidden_network(iwdh);
+}
+
+static void hidden_access_points_cb(DBusMessage *message, void *user_data)
+{
+	DBusMessageIter array, entry;
+	struct iwd_station *iwds;
+	char *path = user_data;
+
+	DBG("");
+
+	if (!dbus_message_iter_init(message, &array))
+		return;
+
+	if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_ARRAY)
+		return;
+
+	iwds = g_hash_table_lookup(stations, path);
+	if (iwds)
+		g_hash_table_remove_all(iwds->hidden_aps);
+
+	dbus_message_iter_recurse(&array, &entry);
+	while (dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRUCT) {
+		create_hidden_ap(path, &entry);
+	}
+}
+
+static void update_hidden_networks(struct iwd_station *iwds)
+{
+	if (!g_dbus_proxy_method_call(iwds->proxy,
+					"GetHiddenAccessPoints",
+					NULL, hidden_access_points_cb,
+					g_strdup(iwds->path), g_free))
+		DBG("GetHiddenAccessPoints() failed");
+}
+
 static void station_property_change(GDBusProxy *proxy, const char *name,
 		DBusMessageIter *iter, void *user_data)
 {
@@ -1234,6 +1352,7 @@ static void station_property_change(GDBusProxy *proxy, const char *name,
 					CONNMAN_SERVICE_TYPE_WIFI, true);
 		} else {
 			update_signal_strength(iwds);
+			update_hidden_networks(iwds);
 		}
 
 
@@ -1354,9 +1473,26 @@ static void station_free(gpointer data)
 	}
 	g_free(iwds->path);
 	g_free(iwds->connected_network);
+	g_hash_table_destroy(iwds->hidden_aps);
 	g_free(iwds);
 }
 
+static void hidden_ap_free(gpointer data)
+{
+	struct iwd_hidden_ap *iwdh = data;
+	struct iwd_network *iwdn;
+
+	iwdn = g_hash_table_lookup(networks, iwdh->path);
+	if (iwdn)
+		remove_network(iwdn);
+
+	g_free(iwdh->path);
+	g_free(iwdh->device);
+	g_free(iwdh->address);
+	g_free(iwdh->type);
+	g_free(iwdh);
+}
+
 static void ap_free(gpointer data)
 {
 	struct iwd_ap *iwdap = data;
@@ -1717,6 +1853,9 @@ static void create_station(GDBusProxy *proxy)
 	iwds->connected_network = g_strdup(proxy_get_string(proxy, "ConnectedNetwork"));
 	iwds->scanning = proxy_get_bool(proxy, "Scanning");
 
+	iwds->hidden_aps = g_hash_table_new_full(g_str_hash, g_str_equal, NULL,
+			hidden_ap_free);
+
 	DBG("state '%s' connected_network %s scanning %d",
 		iwds->state, iwds->connected_network, iwds->scanning);
 
-- 
2.25.1

Re: [PATCH 1/3] iwd: Scan hidden networks

[Daniel Wagner]

Hi Emmanuel,

Thanks for the feature. The patch looks pretty good. Just a minor nitpick for
this and the other 2 patches.

On Fri, Oct 01, 2021 at 03:26:03PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> Only visible networks were retrieved by a scan, via GetOrderedNetworks()
> iwd function. A call to GetHiddenAccessPoints() is necessary to
> retrieve the hidden ones.
> +static void create_hidden_network(struct iwd_hidden_ap *iwdh)
> +{
> +	struct iwd_network *iwdn;
> +
> +	iwdn = g_try_new0(struct iwd_network, 1);
> +	if (!iwdn) {
> +		connman_error("Out of memory creating IWD network");
> +		return;
> +	}

Just use g_new0(). There is no use to try to work around small memory
allocation failures. First they are very likely never to be observed and
if they do our failure path is not tested. Even more...


> +
> +	iwdn->proxy = NULL;
> +	iwdn->path = g_strdup(iwdh->path);

If g_strdup() can't allocate memory it will call abort() anyway.

Thanks,
Daniel

RE: [PATCH 1/3] iwd: Scan hidden networks

[VAUTRIN Emmanuel (Canal Plus Prestataire)]

Only visible networks were retrieved by a scan, via GetOrderedNetworks()
iwd function. A call to GetHiddenAccessPoints() is necessary to
retrieve the hidden ones.
---
 plugins/iwd.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 1 deletion(-)

diff --git a/plugins/iwd.c b/plugins/iwd.c
index ec9a4574cf2d..0f2b638caceb 100644
--- a/plugins/iwd.c
+++ b/plugins/iwd.c
@@ -113,12 +113,22 @@ struct iwd_known_network {
 	bool autoconnect;
 };
 
+struct iwd_hidden_ap {
+	char *path;
+	char *device;
+	char *address;
+	char *type;
+	int16_t signal_strength;
+};
+
 struct iwd_station {
 	GDBusProxy *proxy;
 	char *path;
 	char *state;
 	char *connected_network;
 	bool scanning;
+
+	GHashTable *hidden_aps;
 };
 
 struct iwd_ap {
@@ -926,7 +936,8 @@ static void add_network(const char *path, struct iwd_network *iwdn)
 	connman_network_set_data(iwdn->network, iwdn);
 
 	connman_network_set_name(iwdn->network, iwdn->name);
-	connman_network_set_blob(iwdn->network, "WiFi.SSID", iwdn->name,
+	if (iwdn->name)
+		connman_network_set_blob(iwdn->network, "WiFi.SSID", iwdn->name,
 					strlen(iwdn->name));
 	connman_network_set_string(iwdn->network, "WiFi.Security",
 					security_remap(iwdn->type));
@@ -1189,6 +1200,113 @@ static void update_signal_strength(struct iwd_station *iwds)
 		DBG("GetOrderedNetworks() failed");
 }
 
+static void create_hidden_network(struct iwd_hidden_ap *iwdh)
+{
+	struct iwd_network *iwdn;
+
+	iwdn = g_new0(struct iwd_network, 1);
+	if (!iwdn) {
+		connman_error("Out of memory creating IWD network");
+		return;
+	}
+
+	iwdn->proxy = NULL;
+	iwdn->path = g_strdup(iwdh->path);
+	iwdn->device = g_strdup(iwdh->device);
+	iwdn->name = NULL;
+	iwdn->type = g_strdup(iwdh->type);
+	iwdn->connected = false;
+	iwdn->known_network = NULL;
+
+	DBG("device %s type %s",
+		iwdn->device, iwdn->type);
+
+	g_hash_table_replace(networks, iwdn->path, iwdn);
+
+	add_network(iwdn->path, iwdn);
+
+	_update_signal_strength(iwdn->path, iwdh->signal_strength);
+}
+
+static void create_hidden_ap(char *path, DBusMessageIter *array)
+{
+	struct iwd_hidden_ap *iwdh;
+	const char *address, *type;
+	struct iwd_station *iwds;
+	int16_t signal_strength;
+	DBusMessageIter value;
+
+	dbus_message_iter_recurse(array, &value);
+
+	dbus_message_iter_get_basic(&value, &address);
+
+	dbus_message_iter_next(&value);
+	dbus_message_iter_get_basic(&value, &signal_strength);
+
+	dbus_message_iter_next(&value);
+	dbus_message_iter_get_basic(&value, &type);
+
+	dbus_message_iter_next(array);
+
+	DBG("device '%s' address '%s' type %s signal_strength %d",
+		path, address, type, signal_strength);
+
+	iwds = g_hash_table_lookup(stations, path);
+	if (!iwds) {
+		connman_error("Related IWD station not found %s", path);
+		return;
+	}
+
+	iwdh = g_new0(struct iwd_hidden_ap, 1);
+	if (!iwdh) {
+		connman_error("Out of memory creating IWD hidden ap");
+		return;
+	}
+
+	iwdh->device = g_strdup(path);
+	iwdh->type = g_strdup(type);
+	iwdh->path = g_strdup_printf("%s/hidden_%s", iwdh->device, iwdh->type);
+	iwdh->address = g_strdup(address);
+	iwdh->signal_strength = signal_strength;
+
+	g_hash_table_replace(iwds->hidden_aps, iwdh->path, iwdh);
+
+	create_hidden_network(iwdh);
+}
+
+static void hidden_access_points_cb(DBusMessage *message, void *user_data)
+{
+	DBusMessageIter array, entry;
+	struct iwd_station *iwds;
+	char *path = user_data;
+
+	DBG("");
+
+	if (!dbus_message_iter_init(message, &array))
+		return;
+
+	if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_ARRAY)
+		return;
+
+	iwds = g_hash_table_lookup(stations, path);
+	if (iwds)
+		g_hash_table_remove_all(iwds->hidden_aps);
+
+	dbus_message_iter_recurse(&array, &entry);
+	while (dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRUCT) {
+		create_hidden_ap(path, &entry);
+	}
+}
+
+static void update_hidden_networks(struct iwd_station *iwds)
+{
+	if (!g_dbus_proxy_method_call(iwds->proxy,
+					"GetHiddenAccessPoints",
+					NULL, hidden_access_points_cb,
+					g_strdup(iwds->path), g_free))
+		DBG("GetHiddenAccessPoints() failed");
+}
+
 static void station_property_change(GDBusProxy *proxy, const char *name,
 		DBusMessageIter *iter, void *user_data)
 {
@@ -1234,6 +1352,7 @@ static void station_property_change(GDBusProxy *proxy, const char *name,
 					CONNMAN_SERVICE_TYPE_WIFI, true);
 		} else {
 			update_signal_strength(iwds);
+			update_hidden_networks(iwds);
 		}
 
 
@@ -1354,9 +1473,26 @@ static void station_free(gpointer data)
 	}
 	g_free(iwds->path);
 	g_free(iwds->connected_network);
+	g_hash_table_destroy(iwds->hidden_aps);
 	g_free(iwds);
 }
 
+static void hidden_ap_free(gpointer data)
+{
+	struct iwd_hidden_ap *iwdh = data;
+	struct iwd_network *iwdn;
+
+	iwdn = g_hash_table_lookup(networks, iwdh->path);
+	if (iwdn)
+		remove_network(iwdn);
+
+	g_free(iwdh->path);
+	g_free(iwdh->device);
+	g_free(iwdh->address);
+	g_free(iwdh->type);
+	g_free(iwdh);
+}
+
 static void ap_free(gpointer data)
 {
 	struct iwd_ap *iwdap = data;
@@ -1717,6 +1853,9 @@ static void create_station(GDBusProxy *proxy)
 	iwds->connected_network = g_strdup(proxy_get_string(proxy, "ConnectedNetwork"));
 	iwds->scanning = proxy_get_bool(proxy, "Scanning");
 
+	iwds->hidden_aps = g_hash_table_new_full(g_str_hash, g_str_equal, NULL,
+			hidden_ap_free);
+
 	DBG("state '%s' connected_network %s scanning %d",
 		iwds->state, iwds->connected_network, iwds->scanning);
 
-- 
2.25.1

----------------------------------------------------------------------------------------------------
> Thanks for the feature. The patch looks pretty good. Just a minor nitpick for
> this and the other 2 patches.
Great.

> Just use g_new0(). There is no use to try to work around small memory
> allocation failures. First they are very likely never to be observed and
> if they do our failure path is not tested. Even more...
I have fixed it in the patch, but a cleanup of the former sources of iwd.c
is probably still necessary.

Emmanuel