From f2be625a07191c4256de0ce2ffbaad404dfe7c99 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 13 Aug 2018 17:21:50 +0200 Subject: [PATCH 01/13] wifi/iwd: Check g_dbus_proxy_get_cached_property return values Instead of passing the return values to g_variant_get_string or g_variant_boolean and then checking the return value of that call, add wrappers that first check's whether the variant is non-NULL and of the right type. g_variant_get_string doesn't allow a NULL parameter and will also never return NULL according to the docs. For the State property we assume a state "unknown" and emit a warning if the property can't be read, "unknown" is also a string in IWD itself which would be returned if something went really wrong. In any case this shouldn't happen. [thaller@redhat.com: fix missing initialization of nm_auto() variable interfaces.] --- src/devices/wifi/nm-device-iwd.c | 41 +++++++++++++++---- src/devices/wifi/nm-iwd-manager.c | 65 +++++++++++++++++-------------- 2 files changed, 69 insertions(+), 37 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 3be6159f2..b880340ca 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -772,6 +772,32 @@ complete_connection (NMDevice *device, return TRUE; } +static gboolean +get_variant_boolean (GVariant *v, const char *property) +{ + if (!v || !g_variant_is_of_type (v, G_VARIANT_TYPE_BOOLEAN)) { + nm_log_warn (LOGD_DEVICE | LOGD_WIFI, + "Property %s not cached or not boolean type", property); + + return FALSE; + } + + return g_variant_get_boolean (v); +} + +static const char * +get_variant_state (GVariant *v) +{ + if (!v || !g_variant_is_of_type (v, G_VARIANT_TYPE_STRING)) { + nm_log_warn (LOGD_DEVICE | LOGD_WIFI, + "State property not cached or not a string"); + + return "unknown"; + } + + return g_variant_get_string (v, NULL); +} + static gboolean is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) { @@ -783,7 +809,7 @@ is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) return FALSE; value = g_dbus_proxy_get_cached_property (priv->dbus_proxy, "Powered"); - return g_variant_get_boolean (value); + return get_variant_boolean (value, "Powered"); } static gboolean @@ -1756,7 +1782,8 @@ state_changed (NMDeviceIwd *self, const char *new_state) NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT); return; - } + } else if (nm_streq (new_state, "unknown")) + return; _LOGE (LOGD_WIFI, "State %s unknown", new_state); } @@ -1802,13 +1829,13 @@ properties_changed (GDBusProxy *proxy, GVariant *changed_properties, g_variant_get (changed_properties, "a{sv}", &iter); while (g_variant_iter_next (iter, "{&sv}", &key, &value)) { if (!strcmp (key, "State")) - state_changed (self, g_variant_get_string (value, NULL)); + state_changed (self, get_variant_state (value)); if (!strcmp (key, "Scanning")) - scanning_changed (self, g_variant_get_boolean (value)); + scanning_changed (self, get_variant_boolean (value, "Scanning")); if (!strcmp (key, "Powered")) - powered_changed (self, g_variant_get_boolean (value)); + powered_changed (self, get_variant_boolean (value, "Powered")); g_variant_unref (value); } @@ -1849,12 +1876,12 @@ nm_device_iwd_set_dbus_object (NMDeviceIwd *self, GDBusObject *object) priv->dbus_proxy = G_DBUS_PROXY (interface); value = g_dbus_proxy_get_cached_property (priv->dbus_proxy, "Scanning"); - priv->scanning = g_variant_get_boolean (value); + priv->scanning = get_variant_boolean (value, "Scanning"); g_variant_unref (value); priv->scan_requested = FALSE; value = g_dbus_proxy_get_cached_property (priv->dbus_proxy, "State"); - state_changed (self, g_variant_get_string (value, NULL)); + state_changed (self, get_variant_state (value)); g_variant_unref (value); g_signal_connect (priv->dbus_proxy, "g-properties-changed", diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 37210c99e..05b841637 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -83,6 +83,32 @@ G_DEFINE_TYPE (NMIwdManager, nm_iwd_manager, G_TYPE_OBJECT) /*****************************************************************************/ +static const char * +get_variant_string_or_null (GVariant *v) +{ + if (!v) + return NULL; + + if ( !g_variant_is_of_type (v, G_VARIANT_TYPE_STRING) + && !g_variant_is_of_type (v, G_VARIANT_TYPE_OBJECT_PATH)) + return NULL; + + return g_variant_get_string (v, NULL); +} + +static const char * +get_property_string_or_null (GDBusProxy *proxy, const char *property) +{ + gs_unref_variant GVariant *value = NULL; + + if (!proxy || !property) + return NULL; + + value = g_dbus_proxy_get_cached_property (proxy, property); + + return get_variant_string_or_null (value); +} + static void agent_dbus_method_cb (GDBusConnection *connection, const char *sender, const char *object_path, @@ -95,7 +121,6 @@ agent_dbus_method_cb (GDBusConnection *connection, NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); const char *network_path, *device_path, *ifname; gs_unref_object GDBusInterface *network = NULL, *device_obj = NULL; - gs_unref_variant GVariant *value = NULL; int ifindex; NMDevice *device; gs_free char *name_owner = NULL; @@ -113,9 +138,8 @@ agent_dbus_method_cb (GDBusConnection *connection, network = g_dbus_object_manager_get_interface (priv->object_manager, network_path, NM_IWD_NETWORK_INTERFACE); - value = g_dbus_proxy_get_cached_property (G_DBUS_PROXY (network), "Device"); - device_path = g_variant_get_string (value, NULL); + device_path = get_property_string_or_null (G_DBUS_PROXY (network), "Device"); if (!device_path) { _LOGD ("agent-request: device not cached for network %s in IWD Agent request", network_path); @@ -125,10 +149,8 @@ agent_dbus_method_cb (GDBusConnection *connection, device_obj = g_dbus_object_manager_get_interface (priv->object_manager, device_path, NM_IWD_DEVICE_INTERFACE); - g_variant_unref (value); - value = g_dbus_proxy_get_cached_property (G_DBUS_PROXY (device_obj), "Name"); - ifname = g_variant_get_string (value, NULL); + ifname = get_property_string_or_null (G_DBUS_PROXY (device_obj), "Name"); if (!ifname) { _LOGD ("agent-request: name not cached for device %s in IWD Agent request", device_path); @@ -257,7 +279,6 @@ set_device_dbus_object (NMIwdManager *self, GDBusInterface *interface, { NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); GDBusProxy *proxy; - GVariant *value; const char *ifname; int ifindex; NMDevice *device; @@ -273,16 +294,14 @@ set_device_dbus_object (NMIwdManager *self, GDBusInterface *interface, NM_IWD_DEVICE_INTERFACE)) return; - value = g_dbus_proxy_get_cached_property (proxy, "Name"); - if (!value) { + ifname = get_property_string_or_null (proxy, "Name"); + if (!ifname) { _LOGE ("Name not cached for Device at %s", g_dbus_proxy_get_object_path (proxy)); return; } - ifname = g_variant_get_string (value, NULL); ifindex = if_nametoindex (ifname); - g_variant_unref (value); if (!ifindex) { _LOGE ("if_nametoindex failed for Name %s for Device at %s: %i", @@ -388,10 +407,10 @@ list_known_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) while (g_variant_iter_next (props, "{&sv}", &key, &val)) { if (!strcmp (key, "Name")) - name = g_variant_get_string (val, NULL); + name = get_variant_string_or_null (val); if (!strcmp (key, "Type")) - type = g_variant_get_string (val, NULL); + type = get_variant_string_or_null (val); g_variant_unref (val); } @@ -492,28 +511,14 @@ device_added (NMManager *manager, NMDevice *device, gpointer user_data) objects = g_dbus_object_manager_get_objects (priv->object_manager); for (iter = objects; iter; iter = iter->next) { GDBusObject *object = G_DBUS_OBJECT (iter->data); - GDBusInterface *interface; - GDBusProxy *proxy; - GVariant *value; + gs_unref_object GDBusInterface *interface = NULL; const char *obj_ifname; interface = g_dbus_object_get_interface (object, NM_IWD_DEVICE_INTERFACE); - if (!interface) - continue; + obj_ifname = get_property_string_or_null ((GDBusProxy *) interface, "Name"); - proxy = G_DBUS_PROXY (interface); - value = g_dbus_proxy_get_cached_property (proxy, "Name"); - if (!value) { - g_object_unref (interface); - continue; - } - - obj_ifname = g_variant_get_string (value, NULL); - g_variant_unref (value); - g_object_unref (interface); - - if (strcmp (nm_device_get_iface (device), obj_ifname)) + if (!obj_ifname || strcmp (nm_device_get_iface (device), obj_ifname)) continue; nm_device_iwd_set_dbus_object (NM_DEVICE_IWD (device), object); From eec61a8e81334442f40ef6434d20a08f24f62597 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 13 Aug 2018 18:10:14 +0200 Subject: [PATCH 02/13] wifi/iwd: Drop usage of the KnownNetworks IWD API Before 0.5 IWD has changed the known networks API to expose separate objects for each known network and dropped the KnownNetworks manager-like interface so stop using that interface. Following patches will add tracking of the known networks through ObjectManager. --- src/devices/wifi/nm-iwd-manager.c | 88 ------------------------------- 1 file changed, 88 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 05b841637..68ab4181a 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -376,92 +376,6 @@ known_network_free (KnownNetworkData *network) g_free (network); } -static void -list_known_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) -{ - NMIwdManager *self = user_data; - NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); - gs_free_error GError *error = NULL; - gs_unref_variant GVariant *variant = NULL; - GVariantIter *networks, *props; - - variant = _nm_dbus_proxy_call_finish (G_DBUS_PROXY (source), res, - G_VARIANT_TYPE ("(aa{sv})"), - &error); - if (!variant) { - _LOGE ("ListKnownNetworks() failed: %s", error->message); - return; - } - - g_slist_free_full (priv->known_networks, (GDestroyNotify) known_network_free); - priv->known_networks = NULL; - - g_variant_get (variant, "(aa{sv})", &networks); - - while (g_variant_iter_next (networks, "a{sv}", &props)) { - const char *key; - const char *name = NULL; - const char *type = NULL; - GVariant *val; - KnownNetworkData *network_data; - - while (g_variant_iter_next (props, "{&sv}", &key, &val)) { - if (!strcmp (key, "Name")) - name = get_variant_string_or_null (val); - - if (!strcmp (key, "Type")) - type = get_variant_string_or_null (val); - - g_variant_unref (val); - } - - if (!name || !type) - goto next; - - network_data = g_new (KnownNetworkData, 1); - network_data->name = g_strdup (name); - if (!strcmp (type, "open")) - network_data->security = NM_IWD_NETWORK_SECURITY_NONE; - else if (!strcmp (type, "psk")) - network_data->security = NM_IWD_NETWORK_SECURITY_PSK; - else if (!strcmp (type, "8021x")) - network_data->security = NM_IWD_NETWORK_SECURITY_8021X; - - priv->known_networks = g_slist_append (priv->known_networks, - network_data); - -next: - g_variant_iter_free (props); - } - - g_variant_iter_free (networks); - - /* For completness we may want to call nm_device_emit_recheck_auto_activate - * and nm_device_recheck_available_connections for all affected devices - * now but the ListKnownNetworks call should have been really fast, - * faster than any scan on any newly created devices could have happened. - */ -} - -static void -update_known_networks (NMIwdManager *self) -{ - NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); - GDBusInterface *known_networks_if; - - known_networks_if = g_dbus_object_manager_get_interface (priv->object_manager, - "/", - NM_IWD_KNOWN_NETWORKS_INTERFACE); - - g_dbus_proxy_call (G_DBUS_PROXY (known_networks_if), - "ListKnownNetworks", - g_variant_new ("()"), - G_DBUS_CALL_FLAGS_NONE, -1, - priv->cancellable, list_known_networks_cb, self); - - g_object_unref (known_networks_if); -} - static void prepare_object_manager (NMIwdManager *self); static void @@ -580,8 +494,6 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data) if (priv->agent_id) register_agent (self); - - update_known_networks (self); } } From 2f941c079095d66c542e7b855b7ff6f3a9db076c Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 13 Aug 2018 18:14:32 +0200 Subject: [PATCH 03/13] wifi/iwd: Drop nm_iwd_manager_network_connected There's no need anymore for NMIwdManager to know when a network has been connected to, InterfaceAdded signals are now emitted when a network is saved as a Known Network. --- src/devices/wifi/nm-device-iwd.c | 3 --- src/devices/wifi/nm-iwd-manager.c | 16 ---------------- src/devices/wifi/nm-iwd-manager.h | 2 -- 3 files changed, 21 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index b880340ca..bf1f3d876 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -1331,9 +1331,6 @@ network_connect_cb (GObject *source, GAsyncResult *res, gpointer user_data) ssid_utf8); nm_device_activate_schedule_stage3_ip_config_start (device); - nm_iwd_manager_network_connected (nm_iwd_manager_get (), ssid_utf8, - get_connection_iwd_security (connection)); - return; failed: diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 68ab4181a..44137e873 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -527,22 +527,6 @@ nm_iwd_manager_is_known_network (NMIwdManager *self, const char *name, return false; } -void -nm_iwd_manager_network_connected (NMIwdManager *self, const char *name, - NMIwdNetworkSecurity security) -{ - NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); - KnownNetworkData *network_data; - - if (nm_iwd_manager_is_known_network (self, name, security)) - return; - - network_data = g_new (KnownNetworkData, 1); - network_data->name = g_strdup (name); - network_data->security = security; - priv->known_networks = g_slist_append (priv->known_networks, network_data); -} - /*****************************************************************************/ NM_DEFINE_SINGLETON_GETTER (NMIwdManager, nm_iwd_manager_get, diff --git a/src/devices/wifi/nm-iwd-manager.h b/src/devices/wifi/nm-iwd-manager.h index 6e9bff879..1c867edb6 100644 --- a/src/devices/wifi/nm-iwd-manager.h +++ b/src/devices/wifi/nm-iwd-manager.h @@ -59,7 +59,5 @@ NMIwdManager *nm_iwd_manager_get (void); gboolean nm_iwd_manager_is_known_network (NMIwdManager *self, const char *name, NMIwdNetworkSecurity security); -void nm_iwd_manager_network_connected (NMIwdManager *self, const char *name, - NMIwdNetworkSecurity security); #endif /* __NETWORKMANAGER_IWD_MANAGER_H__ */ From 78303e1ab8ba02b15682d81e886f72f7401a3453 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 13 Aug 2018 18:51:24 +0200 Subject: [PATCH 04/13] wifi/iwd: Convert manager.known_networks to a GHashTable --- src/devices/wifi/nm-iwd-manager.c | 58 +++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 44137e873..512d9a036 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -34,8 +34,13 @@ /*****************************************************************************/ typedef struct { - char *name; + const char *name; NMIwdNetworkSecurity security; + char buf[0]; +} KnownNetworkId; + +typedef struct { + GDBusProxy *known_network; } KnownNetworkData; typedef struct { @@ -45,7 +50,7 @@ typedef struct { GDBusObjectManager *object_manager; guint agent_id; char *agent_path; - GSList *known_networks; + GHashTable *known_networks; } NMIwdManagerPrivate; struct _NMIwdManager { @@ -273,6 +278,30 @@ register_agent (NMIwdManager *self) /*****************************************************************************/ +static guint +known_network_id_hash (KnownNetworkId *id) +{ + return g_str_hash (id->name) + id->security; +} + +static gboolean +known_network_id_equal (KnownNetworkId *a, KnownNetworkId *b) +{ + return g_str_equal (a->name, b->name) && a->security == b->security; +} + +static void +known_network_data_free (KnownNetworkData *network) +{ + if (!network) + return; + + g_object_unref (network->known_network); + g_slice_free (KnownNetworkData, network); +} + +/*****************************************************************************/ + static void set_device_dbus_object (NMIwdManager *self, GDBusInterface *interface, GDBusObject *object) @@ -369,13 +398,6 @@ object_added (NMIwdManager *self, GDBusObject *object) g_list_free_full (interfaces, g_object_unref); } -static void -known_network_free (KnownNetworkData *network) -{ - g_free (network->name); - g_free (network); -} - static void prepare_object_manager (NMIwdManager *self); static void @@ -515,16 +537,9 @@ nm_iwd_manager_is_known_network (NMIwdManager *self, const char *name, NMIwdNetworkSecurity security) { NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); - const GSList *iter; + KnownNetworkId kn_id = { name, security }; - for (iter = priv->known_networks; iter; iter = g_slist_next (iter)) { - const KnownNetworkData *network = iter->data; - - if (!strcmp (network->name, name) && network->security == security) - return true; - } - - return false; + return g_hash_table_contains (priv->known_networks, &kn_id); } /*****************************************************************************/ @@ -543,6 +558,10 @@ nm_iwd_manager_init (NMIwdManager *self) priv->cancellable = g_cancellable_new (); + priv->known_networks = g_hash_table_new_full ((GHashFunc) known_network_id_hash, + (GEqualFunc) known_network_id_equal, g_free, + (GDestroyNotify) known_network_data_free); + prepare_object_manager (self); } @@ -574,7 +593,8 @@ dispose (GObject *object) nm_clear_g_cancellable (&priv->cancellable); - g_slist_free_full (priv->known_networks, (GDestroyNotify) known_network_free); + if (priv->known_networks) + g_hash_table_unref (priv->known_networks); priv->known_networks = NULL; if (priv->manager) { From 142d83b01973b843ea2eba38d0c19eee2c8dfbb6 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 13 Aug 2018 19:25:37 +0200 Subject: [PATCH 05/13] wifi/iwd: Track known networks using interface-added/-removed signals The known networks hash table is indexed by the (ssid, security) tuple for fast lookups both on DBus signals related to an IWD known network and local NMConnection signals such as on removal. --- src/devices/wifi/nm-iwd-manager.c | 117 ++++++++++++++++++++++++------ src/devices/wifi/nm-iwd-manager.h | 2 +- 2 files changed, 96 insertions(+), 23 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 512d9a036..d9d539114 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -278,6 +278,20 @@ register_agent (NMIwdManager *self) /*****************************************************************************/ +static KnownNetworkId * +known_network_id_new (const char *name, NMIwdNetworkSecurity security) +{ + KnownNetworkId *id; + size_t strsize = strlen (name) + 1; + + id = g_malloc (sizeof (KnownNetworkId) + strsize); + id->name = id->buf; + id->security = security; + memcpy (id->buf, name, strsize); + + return id; +} + static guint known_network_id_hash (KnownNetworkId *id) { @@ -303,26 +317,14 @@ known_network_data_free (KnownNetworkData *network) /*****************************************************************************/ static void -set_device_dbus_object (NMIwdManager *self, GDBusInterface *interface, +set_device_dbus_object (NMIwdManager *self, GDBusProxy *proxy, GDBusObject *object) { NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); - GDBusProxy *proxy; const char *ifname; int ifindex; NMDevice *device; - if (!priv->running) - return; - - g_return_if_fail (G_IS_DBUS_PROXY (interface)); - - proxy = G_DBUS_PROXY (interface); - - if (strcmp (g_dbus_proxy_get_interface_name (proxy), - NM_IWD_DEVICE_INTERFACE)) - return; - ifname = get_property_string_or_null (proxy, "Name"); if (!ifname) { _LOGE ("Name not cached for Device at %s", @@ -352,8 +354,50 @@ interface_added (GDBusObjectManager *object_manager, GDBusObject *object, GDBusInterface *interface, gpointer user_data) { NMIwdManager *self = user_data; + NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); + GDBusProxy *proxy; + const char *iface_name; - set_device_dbus_object (self, interface, object); + if (!priv->running) + return; + + g_return_if_fail (G_IS_DBUS_PROXY (interface)); + + proxy = G_DBUS_PROXY (interface); + iface_name = g_dbus_proxy_get_interface_name (proxy); + + if (nm_streq (iface_name, NM_IWD_DEVICE_INTERFACE)) { + set_device_dbus_object (self, proxy, object); + return; + } + + if (nm_streq (iface_name, NM_IWD_KNOWN_NETWORK_INTERFACE)) { + KnownNetworkId *id; + KnownNetworkData *data; + NMIwdNetworkSecurity security; + const char *type_str, *name; + + type_str = get_property_string_or_null (proxy, "Type"); + name = get_property_string_or_null (proxy, "Name"); + if (!type_str || !name) + return; + + if (nm_streq (type_str, "open")) + security = NM_IWD_NETWORK_SECURITY_NONE; + else if (nm_streq (type_str, "psk")) + security = NM_IWD_NETWORK_SECURITY_PSK; + else if (nm_streq (type_str, "8021x")) + security = NM_IWD_NETWORK_SECURITY_8021X; + else + return; + + id = known_network_id_new (name, security); + + data = g_slice_new (KnownNetworkData); + data->known_network = (GDBusProxy *) g_object_ref (proxy); + g_hash_table_insert (priv->known_networks, id, data); + return; + } } static void @@ -361,15 +405,41 @@ interface_removed (GDBusObjectManager *object_manager, GDBusObject *object, GDBusInterface *interface, gpointer user_data) { NMIwdManager *self = user_data; + NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE (self); + GDBusProxy *proxy; + const char *iface_name; - /* - * TODO: we may need to save the GDBusInterface or GDBusObject - * pointer in the hash table because we may be no longer able to - * access the Name property or map the name to ifindex with - * if_nametoindex at this point. - */ + g_return_if_fail (G_IS_DBUS_PROXY (interface)); - set_device_dbus_object (self, interface, NULL); + proxy = G_DBUS_PROXY (interface); + iface_name = g_dbus_proxy_get_interface_name (proxy); + + if (nm_streq (iface_name, NM_IWD_DEVICE_INTERFACE)) { + set_device_dbus_object (self, proxy, NULL); + return; + } + + if (nm_streq (iface_name, NM_IWD_KNOWN_NETWORK_INTERFACE)) { + KnownNetworkId id; + const char *type_str; + + type_str = get_property_string_or_null (proxy, "Type"); + id.name = get_property_string_or_null (proxy, "Name"); + if (!type_str || !id.name) + return; + + if (nm_streq (type_str, "open")) + id.security = NM_IWD_NETWORK_SECURITY_NONE; + else if (nm_streq (type_str, "psk")) + id.security = NM_IWD_NETWORK_SECURITY_PSK; + else if (nm_streq (type_str, "8021x")) + id.security = NM_IWD_NETWORK_SECURITY_8021X; + else + return; + + g_hash_table_remove (priv->known_networks, &id); + return; + } } static gboolean @@ -389,10 +459,11 @@ object_added (NMIwdManager *self, GDBusObject *object) GList *interfaces, *iter; interfaces = g_dbus_object_get_interfaces (object); + for (iter = interfaces; iter; iter = iter->next) { GDBusInterface *interface = G_DBUS_INTERFACE (iter->data); - set_device_dbus_object (self, interface, object); + interface_added (NULL, object, interface, self); } g_list_free_full (interfaces, g_object_unref); @@ -508,6 +579,8 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data) g_signal_connect (priv->object_manager, "interface-removed", G_CALLBACK (interface_removed), self); + g_hash_table_remove_all (priv->known_networks); + objects = g_dbus_object_manager_get_objects (object_manager); for (iter = objects; iter; iter = iter->next) object_added (self, G_DBUS_OBJECT (iter->data)); diff --git a/src/devices/wifi/nm-iwd-manager.h b/src/devices/wifi/nm-iwd-manager.h index 1c867edb6..13622919c 100644 --- a/src/devices/wifi/nm-iwd-manager.h +++ b/src/devices/wifi/nm-iwd-manager.h @@ -33,7 +33,7 @@ #define NM_IWD_AGENT_INTERFACE "net.connman.iwd.Agent" #define NM_IWD_WSC_INTERFACE \ "net.connman.iwd.WiFiSimpleConfiguration" -#define NM_IWD_KNOWN_NETWORKS_INTERFACE "net.connman.iwd.KnownNetworks" +#define NM_IWD_KNOWN_NETWORK_INTERFACE "net.connman.iwd.KnownNetwork" #define NM_IWD_SIGNAL_AGENT_INTERFACE "net.connman.iwd.SignalLevelAgent" typedef enum { From 43ea446a50d62539c58fd30106a369b9386a24ab Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 11 Jun 2018 16:06:47 +0200 Subject: [PATCH 06/13] wifi: Move get_connection_iwd_security to nm-wifi-utils.c Make this function public. I'm not sure if at this point it makes much sense to add a new file for iwd-specific utilities. While there add a way for the function to return error if security type can't be mapped to an IWD-supported security type. --- src/devices/wifi/nm-device-iwd.c | 41 +++++++++---------------------- src/devices/wifi/nm-iwd-manager.h | 8 +----- src/devices/wifi/nm-wifi-utils.c | 36 +++++++++++++++++++++++++++ src/devices/wifi/nm-wifi-utils.h | 10 ++++++++ 4 files changed, 59 insertions(+), 36 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index bf1f3d876..5781bf9a0 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -448,33 +448,12 @@ deactivate_async (NMDevice *device, G_DBUS_CALL_FLAGS_NONE, -1, cancellable, disconnect_cb, ctx); } -static NMIwdNetworkSecurity -get_connection_iwd_security (NMConnection *connection) -{ - NMSettingWirelessSecurity *s_wireless_sec; - const char *key_mgmt = NULL; - - s_wireless_sec = nm_connection_get_setting_wireless_security (connection); - if (!s_wireless_sec) - return NM_IWD_NETWORK_SECURITY_NONE; - - key_mgmt = nm_setting_wireless_security_get_key_mgmt (s_wireless_sec); - nm_assert (key_mgmt); - - if (!strcmp (key_mgmt, "none") || !strcmp (key_mgmt, "ieee8021x")) - return NM_IWD_NETWORK_SECURITY_WEP; - - if (!strcmp (key_mgmt, "wpa-psk")) - return NM_IWD_NETWORK_SECURITY_PSK; - - nm_assert (!strcmp (key_mgmt, "wpa-eap")); - return NM_IWD_NETWORK_SECURITY_8021X; -} - static gboolean is_connection_known_network (NMConnection *connection) { NMSettingWireless *s_wireless; + NMIwdNetworkSecurity security; + gboolean security_ok; GBytes *ssid; gs_free char *ssid_utf8 = NULL; @@ -487,9 +466,13 @@ is_connection_known_network (NMConnection *connection) return FALSE; ssid_utf8 = _nm_utils_ssid_to_utf8 (ssid); + + security = nm_wifi_connection_get_iwd_security (connection, &security_ok); + if (!security_ok) + return FALSE; + return nm_iwd_manager_is_known_network (nm_iwd_manager_get (), - ssid_utf8, - get_connection_iwd_security (connection)); + ssid_utf8, security); } static gboolean @@ -543,7 +526,7 @@ check_connection_compatible (NMDevice *device, NMConnection *connection, GError /* 8021x networks can only be used if they've been provisioned on the IWD side and * thus are Known Networks. */ - if (get_connection_iwd_security (connection) == NM_IWD_NETWORK_SECURITY_8021X) { + if (nm_wifi_connection_get_iwd_security (connection, NULL) == NM_IWD_NETWORK_SECURITY_8021X) { if (!is_connection_known_network (connection)) { nm_utils_error_set_literal (error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_TEMPORARY, "802.1x profile is not a known network"); @@ -587,7 +570,7 @@ check_connection_available (NMDevice *device, /* 8021x networks can only be used if they've been provisioned on the IWD side and * thus are Known Networks. */ - if (get_connection_iwd_security (connection) == NM_IWD_NETWORK_SECURITY_8021X) { + if (nm_wifi_connection_get_iwd_security (connection, NULL) == NM_IWD_NETWORK_SECURITY_8021X) { if (!is_connection_known_network (connection)) { nm_utils_error_set_literal (error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_TEMPORARY, "network is not known to iwd"); @@ -731,7 +714,7 @@ complete_connection (NMDevice *device, /* 8021x networks can only be used if they've been provisioned on the IWD side and * thus are Known Networks. */ - if (get_connection_iwd_security (connection) == NM_IWD_NETWORK_SECURITY_8021X) { + if (nm_wifi_connection_get_iwd_security (connection, NULL) == NM_IWD_NETWORK_SECURITY_8021X) { if (!is_connection_known_network (connection)) { g_set_error_literal (error, NM_CONNECTION_ERROR, @@ -861,7 +844,7 @@ can_auto_connect (NMDevice *device, /* 8021x networks can only be used if they've been provisioned on the IWD side and * thus are Known Networks. */ - if (get_connection_iwd_security (connection) == NM_IWD_NETWORK_SECURITY_8021X) { + if (nm_wifi_connection_get_iwd_security (connection, NULL) == NM_IWD_NETWORK_SECURITY_8021X) { if (!is_connection_known_network (connection)) return FALSE; } diff --git a/src/devices/wifi/nm-iwd-manager.h b/src/devices/wifi/nm-iwd-manager.h index 13622919c..59b70f6e3 100644 --- a/src/devices/wifi/nm-iwd-manager.h +++ b/src/devices/wifi/nm-iwd-manager.h @@ -22,6 +22,7 @@ #define __NETWORKMANAGER_IWD_MANAGER_H__ #include "devices/nm-device.h" +#include "nm-wifi-utils.h" #define NM_IWD_BUS_TYPE G_BUS_TYPE_SYSTEM #define NM_IWD_SERVICE "net.connman.iwd" @@ -36,13 +37,6 @@ #define NM_IWD_KNOWN_NETWORK_INTERFACE "net.connman.iwd.KnownNetwork" #define NM_IWD_SIGNAL_AGENT_INTERFACE "net.connman.iwd.SignalLevelAgent" -typedef enum { - NM_IWD_NETWORK_SECURITY_NONE, - NM_IWD_NETWORK_SECURITY_WEP, - NM_IWD_NETWORK_SECURITY_PSK, - NM_IWD_NETWORK_SECURITY_8021X, -} NMIwdNetworkSecurity; - #define NM_TYPE_IWD_MANAGER (nm_iwd_manager_get_type ()) #define NM_IWD_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_IWD_MANAGER, NMIwdManager)) #define NM_IWD_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_IWD_MANAGER, NMIwdManagerClass)) diff --git a/src/devices/wifi/nm-wifi-utils.c b/src/devices/wifi/nm-wifi-utils.c index c9806a53c..db8f63705 100644 --- a/src/devices/wifi/nm-wifi-utils.c +++ b/src/devices/wifi/nm-wifi-utils.c @@ -815,3 +815,39 @@ nm_wifi_utils_is_manf_default_ssid (GBytes *ssid) } return FALSE; } + +NMIwdNetworkSecurity +nm_wifi_connection_get_iwd_security (NMConnection *connection, + gboolean *mapped) +{ + NMSettingWirelessSecurity *s_wireless_sec; + const char *key_mgmt = NULL; + + if (!nm_connection_get_setting_wireless (connection)) + goto error; + + if (mapped) + *mapped = TRUE; + + s_wireless_sec = nm_connection_get_setting_wireless_security (connection); + if (!s_wireless_sec) + return NM_IWD_NETWORK_SECURITY_NONE; + + key_mgmt = nm_setting_wireless_security_get_key_mgmt (s_wireless_sec); + nm_assert (key_mgmt); + + if (NM_IN_STRSET (key_mgmt, "none", "ieee8021x")) + return NM_IWD_NETWORK_SECURITY_WEP; + + if (nm_streq (key_mgmt, "wpa-psk")) + return NM_IWD_NETWORK_SECURITY_PSK; + + if (nm_streq (key_mgmt, "wpa-eap")) + return NM_IWD_NETWORK_SECURITY_8021X; + +error: + if (mapped) + *mapped = FALSE; + + return NM_IWD_NETWORK_SECURITY_NONE; +} diff --git a/src/devices/wifi/nm-wifi-utils.h b/src/devices/wifi/nm-wifi-utils.h index 88dc37911..03238c246 100644 --- a/src/devices/wifi/nm-wifi-utils.h +++ b/src/devices/wifi/nm-wifi-utils.h @@ -27,6 +27,13 @@ #include "nm-setting-wireless-security.h" #include "nm-setting-8021x.h" +typedef enum { + NM_IWD_NETWORK_SECURITY_NONE, + NM_IWD_NETWORK_SECURITY_WEP, + NM_IWD_NETWORK_SECURITY_PSK, + NM_IWD_NETWORK_SECURITY_8021X, +} NMIwdNetworkSecurity; + gboolean nm_wifi_utils_complete_connection (GBytes *ssid, const char *bssid, NM80211Mode mode, @@ -41,4 +48,7 @@ guint32 nm_wifi_utils_level_to_quality (int val); gboolean nm_wifi_utils_is_manf_default_ssid (GBytes *ssid); +NMIwdNetworkSecurity nm_wifi_connection_get_iwd_security (NMConnection *connection, + gboolean *mapped); + #endif /* __NM_WIFI_UTILS_H__ */ From 977d298c5fb79999bbfddcbd758bf1f41824c970 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Mon, 11 Jun 2018 18:14:41 +0200 Subject: [PATCH 07/13] libnm-core: 8021x: Allow a new eap value "external" To allow connections that mirror IWD's configured WPA-Enterprise networks to be seen as valid by NM, add a new value for the eap key in 802-1x settings. 802-1x.eap stores EAP method names. In the IWD connections we don't know what EAP method is configured and we don't have any of the other 802-1x properties that would be required for the settings to verify. These connections can't be activated on devices managed by wpa_supplicant. --- libnm-core/nm-setting-8021x.c | 3 ++- src/supplicant/nm-supplicant-config.c | 31 ++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-setting-8021x.c b/libnm-core/nm-setting-8021x.c index 529029de8..7e004c903 100644 --- a/libnm-core/nm-setting-8021x.c +++ b/libnm-core/nm-setting-8021x.c @@ -2804,6 +2804,7 @@ static EAPMethodsTable eap_methods_table[] = { { "sim", need_secrets_sim, NULL }, { "gtc", need_secrets_password, verify_identity }, { "otp", NULL, NULL }, // FIXME: implement + { "external", NULL, NULL }, { NULL, NULL, NULL } }; @@ -2812,7 +2813,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) { NMSetting8021x *self = NM_SETTING_802_1X (setting); NMSetting8021xPrivate *priv = NM_SETTING_802_1X_GET_PRIVATE (self); - const char *valid_eap[] = { "leap", "md5", "tls", "peap", "ttls", "sim", "fast", "pwd", NULL }; + const char *valid_eap[] = { "leap", "md5", "tls", "peap", "ttls", "sim", "fast", "pwd", "external", NULL }; GSList *iter; if (error) diff --git a/src/supplicant/nm-supplicant-config.c b/src/supplicant/nm-supplicant-config.c index a0628d74a..042820708 100644 --- a/src/supplicant/nm-supplicant-config.c +++ b/src/supplicant/nm-supplicant-config.c @@ -1001,6 +1001,7 @@ nm_supplicant_config_add_setting_8021x (NMSupplicantConfig *self, guint32 frag, hdrs; gs_free char *frag_str = NULL; NMSetting8021xAuthFlags phase1_auth_flags; + nm_auto_free_gstring GString *eap_str = NULL; g_return_val_if_fail (NM_IS_SUPPLICANT_CONFIG (self), FALSE); g_return_val_if_fail (setting != NULL, FALSE); @@ -1037,20 +1038,40 @@ nm_supplicant_config_add_setting_8021x (NMSupplicantConfig *self, priv->ap_scan = 0; } - if (!ADD_STRING_LIST_VAL (self, setting, 802_1x, eap_method, eap_methods, "eap", ' ', TRUE, NULL, error)) - return FALSE; - - /* Check EAP method for special handling: PEAP + GTC, FAST */ + /* Build the "eap" option string while we check for EAP methods needing + * special handling: PEAP + GTC, FAST, external */ + eap_str = g_string_new (NULL); num_eap = nm_setting_802_1x_get_num_eap_methods (setting); for (i = 0; i < num_eap; i++) { const char *method = nm_setting_802_1x_get_eap_method (setting, i); - if (method && (strcasecmp (method, "fast") == 0)) { + if (!method) + continue; + + if (strcasecmp (method, "fast") == 0) { fast = TRUE; priv->fast_required = TRUE; } + + if (nm_streq (method, "external")) { + if (num_eap == 1) { + g_set_error (error, NM_SUPPLICANT_ERROR, NM_SUPPLICANT_ERROR_CONFIG, + "Connection settings managed externally to NM, connection" + " cannot be used with wpa_supplicant"); + return FALSE; + } + continue; + } + + if (eap_str->len) + g_string_append_c (eap_str, ' '); + g_string_append (eap_str, method); } + g_string_ascii_up (eap_str); + if (eap_str->len && !nm_supplicant_config_add_option (self, "eap", eap_str->str, -1, NULL, error)) + return FALSE; + /* Adjust the fragment size according to MTU, but do not set it higher than 1280-14 * for better compatibility */ hdrs = 14; /* EAPOL + EAP-TLS */ From 2c8161868ecce5f01dc09b1cc19e75c3f811ba15 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Thu, 16 Aug 2018 16:03:51 +0200 Subject: [PATCH 08/13] wifi/iwd: Create connections for IWD-side known networks IWD's mechanism for connecting to EAP networks requires a network config file to be present in IWD's storage. NM and its clients however won't allow a connection to be attempted until a valid NMConnection is created on the NM side for the network. To avoid duplicating the settings from the IWD-side profiles in NM, automatically create NMSettingConnections for EAP networks preconfigured on the IWD side, unless a matching connection already exists. These connections will use the "external" EAP method to mean their EAP settings can't be modified through NM, also they won't be valid for devices configured to use the wpa_supplicant backend unfortunately. Those nm-generated connections can be modified by NM users (makes sense for settings not related to the wifi authentication) in which case they get saved as normal profiles and will not be recreated as nm-generated connections on the next run. I want to additionally handle deleting connections from NM clients so that they're also forgotten by IWD, in a later patch. --- src/devices/wifi/nm-iwd-manager.c | 127 +++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index d9d539114..7beb44e96 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -29,7 +29,9 @@ #include "nm-core-internal.h" #include "nm-manager.h" #include "nm-device-iwd.h" +#include "nm-wifi-utils.h" #include "nm-utils/nm-random-utils.h" +#include "settings/nm-settings.h" /*****************************************************************************/ @@ -41,6 +43,7 @@ typedef struct { typedef struct { GDBusProxy *known_network; + NMSettingsConnection *mirror_connection; } KnownNetworkData; typedef struct { @@ -311,6 +314,8 @@ known_network_data_free (KnownNetworkData *network) return; g_object_unref (network->known_network); + if (network->mirror_connection) + g_object_unref (network->mirror_connection); g_slice_free (KnownNetworkData, network); } @@ -349,6 +354,100 @@ set_device_dbus_object (NMIwdManager *self, GDBusProxy *proxy, nm_device_iwd_set_dbus_object (NM_DEVICE_IWD (device), object); } +/* Create an in-memory NMConnection for a WPA2-Enterprise network that + * has been preprovisioned with an IWD config file so that NM autoconnect + * mechanism and the clients know this networks needs no additional EAP + * configuration from the user. Only do this if no existing connection + * SSID and security type match that network yet. + */ +static void +mirror_8021x_connection (NMIwdManager *self, KnownNetworkId *id, + KnownNetworkData *data) +{ + NMSettings *settings = NM_SETTINGS_GET; + NMSettingsConnection *const *iter; + NMConnection *connection; + NMSettingsConnection *settings_connection; + char uuid[37]; + NMSetting *setting; + GBytes *ssid; + GError *error = NULL; + + for (iter = nm_settings_get_connections (settings, NULL); *iter; iter++) { + const NMSettingsConnection *conn = *iter; + NMIwdNetworkSecurity security; + gs_free char *name = NULL; + NMSettingWireless *s_wifi; + + security = nm_wifi_connection_get_iwd_security (NM_CONNECTION (conn), NULL); + if (security != NM_IWD_NETWORK_SECURITY_8021X) + continue; + + s_wifi = nm_connection_get_setting_wireless (NM_CONNECTION (conn)); + if (!s_wifi) + continue; + + ssid = nm_setting_wireless_get_ssid (s_wifi); + if (!ssid) + continue; + + name = nm_utils_ssid_to_utf8 (g_bytes_get_data (ssid, NULL), + g_bytes_get_size (ssid)); + + /* We already have an NMSettingsConnection matching this + * KnownNetwork, whether it's saved or an in-memory connection + * potentially created by ourselves. Nothing to do here. + */ + if (!strcmp (name, id->name)) + return; + } + + connection = nm_simple_connection_new (); + + setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_CONNECTION, + NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRELESS_SETTING_NAME, + NM_SETTING_CONNECTION_ID, id->name, + NM_SETTING_CONNECTION_UUID, nm_utils_uuid_generate_buf (uuid), + NM_SETTING_CONNECTION_READ_ONLY, TRUE, + NULL)); + nm_connection_add_setting (connection, setting); + + ssid = g_bytes_new (id->name, strlen (id->name)); + setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_WIRELESS, + NM_SETTING_WIRELESS_SSID, ssid, + NM_SETTING_WIRELESS_MODE, NM_SETTING_WIRELESS_MODE_INFRA, + NULL)); + nm_connection_add_setting (connection, setting); + + setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_WIRELESS_SECURITY, + NM_SETTING_WIRELESS_SECURITY_AUTH_ALG, "open", + NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-eap", + NULL)); + nm_connection_add_setting (connection, setting); + + setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_802_1X, NULL)); + nm_setting_802_1x_add_eap_method (NM_SETTING_802_1X (setting), "external"); + nm_connection_add_setting (connection, setting); + + settings_connection = nm_settings_add_connection (settings, connection, + FALSE, &error); + g_object_unref (connection); + + if (!settings_connection) { + _LOGW ("failed to add a mirror NMConnection for IWD's Known Network '%s': %s", + id->name, NM_G_ERROR_MSG (error)); + g_clear_error (&error); + return; + } + + nm_settings_connection_set_flags (settings_connection, + NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED | + NM_SETTINGS_CONNECTION_INT_FLAGS_UNSAVED, + TRUE); + + data->mirror_connection = g_object_ref (settings_connection); +} + static void interface_added (GDBusObjectManager *object_manager, GDBusObject *object, GDBusInterface *interface, gpointer user_data) @@ -393,9 +492,13 @@ interface_added (GDBusObjectManager *object_manager, GDBusObject *object, id = known_network_id_new (name, security); - data = g_slice_new (KnownNetworkData); + data = g_slice_new0 (KnownNetworkData); data->known_network = (GDBusProxy *) g_object_ref (proxy); g_hash_table_insert (priv->known_networks, id, data); + + if (security == NM_IWD_NETWORK_SECURITY_8021X) + mirror_8021x_connection (self, id, data); + return; } } @@ -421,6 +524,7 @@ interface_removed (GDBusObjectManager *object_manager, GDBusObject *object, if (nm_streq (iface_name, NM_IWD_KNOWN_NETWORK_INTERFACE)) { KnownNetworkId id; + KnownNetworkData *data; const char *type_str; type_str = get_property_string_or_null (proxy, "Type"); @@ -437,6 +541,27 @@ interface_removed (GDBusObjectManager *object_manager, GDBusObject *object, else return; + data = g_hash_table_lookup (priv->known_networks, &id); + if (!data) + return; + + if (data->mirror_connection) { + NMSettingsConnectionIntFlags flags; + NMSettingsConnection *mirror_connection = data->mirror_connection; + + flags = nm_settings_connection_get_flags (mirror_connection); + + /* If connection has not been saved since we created it + * in interface_added it too can be removed now. */ + if (flags & NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED) { + data->mirror_connection = NULL; + + nm_settings_connection_delete (mirror_connection, NULL); + + g_object_unref (mirror_connection); + } + } + g_hash_table_remove (priv->known_networks, &id); return; } From be875fe3825ec0c9057f2a8ae8a5c85d38e65868 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 12:17:03 +0200 Subject: [PATCH 09/13] wifi/iwd: in manager's interface_added() ensure known-network ID is not wrongly destroyed Calling g_hash_table_insert() with a key which is already hashed will destroy the *new* key. Since @id is used below, that would be use after free. Fixes: d635caf940551f8f5b52683b8379a1f81c58f8fc --- src/devices/wifi/nm-iwd-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 7beb44e96..8ff3b9845 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -494,7 +494,7 @@ interface_added (GDBusObjectManager *object_manager, GDBusObject *object, data = g_slice_new0 (KnownNetworkData); data->known_network = (GDBusProxy *) g_object_ref (proxy); - g_hash_table_insert (priv->known_networks, id, data); + g_hash_table_replace (priv->known_networks, id, data); if (security == NM_IWD_NETWORK_SECURITY_8021X) mirror_8021x_connection (self, id, data); From ccf36ff4cea6eb6a7ecd872563dfbd3a00f8cfdf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 12:22:32 +0200 Subject: [PATCH 10/13] wifi/iwd: use NMHashState (siphash24) for hashing We shall use nm_hash_*() functions everywhere where we need a hash for a dictionary. --- src/devices/wifi/nm-iwd-manager.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index 8ff3b9845..bdfff05c3 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -298,7 +298,12 @@ known_network_id_new (const char *name, NMIwdNetworkSecurity security) static guint known_network_id_hash (KnownNetworkId *id) { - return g_str_hash (id->name) + id->security; + NMHashState h; + + nm_hash_init (&h, 1947951703u); + nm_hash_update_val (&h, id->security); + nm_hash_update_str (&h, id->name); + return nm_hash_complete (&h); } static gboolean From 1181f88ef1017fafc4546cc396ffb0e1b48b056f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 12:21:39 +0200 Subject: [PATCH 11/13] wifi/iwd: various minor cleanups in nm-iwd-manager.c - prefer "gsize" instead of "size_t". --- src/devices/wifi/nm-iwd-manager.c | 28 +++++++++++++++------------- src/devices/wifi/nm-wifi-utils.c | 7 ++----- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index bdfff05c3..c9122a69f 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -285,7 +285,7 @@ static KnownNetworkId * known_network_id_new (const char *name, NMIwdNetworkSecurity security) { KnownNetworkId *id; - size_t strsize = strlen (name) + 1; + gsize strsize = strlen (name) + 1; id = g_malloc (sizeof (KnownNetworkId) + strsize); id->name = id->buf; @@ -309,7 +309,8 @@ known_network_id_hash (KnownNetworkId *id) static gboolean known_network_id_equal (KnownNetworkId *a, KnownNetworkId *b) { - return g_str_equal (a->name, b->name) && a->security == b->security; + return a->security == b->security + && nm_streq (a->name, b->name); } static void @@ -319,8 +320,7 @@ known_network_data_free (KnownNetworkData *network) return; g_object_unref (network->known_network); - if (network->mirror_connection) - g_object_unref (network->mirror_connection); + nm_g_object_unref (network->mirror_connection); g_slice_free (KnownNetworkData, network); } @@ -366,7 +366,8 @@ set_device_dbus_object (NMIwdManager *self, GDBusProxy *proxy, * SSID and security type match that network yet. */ static void -mirror_8021x_connection (NMIwdManager *self, KnownNetworkId *id, +mirror_8021x_connection (NMIwdManager *self, + KnownNetworkId *id, KnownNetworkData *data) { NMSettings *settings = NM_SETTINGS_GET; @@ -677,7 +678,7 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data) object_manager = g_dbus_object_manager_client_new_for_bus_finish (result, &error); if (object_manager == NULL) { _LOGE ("failed to acquire IWD Object Manager: Wi-Fi will not be available (%s)", - NM_G_ERROR_MSG (error)); + error->message); g_clear_error (&error); return; } @@ -691,11 +692,13 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data) connection = g_dbus_object_manager_client_get_connection (G_DBUS_OBJECT_MANAGER_CLIENT (object_manager)); - priv->agent_id = iwd_agent_export (connection, self, - &priv->agent_path, &error); + priv->agent_id = iwd_agent_export (connection, + self, + &priv->agent_path, + &error); if (!priv->agent_id) { _LOGE ("failed to export the IWD Agent: PSK/8021x WiFi networks will not work: %s", - NM_G_ERROR_MSG (error)); + error->message); g_clear_error (&error); } @@ -762,7 +765,8 @@ nm_iwd_manager_init (NMIwdManager *self) priv->cancellable = g_cancellable_new (); priv->known_networks = g_hash_table_new_full ((GHashFunc) known_network_id_hash, - (GEqualFunc) known_network_id_equal, g_free, + (GEqualFunc) known_network_id_equal, + g_free, (GDestroyNotify) known_network_data_free); prepare_object_manager (self); @@ -796,9 +800,7 @@ dispose (GObject *object) nm_clear_g_cancellable (&priv->cancellable); - if (priv->known_networks) - g_hash_table_unref (priv->known_networks); - priv->known_networks = NULL; + nm_clear_pointer (&priv->known_networks, g_hash_table_destroy); if (priv->manager) { g_signal_handlers_disconnect_by_data (priv->manager, self); diff --git a/src/devices/wifi/nm-wifi-utils.c b/src/devices/wifi/nm-wifi-utils.c index db8f63705..0f7836beb 100644 --- a/src/devices/wifi/nm-wifi-utils.c +++ b/src/devices/wifi/nm-wifi-utils.c @@ -826,8 +826,7 @@ nm_wifi_connection_get_iwd_security (NMConnection *connection, if (!nm_connection_get_setting_wireless (connection)) goto error; - if (mapped) - *mapped = TRUE; + NM_SET_OUT (mapped, TRUE); s_wireless_sec = nm_connection_get_setting_wireless_security (connection); if (!s_wireless_sec) @@ -846,8 +845,6 @@ nm_wifi_connection_get_iwd_security (NMConnection *connection, return NM_IWD_NETWORK_SECURITY_8021X; error: - if (mapped) - *mapped = FALSE; - + NM_SET_OUT (mapped, FALSE); return NM_IWD_NETWORK_SECURITY_NONE; } From 13d8455a7c8be7798bd2b5e60165cf0c18bf897f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 12:39:27 +0200 Subject: [PATCH 12/13] wifi: trust eap methods from profile to be lower-case NMSetting8021x::verify() checks the string values for eap methods. They must all be non-NULL and are not compared case-insensitive. --- src/supplicant/nm-supplicant-config.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/supplicant/nm-supplicant-config.c b/src/supplicant/nm-supplicant-config.c index 042820708..41d510e87 100644 --- a/src/supplicant/nm-supplicant-config.c +++ b/src/supplicant/nm-supplicant-config.c @@ -1045,10 +1045,7 @@ nm_supplicant_config_add_setting_8021x (NMSupplicantConfig *self, for (i = 0; i < num_eap; i++) { const char *method = nm_setting_802_1x_get_eap_method (setting, i); - if (!method) - continue; - - if (strcasecmp (method, "fast") == 0) { + if (nm_streq (method, "fast")) { fast = TRUE; priv->fast_required = TRUE; } @@ -1057,7 +1054,7 @@ nm_supplicant_config_add_setting_8021x (NMSupplicantConfig *self, if (num_eap == 1) { g_set_error (error, NM_SUPPLICANT_ERROR, NM_SUPPLICANT_ERROR_CONFIG, "Connection settings managed externally to NM, connection" - " cannot be used with wpa_supplicant"); + " cannot be used with wpa_supplicant"); return FALSE; } continue; @@ -1069,7 +1066,8 @@ nm_supplicant_config_add_setting_8021x (NMSupplicantConfig *self, } g_string_ascii_up (eap_str); - if (eap_str->len && !nm_supplicant_config_add_option (self, "eap", eap_str->str, -1, NULL, error)) + if ( eap_str->len + && !nm_supplicant_config_add_option (self, "eap", eap_str->str, -1, NULL, error)) return FALSE; /* Adjust the fragment size according to MTU, but do not set it higher than 1280-14 From 099886891287ae994e815b2d3b5304da7bef3b0d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 13:09:40 +0200 Subject: [PATCH 13/13] wifi/iwd: fix tracking of IWD-side known networks - since commit d17d26887c69c8c3adcd6003a7ceea16b4eecd14, a NMSettingsConnection no longer "is-a" NMConnection. Instead, we must call nm_settings_connection_get_connection() to obtain the NMConnection instance. Adjust this in mirror_8021x_connection() - don't leak "ssid" in mirror_8021x_connection() - move deletion of the mirror-connection to known_network_data_free(). Previously, we must have made sure that every g_hash_table_remove() and g_hash_table_insert()(!!) first deletes the mirror connection. Likewise, in got_object_manager() when we call g_hash_table_remove_all(), delete created mirror connections. - rework interface_added() to make it robust against calling interface_added() more than once without removing the interface in between. Essentially, this just means that we first look into "priv->known_networks" to see whether the @id is already tracked. And if so, delete an existing mirror-connection as necessary. --- src/devices/wifi/nm-iwd-manager.c | 121 ++++++++++++++++-------------- 1 file changed, 66 insertions(+), 55 deletions(-) diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c index c9122a69f..2f7bd1609 100644 --- a/src/devices/wifi/nm-iwd-manager.c +++ b/src/devices/wifi/nm-iwd-manager.c @@ -91,6 +91,10 @@ G_DEFINE_TYPE (NMIwdManager, nm_iwd_manager, G_TYPE_OBJECT) /*****************************************************************************/ +static void mirror_8021x_connection_take_and_delete (NMSettingsConnection *sett_conn); + +/*****************************************************************************/ + static const char * get_variant_string_or_null (GVariant *v) { @@ -320,7 +324,7 @@ known_network_data_free (KnownNetworkData *network) return; g_object_unref (network->known_network); - nm_g_object_unref (network->mirror_connection); + mirror_8021x_connection_take_and_delete (network->mirror_connection); g_slice_free (KnownNetworkData, network); } @@ -365,62 +369,57 @@ set_device_dbus_object (NMIwdManager *self, GDBusProxy *proxy, * configuration from the user. Only do this if no existing connection * SSID and security type match that network yet. */ -static void +static NMSettingsConnection * mirror_8021x_connection (NMIwdManager *self, - KnownNetworkId *id, - KnownNetworkData *data) + const char *name) { NMSettings *settings = NM_SETTINGS_GET; - NMSettingsConnection *const *iter; - NMConnection *connection; + NMSettingsConnection *const*iter; + gs_unref_object NMConnection *connection = NULL; NMSettingsConnection *settings_connection; char uuid[37]; NMSetting *setting; - GBytes *ssid; GError *error = NULL; + gs_unref_bytes GBytes *new_ssid = NULL; for (iter = nm_settings_get_connections (settings, NULL); *iter; iter++) { - const NMSettingsConnection *conn = *iter; + NMSettingsConnection *sett_conn = *iter; + NMConnection *conn = nm_settings_connection_get_connection (sett_conn); NMIwdNetworkSecurity security; - gs_free char *name = NULL; + gs_free char *ssid_name = NULL; NMSettingWireless *s_wifi; - security = nm_wifi_connection_get_iwd_security (NM_CONNECTION (conn), NULL); + security = nm_wifi_connection_get_iwd_security (conn, NULL); if (security != NM_IWD_NETWORK_SECURITY_8021X) continue; - s_wifi = nm_connection_get_setting_wireless (NM_CONNECTION (conn)); + s_wifi = nm_connection_get_setting_wireless (conn); if (!s_wifi) continue; - ssid = nm_setting_wireless_get_ssid (s_wifi); - if (!ssid) - continue; - - name = nm_utils_ssid_to_utf8 (g_bytes_get_data (ssid, NULL), - g_bytes_get_size (ssid)); + ssid_name = _nm_utils_ssid_to_utf8 (nm_setting_wireless_get_ssid (s_wifi)); /* We already have an NMSettingsConnection matching this * KnownNetwork, whether it's saved or an in-memory connection * potentially created by ourselves. Nothing to do here. */ - if (!strcmp (name, id->name)) - return; + if (nm_streq (ssid_name, name)) + return NULL; } connection = nm_simple_connection_new (); setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_CONNECTION, NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRELESS_SETTING_NAME, - NM_SETTING_CONNECTION_ID, id->name, + NM_SETTING_CONNECTION_ID, name, NM_SETTING_CONNECTION_UUID, nm_utils_uuid_generate_buf (uuid), NM_SETTING_CONNECTION_READ_ONLY, TRUE, NULL)); nm_connection_add_setting (connection, setting); - ssid = g_bytes_new (id->name, strlen (id->name)); + new_ssid = g_bytes_new (name, strlen (name)); setting = NM_SETTING (g_object_new (NM_TYPE_SETTING_WIRELESS, - NM_SETTING_WIRELESS_SSID, ssid, + NM_SETTING_WIRELESS_SSID, new_ssid, NM_SETTING_WIRELESS_MODE, NM_SETTING_WIRELESS_MODE_INFRA, NULL)); nm_connection_add_setting (connection, setting); @@ -435,23 +434,41 @@ mirror_8021x_connection (NMIwdManager *self, nm_setting_802_1x_add_eap_method (NM_SETTING_802_1X (setting), "external"); nm_connection_add_setting (connection, setting); + if (!nm_connection_normalize (connection, NULL, NULL, NULL)) + return NULL; + settings_connection = nm_settings_add_connection (settings, connection, FALSE, &error); - g_object_unref (connection); - if (!settings_connection) { _LOGW ("failed to add a mirror NMConnection for IWD's Known Network '%s': %s", - id->name, NM_G_ERROR_MSG (error)); - g_clear_error (&error); - return; + name, error->message); + g_error_free (error); + return NULL; } nm_settings_connection_set_flags (settings_connection, NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED | NM_SETTINGS_CONNECTION_INT_FLAGS_UNSAVED, TRUE); + return settings_connection; +} - data->mirror_connection = g_object_ref (settings_connection); +static void +mirror_8021x_connection_take_and_delete (NMSettingsConnection *sett_conn) +{ + NMSettingsConnectionIntFlags flags; + + if (!sett_conn) + return; + + flags = nm_settings_connection_get_flags (sett_conn); + + /* If connection has not been saved since we created it + * in interface_added it too can be removed now. */ + if (NM_FLAGS_HAS (flags, NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED)) + nm_settings_connection_delete (sett_conn, NULL); + + g_object_unref (sett_conn); } static void @@ -481,6 +498,7 @@ interface_added (GDBusObjectManager *object_manager, GDBusObject *object, KnownNetworkData *data; NMIwdNetworkSecurity security; const char *type_str, *name; + NMSettingsConnection *sett_conn = NULL; type_str = get_property_string_or_null (proxy, "Type"); name = get_property_string_or_null (proxy, "Name"); @@ -498,12 +516,27 @@ interface_added (GDBusObjectManager *object_manager, GDBusObject *object, id = known_network_id_new (name, security); - data = g_slice_new0 (KnownNetworkData); - data->known_network = (GDBusProxy *) g_object_ref (proxy); - g_hash_table_replace (priv->known_networks, id, data); + data = g_hash_table_lookup (priv->known_networks, id); + if (data) + g_free (id); + else { + data = g_slice_new0 (KnownNetworkData); + data->known_network = g_object_ref (proxy); + g_hash_table_insert (priv->known_networks, id, data); + } - if (security == NM_IWD_NETWORK_SECURITY_8021X) - mirror_8021x_connection (self, id, data); + if (security == NM_IWD_NETWORK_SECURITY_8021X) { + sett_conn = mirror_8021x_connection (self, name); + + if ( sett_conn + && sett_conn != data->mirror_connection) { + NMSettingsConnection *sett_conn_old = data->mirror_connection; + + data->mirror_connection = nm_g_object_ref (sett_conn); + mirror_8021x_connection_take_and_delete (sett_conn_old); + } + } else + mirror_8021x_connection_take_and_delete (g_steal_pointer (&data->mirror_connection)); return; } @@ -530,7 +563,6 @@ interface_removed (GDBusObjectManager *object_manager, GDBusObject *object, if (nm_streq (iface_name, NM_IWD_KNOWN_NETWORK_INTERFACE)) { KnownNetworkId id; - KnownNetworkData *data; const char *type_str; type_str = get_property_string_or_null (proxy, "Type"); @@ -547,27 +579,6 @@ interface_removed (GDBusObjectManager *object_manager, GDBusObject *object, else return; - data = g_hash_table_lookup (priv->known_networks, &id); - if (!data) - return; - - if (data->mirror_connection) { - NMSettingsConnectionIntFlags flags; - NMSettingsConnection *mirror_connection = data->mirror_connection; - - flags = nm_settings_connection_get_flags (mirror_connection); - - /* If connection has not been saved since we created it - * in interface_added it too can be removed now. */ - if (flags & NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED) { - data->mirror_connection = NULL; - - nm_settings_connection_delete (mirror_connection, NULL); - - g_object_unref (mirror_connection); - } - } - g_hash_table_remove (priv->known_networks, &id); return; }