From c06a55958b96d549726411efd5583ea05df62a86 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 13 Aug 2018 21:21:14 +0200 Subject: [PATCH 01/13] shared: add nm_utils_gbytes_to_variant_ay() util --- shared/nm-utils/nm-shared-utils.c | 17 +++++++++++++++++ shared/nm-utils/nm-shared-utils.h | 4 ++++ 2 files changed, 21 insertions(+) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 96b5f2acab..43981e3177 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -117,6 +117,23 @@ nm_utils_strbuf_append (char **buf, gsize *len, const char *format, ...) /*****************************************************************************/ +GVariant * +nm_utils_gbytes_to_variant_ay (GBytes *bytes) +{ + const guint8 *p; + gsize l; + + if (!bytes) { + /* for convenience, accept NULL to return an empty variant */ + return g_variant_new_array (G_VARIANT_TYPE_BYTE, NULL, 0); + } + + p = g_bytes_get_data (bytes, &l); + return g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, p, l, 1); +} + +/*****************************************************************************/ + /** * nm_strquote: * @buf: the output buffer of where to write the quoted @str argument. diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 265d2ded36..0670c6c2f6 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -202,6 +202,10 @@ nm_utils_is_separator (const char c) /*****************************************************************************/ +GVariant *nm_utils_gbytes_to_variant_ay (GBytes *bytes); + +/*****************************************************************************/ + const char *nm_utils_dbus_path_get_last_component (const char *dbus_path); int nm_utils_dbus_path_cmp (const char *dbus_path_a, const char *dbus_path_b); From e730f7429dabd92920691bf210eca32323ebecab Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 13 Aug 2018 21:33:56 +0200 Subject: [PATCH 02/13] libnm: replace _nm_utils_bytes_to_dbus() with nm_utils_gbytes_get_variant_ay() --- libnm-core/nm-setting.c | 2 +- libnm-core/nm-utils-private.h | 1 - libnm-core/nm-utils.c | 17 ----------------- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index d59e3df79d..0cec516517 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -573,7 +573,7 @@ get_property_for_dbus (NMSetting *setting, else if (g_type_is_a (prop_value.g_type, G_TYPE_FLAGS)) dbus_value = g_variant_new_uint32 (g_value_get_flags (&prop_value)); else if (prop_value.g_type == G_TYPE_BYTES) - dbus_value = _nm_utils_bytes_to_dbus (&prop_value); + dbus_value = nm_utils_gbytes_to_variant_ay (g_value_get_boxed (&prop_value)); else dbus_value = g_dbus_gvalue_to_gvariant (&prop_value, variant_type_for_gtype (prop_value.g_type)); g_value_unset (&prop_value); diff --git a/libnm-core/nm-utils-private.h b/libnm-core/nm-utils-private.h index 0605d83fa1..b886730a35 100644 --- a/libnm-core/nm-utils-private.h +++ b/libnm-core/nm-utils-private.h @@ -74,7 +74,6 @@ GVariant * _nm_utils_strdict_to_dbus (const GValue *prop_value); void _nm_utils_strdict_from_dbus (GVariant *dbus_value, GValue *prop_value); -GVariant * _nm_utils_bytes_to_dbus (const GValue *prop_value); void _nm_utils_bytes_from_dbus (GVariant *dbus_value, GValue *prop_value); diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index eb002d4811..0acb96b281 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -647,23 +647,6 @@ _nm_utils_ptrarray_find_first (gconstpointer *list, gssize len, gconstpointer ne return -1; } -GVariant * -_nm_utils_bytes_to_dbus (const GValue *prop_value) -{ - GBytes *bytes = g_value_get_boxed (prop_value); - - if (bytes) { - return g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, - g_bytes_get_data (bytes, NULL), - g_bytes_get_size (bytes), - 1); - } else { - return g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, - NULL, 0, - 1); - } -} - void _nm_utils_bytes_from_dbus (GVariant *dbus_value, GValue *prop_value) From 39efc650964a3509dd8d45ce67ed171c0aa11ca7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 16:34:32 +0200 Subject: [PATCH 03/13] platform: drop unused virtual function NMPlatformClass.wifi_get_ssid() --- src/platform/nm-fake-platform.c | 7 ------- src/platform/nm-platform.h | 1 - 2 files changed, 8 deletions(-) diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index d8d192c712..82f9e2fb29 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -887,12 +887,6 @@ wifi_get_bssid (NMPlatform *platform, int ifindex, guint8 *bssid) return FALSE; } -static GByteArray * -wifi_get_ssid (NMPlatform *platform, int ifindex) -{ - return NULL; -} - static guint32 wifi_get_frequency (NMPlatform *platform, int ifindex) { @@ -1435,7 +1429,6 @@ nm_fake_platform_class_init (NMFakePlatformClass *klass) platform_class->wifi_get_capabilities = wifi_get_capabilities; platform_class->wifi_get_bssid = wifi_get_bssid; - platform_class->wifi_get_ssid = wifi_get_ssid; platform_class->wifi_get_frequency = wifi_get_frequency; platform_class->wifi_get_quality = wifi_get_quality; platform_class->wifi_get_rate = wifi_get_rate; diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 994e041719..a1171342f1 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -905,7 +905,6 @@ typedef struct { gboolean (*wifi_get_capabilities) (NMPlatform *, int ifindex, NMDeviceWifiCapabilities *caps); gboolean (*wifi_get_bssid) (NMPlatform *, int ifindex, guint8 *bssid); - GByteArray *(*wifi_get_ssid) (NMPlatform *, int ifindex); guint32 (*wifi_get_frequency) (NMPlatform *, int ifindex); int (*wifi_get_quality) (NMPlatform *, int ifindex); guint32 (*wifi_get_rate) (NMPlatform *, int ifindex); From 4607970288c22bebed86c287dfe1b658db9cec99 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 16:32:41 +0200 Subject: [PATCH 04/13] wifi/olpc: fix setting SSID for OLPC mesh in complete_connection() NM_SETTING_OLPC_MESH_SSID is of type GBytes, not GByteArray. --- src/devices/wifi/nm-device-olpc-mesh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index 7737f19a71..6f833635a1 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -95,7 +95,6 @@ complete_connection (NMDevice *device, GError **error) { NMSettingOlpcMesh *s_mesh; - GByteArray *tmp; s_mesh = nm_connection_get_setting_olpc_mesh (connection); if (!s_mesh) { @@ -104,10 +103,10 @@ complete_connection (NMDevice *device, } if (!nm_setting_olpc_mesh_get_ssid (s_mesh)) { - tmp = g_byte_array_sized_new (strlen (DEFAULT_SSID)); - g_byte_array_append (tmp, (const guint8 *) DEFAULT_SSID, strlen (DEFAULT_SSID)); - g_object_set (G_OBJECT (s_mesh), NM_SETTING_OLPC_MESH_SSID, tmp, NULL); - g_byte_array_free (tmp, TRUE); + gs_unref_bytes GBytes *ssid = NULL; + + ssid = g_bytes_new_static (DEFAULT_SSID, NM_STRLEN (DEFAULT_SSID)); + g_object_set (G_OBJECT (s_mesh), NM_SETTING_OLPC_MESH_SSID, ssid, NULL); } if (!nm_setting_olpc_mesh_get_dhcp_anycast_address (s_mesh)) { From dc316c8afe0fae6d85da76c43929cc6bb08e4ee9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 16:51:16 +0200 Subject: [PATCH 05/13] wifi: use GBytes instead of GBytesArray for tracking blobs in supplicant --- src/supplicant/nm-supplicant-config.c | 16 ++++------------ src/supplicant/nm-supplicant-interface.c | 5 ++--- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/supplicant/nm-supplicant-config.c b/src/supplicant/nm-supplicant-config.c index db2fe305ba..d090ffacbf 100644 --- a/src/supplicant/nm-supplicant-config.c +++ b/src/supplicant/nm-supplicant-config.c @@ -88,12 +88,6 @@ config_option_free (ConfigOption *opt) g_slice_free (ConfigOption, opt); } -static void -blob_free (GByteArray *array) -{ - g_byte_array_free (array, TRUE); -} - static void nm_supplicant_config_init (NMSupplicantConfig * self) { @@ -105,7 +99,7 @@ nm_supplicant_config_init (NMSupplicantConfig * self) priv->blobs = g_hash_table_new_full (nm_str_hash, g_str_equal, (GDestroyNotify) g_free, - (GDestroyNotify) blob_free); + (GDestroyNotify) g_bytes_unref); priv->ap_scan = 1; priv->dispose_has_run = FALSE; @@ -198,7 +192,6 @@ nm_supplicant_config_add_blob (NMSupplicantConfig *self, ConfigOption *old_opt; ConfigOption *opt; OptType type; - GByteArray *blob; const guint8 *data; gsize data_len; @@ -226,9 +219,6 @@ nm_supplicant_config_add_blob (NMSupplicantConfig *self, return FALSE; } - blob = g_byte_array_sized_new (data_len); - g_byte_array_append (blob, data, data_len); - opt = g_slice_new0 (ConfigOption); opt->value = g_strdup_printf ("blob://%s", blobid); opt->len = strlen (opt->value); @@ -237,7 +227,9 @@ nm_supplicant_config_add_blob (NMSupplicantConfig *self, nm_log_info (LOGD_SUPPLICANT, "Config: added '%s' value '%s'", key, opt->value); g_hash_table_insert (priv->config, g_strdup (key), opt); - g_hash_table_insert (priv->blobs, g_strdup (blobid), blob); + g_hash_table_insert (priv->blobs, + g_strdup (blobid), + g_bytes_ref (value)); return TRUE; } diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 71e6a35ac5..90f1b06198 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -1579,7 +1579,7 @@ assoc_add_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat GHashTable *blobs; GHashTableIter iter; const char *blob_name; - GByteArray *blob_data; + GBytes *blob_data; assoc_data = add_network_data->assoc_data; if (assoc_data) @@ -1637,8 +1637,7 @@ assoc_add_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat "AddBlob", g_variant_new ("(s@ay)", blob_name, - g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, - blob_data->data, blob_data->len, 1)), + nm_utils_gbytes_to_variant_ay (blob_data)), G_DBUS_CALL_FLAGS_NONE, -1, priv->assoc_data->cancellable, From dba19ebd7df9d5e4c65e0fbb2d9f33a28c884523 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 16:53:18 +0200 Subject: [PATCH 06/13] all: avoid useless cast of g_free() to GDestroyNotify --- libnm-core/nm-utils.c | 2 +- libnm-core/tests/test-setting.c | 4 ++-- libnm-util/nm-setting.c | 2 +- libnm-util/nm-utils.c | 2 +- src/supplicant/nm-supplicant-config.c | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 0acb96b281..606624e0e1 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -5654,7 +5654,7 @@ _nm_utils_team_config_get (const char *conf, g_ptr_array_free (data, TRUE); } else if (json_is_array (json_element)) { - GPtrArray *data = g_ptr_array_new_with_free_func ((GDestroyNotify) g_free); + GPtrArray *data = g_ptr_array_new_with_free_func (g_free); json_t *str_element; int index; diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c index 5526ad8a7e..18dadd886e 100644 --- a/libnm-core/tests/test-setting.c +++ b/libnm-core/tests/test-setting.c @@ -1002,7 +1002,7 @@ test_runner_loadbalance_sync_from_config (void) { gs_unref_ptrarray GPtrArray *tx_hash = NULL; - tx_hash = g_ptr_array_new_with_free_func ((GDestroyNotify) g_free); + tx_hash = g_ptr_array_new_with_free_func (g_free); g_ptr_array_add (tx_hash, g_strdup ("eth")); g_ptr_array_add (tx_hash, g_strdup ("ipv4")); g_ptr_array_add (tx_hash, g_strdup ("ipv6")); @@ -1039,7 +1039,7 @@ test_runner_lacp_sync_from_config (void) { gs_unref_ptrarray GPtrArray *tx_hash = NULL; - tx_hash = g_ptr_array_new_with_free_func ((GDestroyNotify) g_free); + tx_hash = g_ptr_array_new_with_free_func (g_free); g_ptr_array_add (tx_hash, g_strdup ("eth")); g_ptr_array_add (tx_hash, g_strdup ("ipv4")); g_ptr_array_add (tx_hash, g_strdup ("ipv6")); diff --git a/libnm-util/nm-setting.c b/libnm-util/nm-setting.c index 36d6dab1a3..9147dfe150 100644 --- a/libnm-util/nm-setting.c +++ b/libnm-util/nm-setting.c @@ -321,7 +321,7 @@ nm_setting_to_hash (NMSetting *setting, NMSettingHashFlags flags) property_specs = g_object_class_list_properties (G_OBJECT_GET_CLASS (setting), &n_property_specs); hash = g_hash_table_new_full (g_str_hash, g_str_equal, - (GDestroyNotify) g_free, destroy_gvalue); + g_free, destroy_gvalue); for (i = 0; i < n_property_specs; i++) { GParamSpec *prop_spec = property_specs[i]; diff --git a/libnm-util/nm-utils.c b/libnm-util/nm-utils.c index 26f5533a28..054d69f4c1 100644 --- a/libnm-util/nm-utils.c +++ b/libnm-util/nm-utils.c @@ -491,7 +491,7 @@ nm_utils_gvalue_hash_dup (GHashTable *hash) g_return_val_if_fail (hash != NULL, NULL); table = g_hash_table_new_full (g_str_hash, g_str_equal, - (GDestroyNotify) g_free, + g_free, value_destroy); g_hash_table_foreach (hash, value_dup, table); diff --git a/src/supplicant/nm-supplicant-config.c b/src/supplicant/nm-supplicant-config.c index d090ffacbf..a0628d74a7 100644 --- a/src/supplicant/nm-supplicant-config.c +++ b/src/supplicant/nm-supplicant-config.c @@ -94,11 +94,11 @@ nm_supplicant_config_init (NMSupplicantConfig * self) NMSupplicantConfigPrivate *priv = NM_SUPPLICANT_CONFIG_GET_PRIVATE (self); priv->config = g_hash_table_new_full (nm_str_hash, g_str_equal, - (GDestroyNotify) g_free, + g_free, (GDestroyNotify) config_option_free); priv->blobs = g_hash_table_new_full (nm_str_hash, g_str_equal, - (GDestroyNotify) g_free, + g_free, (GDestroyNotify) g_bytes_unref); priv->ap_scan = 1; From 5b5b651bcfd3446c5c4e91b475f707e529e0d966 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 16:39:38 +0200 Subject: [PATCH 07/13] dhcp/trivial: add fixme comments to nm_dhcp_dhclient_unescape_duid() --- src/dhcp/nm-dhcp-dhclient-utils.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index 16d60c35a2..d8f8dd98d8 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -530,6 +530,10 @@ nm_dhcp_dhclient_unescape_duid (const char *duid) guint i, len; guint8 octal; + /* FIXME: it's wrong to have an "unescape-duid" function. dhclient + * defines a file format with escaping. So we need a general unescape + * function that can handle dhclient syntax. */ + len = strlen (duid); unescaped = g_byte_array_sized_new (len); for (i = 0; i < len; i++) { @@ -543,6 +547,9 @@ nm_dhcp_dhclient_unescape_duid (const char *duid) g_byte_array_append (unescaped, &octal, 1); i += 2; } else { + /* FIXME: don't warn on untrusted data. Either signal an error, or accept + * it silently. */ + /* One of ", ', $, `, \, |, or & */ g_warn_if_fail (p[i] == '"' || p[i] == '\'' || p[i] == '$' || p[i] == '`' || p[i] == '\\' || p[i] == '|' || From f5792881a0a3bf6673ee20f59f4aff8dc693d5bf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 16:43:30 +0200 Subject: [PATCH 08/13] device: avoid intermediary GByteArray when creating DUID GBytes Creating it directly is simple enough. --- src/devices/nm-device.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 81cd57bd2e..9f425b5370 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8065,35 +8065,35 @@ static GBytes * generate_duid_llt (const guint8 *hwaddr /* ETH_ALEN bytes */, gint64 time) { - GByteArray *duid_arr; + guint8 *arr; const guint16 duid_type = htons (1); const guint16 hw_type = htons (ARPHRD_ETHER); const guint32 duid_time = htonl (NM_MAX (0, time - EPOCH_DATETIME_200001010000)); - duid_arr = g_byte_array_sized_new (2 + 4 + 2 + ETH_ALEN); + arr = g_new (guint8, 2 + 2 + 4 + ETH_ALEN); - g_byte_array_append (duid_arr, (const guint8 *) &duid_type, 2); - g_byte_array_append (duid_arr, (const guint8 *) &hw_type, 2); - g_byte_array_append (duid_arr, (const guint8 *) &duid_time, 4); - g_byte_array_append (duid_arr, hwaddr, ETH_ALEN); + memcpy (&arr[0], &duid_type, 2); + memcpy (&arr[2], &hw_type, 2); + memcpy (&arr[4], &duid_time, 4); + memcpy (&arr[8], hwaddr, ETH_ALEN); - return g_byte_array_free_to_bytes (duid_arr); + return g_bytes_new_take (arr, 2 + 2 + 4 + ETH_ALEN); } static GBytes * generate_duid_ll (const guint8 *hwaddr /* ETH_ALEN bytes */) { - GByteArray *duid_arr; + guint8 *arr; const guint16 duid_type = htons (3); const guint16 hw_type = htons (ARPHRD_ETHER); - duid_arr = g_byte_array_sized_new (2 + 2 + ETH_ALEN); + arr = g_new (guint8, 2 + 2 + ETH_ALEN); - g_byte_array_append (duid_arr, (const guint8 *) &duid_type, 2); - g_byte_array_append (duid_arr, (const guint8 *) &hw_type, 2); - g_byte_array_append (duid_arr, hwaddr, ETH_ALEN); + memcpy (&arr[0], &duid_type, 2); + memcpy (&arr[2], &hw_type, 2); + memcpy (&arr[4], hwaddr, ETH_ALEN); - return g_byte_array_free_to_bytes (duid_arr); + return g_bytes_new_take (arr, 2 + 2 + ETH_ALEN); } static GBytes * From ced0dd2e4a908c4c76c338b4575f8762f4a0f418 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 11:43:25 +0200 Subject: [PATCH 09/13] wifi: use GBytes for ssids scan list Use GBytes instead of GBytesArray. GBytes is immutable and can be shared. It is also the type that we natively get from nm_setting_wireless_get_ssid(). This way we avoid some conversions. --- src/devices/wifi/nm-device-wifi.c | 44 +++++++++--------------- src/supplicant/nm-supplicant-interface.c | 13 +++---- src/supplicant/nm-supplicant-interface.h | 4 ++- 3 files changed, 27 insertions(+), 34 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 17955f3937..4fc69061a4 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1083,7 +1083,6 @@ static GPtrArray * ssids_options_to_ptrarray (GVariant *value, GError **error) { GPtrArray *ssids = NULL; - GByteArray *ssid_array; GVariant *v; const guint8 *bytes; gsize len; @@ -1099,7 +1098,7 @@ ssids_options_to_ptrarray (GVariant *value, GError **error) } if (num_ssids) { - ssids = g_ptr_array_new_full (num_ssids, (GDestroyNotify) g_byte_array_unref); + ssids = g_ptr_array_new_full (num_ssids, (GDestroyNotify) g_bytes_unref); for (i = 0; i < num_ssids; i++) { v = g_variant_get_child_value (value, i); bytes = g_variant_get_fixed_array (v, &len, sizeof (guint8)); @@ -1112,9 +1111,7 @@ ssids_options_to_ptrarray (GVariant *value, GError **error) return NULL; } - ssid_array = g_byte_array_new (); - g_byte_array_append (ssid_array, bytes, len); - g_ptr_array_add (ssids, ssid_array); + g_ptr_array_add (ssids, g_bytes_new (bytes, len)); } } return ssids; @@ -1305,7 +1302,7 @@ build_hidden_probe_list (NMDeviceWifi *self) gs_free NMSettingsConnection **connections = NULL; guint i, len; GPtrArray *ssids = NULL; - static GByteArray *nullssid = NULL; + static GBytes *nullssid = NULL; /* Need at least two: wildcard SSID and one or more hidden SSIDs */ if (max_scan_ssids < 2) @@ -1320,30 +1317,23 @@ build_hidden_probe_list (NMDeviceWifi *self) g_qsort_with_data (connections, len, sizeof (NMSettingsConnection *), nm_settings_connection_cmp_timestamp_p_with_data, NULL); - ssids = g_ptr_array_new_full (max_scan_ssids, (GDestroyNotify) g_byte_array_unref); + ssids = g_ptr_array_new_full (max_scan_ssids, (GDestroyNotify) g_bytes_unref); /* Add wildcard SSID using a static wildcard SSID used for every scan */ if (G_UNLIKELY (nullssid == NULL)) - nullssid = g_byte_array_new (); - g_ptr_array_add (ssids, g_byte_array_ref (nullssid)); + nullssid = g_bytes_new_static ("", 0); + g_ptr_array_add (ssids, g_bytes_ref (nullssid)); for (i = 0; connections[i]; i++) { NMSettingWireless *s_wifi; GBytes *ssid; - GByteArray *ssid_array; if (i >= max_scan_ssids - 1) break; s_wifi = (NMSettingWireless *) nm_connection_get_setting_wireless (NM_CONNECTION (connections[i])); - g_assert (s_wifi); ssid = nm_setting_wireless_get_ssid (s_wifi); - g_assert (ssid); - ssid_array = g_byte_array_new (); - g_byte_array_append (ssid_array, - g_bytes_get_data (ssid, NULL), - g_bytes_get_size (ssid)); - g_ptr_array_add (ssids, ssid_array); + g_ptr_array_add (ssids, g_bytes_ref (ssid)); } return ssids; @@ -1370,24 +1360,22 @@ request_wireless_scan (NMDeviceWifi *self, _LOGD (LOGD_WIFI, "wifi-scan: scanning requested"); - if (!ssids) { + if (!ssids) ssids = hidden_ssids = build_hidden_probe_list (self); - } if (_LOGD_ENABLED (LOGD_WIFI)) { if (ssids) { - const GByteArray *ssid; guint i; - char *foo; for (i = 0; i < ssids->len; i++) { - ssid = g_ptr_array_index (ssids, i); - foo = ssid->len > 0 - ? nm_utils_ssid_to_utf8 (ssid->data, ssid->len) - : NULL; + gs_free char *foo = NULL; + const guint8 *p; + gsize l; + + p = g_bytes_get_data (ssids->pdata[i], &l); + foo = l > 0 ? nm_utils_ssid_to_utf8 (p, l) : NULL; _LOGD (LOGD_WIFI, "wifi-scan: (%u) probe scanning SSID %s%s%s", i, NM_PRINT_FMT_QUOTED (foo, "\"", foo, "\"", "*any*")); - g_free (foo); } } else _LOGD (LOGD_WIFI, "wifi-scan: no SSIDs to probe scan"); @@ -1395,7 +1383,9 @@ request_wireless_scan (NMDeviceWifi *self, _hw_addr_set_scanning (self, FALSE); - nm_supplicant_interface_request_scan (priv->sup_iface, ssids); + nm_supplicant_interface_request_scan (priv->sup_iface, + ssids ? (GBytes *const*) ssids->pdata : NULL, + ssids ? ssids->len : 0u); request_started = TRUE; } else _LOGD (LOGD_WIFI, "wifi-scan: scanning requested but not allowed at this time"); diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 90f1b06198..dca600857d 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -1789,7 +1789,9 @@ scan_request_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) } void -nm_supplicant_interface_request_scan (NMSupplicantInterface *self, const GPtrArray *ssids) +nm_supplicant_interface_request_scan (NMSupplicantInterface *self, + GBytes *const*ssids, + guint ssids_len) { NMSupplicantInterfacePrivate *priv; GVariantBuilder builder; @@ -1803,15 +1805,14 @@ nm_supplicant_interface_request_scan (NMSupplicantInterface *self, const GPtrArr g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); g_variant_builder_add (&builder, "{sv}", "Type", g_variant_new_string ("active")); g_variant_builder_add (&builder, "{sv}", "AllowRoam", g_variant_new_boolean (FALSE)); - if (ssids) { + if (ssids_len > 0) { GVariantBuilder ssids_builder; g_variant_builder_init (&ssids_builder, G_VARIANT_TYPE_BYTESTRING_ARRAY); - for (i = 0; i < ssids->len; i++) { - GByteArray *ssid = g_ptr_array_index (ssids, i); + for (i = 0; i < ssids_len; i++) { + nm_assert (ssids[i]); g_variant_builder_add (&ssids_builder, "@ay", - g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, - ssid->data, ssid->len, 1)); + nm_utils_gbytes_to_variant_ay (ssids[i])); } g_variant_builder_add (&builder, "{sv}", "SSIDs", g_variant_builder_end (&ssids_builder)); } diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index 31272b3c11..0365fdcd63 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -100,7 +100,9 @@ void nm_supplicant_interface_disconnect (NMSupplicantInterface * iface); const char *nm_supplicant_interface_get_object_path (NMSupplicantInterface * iface); -void nm_supplicant_interface_request_scan (NMSupplicantInterface * self, const GPtrArray *ssids); +void nm_supplicant_interface_request_scan (NMSupplicantInterface *self, + GBytes *const*ssids, + guint ssids_len); NMSupplicantInterfaceState nm_supplicant_interface_get_state (NMSupplicantInterface * self); From 57c371e32f20934affd423cbd76c6e9e05d3019c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 11:43:25 +0200 Subject: [PATCH 10/13] shared: add nm_utils_buf_utf8safe_escape() util We already have nm_utils_str_utf8safe_escape() to convert a NUL termianted string to an UTF-8 string. nm_utils_str_utf8safe_escape() operates under the assumption, that the input strig is already valid UTF-8 and returns the input string verbatim. That way, in the common expected cases, the string just looks like a regular UTF-8 string. However, in case there are invalid UTF-8 sequences (or a backslash escape characters), the function will use backslash escaping to encode the input string as a valid UTF-8 sequence. Note that the escaped sequence, can be reverted to the original non-UTF-8 string via unescape. An example, where this is useful are file names or interface names. Which are not in a defined encoding, but NUL terminated and commonly ASCII or UTF-8 encoded. Extend this, to also handle not NUL terminated buffers. The same applies, except that the process cannot be reverted via g_strcompress() -- because the NUL character cannot be unescaped. This will be useful to escape a Wi-Fi SSID. Commonly we expect the SSID to be in UTF-8/ASCII encoding and we want to print it verbatim. Only if that is not the case, we fallback to backslash escaping. However, the orginal value can be fully recovered via unescape(). The difference between an SSID and a filename is, that the former can contain '\0' bytes. --- libnm-core/tests/test-general.c | 181 ++++++++++++++----- shared/nm-utils/nm-shared-utils.c | 283 ++++++++++++++++++++++++------ shared/nm-utils/nm-shared-utils.h | 4 + 3 files changed, 370 insertions(+), 98 deletions(-) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index ad40ee947f..31dd704189 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -6559,74 +6559,161 @@ test_nm_utils_enum (void) /*****************************************************************************/ static void -do_test_utils_str_utf8safe (const char *str, const char *expected, NMUtilsStrUtf8SafeFlags flags) +_do_test_utils_str_utf8safe_unescape (const char *str, const char *expected, gsize expected_len) { - const char *str_safe, *s; - gs_free char *str2 = NULL; - gs_free char *str3 = NULL; + gsize l; + const char *s; + gs_free gpointer buf_free_1 = NULL; + gs_free char *str_free_1 = NULL; - str_safe = nm_utils_str_utf8safe_escape (str, flags, &str2); + s = nm_utils_buf_utf8safe_unescape (str, &l, &buf_free_1); + g_assert_cmpint (expected_len, ==, l); + g_assert_cmpstr (s, ==, expected); - str3 = nm_utils_str_utf8safe_escape_cp (str, flags); - g_assert_cmpstr (str3, ==, str_safe); - g_assert ((!str && !str3) || (str != str3)); - g_clear_pointer (&str3, g_free); + if (str == NULL) { + g_assert (!s); + g_assert (!buf_free_1); + g_assert_cmpint (l, ==, 0); + } else { + g_assert (s); + if (!strchr (str, '\\')) { + g_assert (!buf_free_1); + g_assert (s == str); + g_assert_cmpint (l, ==, strlen (str)); + } else { + g_assert (buf_free_1); + g_assert (s == buf_free_1); + g_assert (memcmp (s, expected, expected_len) == 0); + } + } + + if ( expected + && l == strlen (expected)) { + /* there are no embeeded NULs. Check that nm_utils_str_utf8safe_unescape() yields the same result. */ + s = nm_utils_str_utf8safe_unescape (str, &str_free_1); + g_assert_cmpstr (s, ==, expected); + if (strchr (str, '\\')) { + g_assert (str_free_1 != str); + g_assert (s == str_free_1); + } else + g_assert (s == str); + } +} + +#define do_test_utils_str_utf8safe_unescape(str, expected) \ + _do_test_utils_str_utf8safe_unescape (""str"", expected, NM_STRLEN (expected)) + +static void +_do_test_utils_str_utf8safe (const char *str, gsize str_len, const char *expected, NMUtilsStrUtf8SafeFlags flags) +{ + const char *str_safe; + const char *buf_safe; + const char *s; + gs_free gpointer buf_free_1 = NULL; + gs_free char *str_free_1 = NULL; + gs_free char *str_free_2 = NULL; + gs_free char *str_free_3 = NULL; + gs_free char *str_free_4 = NULL; + gs_free char *str_free_5 = NULL; + gs_free char *str_free_6 = NULL; + gs_free char *str_free_7 = NULL; + gs_free char *str_free_8 = NULL; + gboolean str_has_nul = FALSE; + + buf_safe = nm_utils_buf_utf8safe_escape (str, str_len, flags, &str_free_1); + + str_safe = nm_utils_str_utf8safe_escape (str, flags, &str_free_2); + + if (str_len == 0) { + g_assert (buf_safe == NULL); + g_assert (str_free_1 == NULL); + g_assert (str_safe == str); + g_assert (str == NULL || str[0] == '\0'); + g_assert (str_free_2 == NULL); + } else if (str_len == strlen (str)) { + g_assert (buf_safe); + g_assert_cmpstr (buf_safe, ==, str_safe); + + /* nm_utils_buf_utf8safe_escape() can only return a pointer equal to the input string, + * if and only if str_len is negative. Otherwise, the input str won't be NUL terminated + * and cannot be returned. */ + g_assert (buf_safe != str); + g_assert (buf_safe == str_free_1); + } else + str_has_nul = TRUE; + + str_free_3 = nm_utils_str_utf8safe_escape_cp (str, flags); + g_assert_cmpstr (str_free_3, ==, str_safe); + g_assert ((!str && !str_free_3) || (str != str_free_3)); + + if (str_len > 0) + _do_test_utils_str_utf8safe_unescape (buf_safe, str, str_len); if (expected == NULL) { + g_assert (!str_has_nul); + g_assert (str_safe == str); - g_assert (!str2); + g_assert (!str_free_2); if (str) { g_assert (!strchr (str, '\\')); g_assert (g_utf8_validate (str, -1, NULL)); } - g_assert (str == nm_utils_str_utf8safe_unescape (str_safe, &str3)); - g_assert (!str3); + g_assert (str == nm_utils_str_utf8safe_unescape (str_safe, &str_free_4)); + g_assert (!str_free_4); - str3 = nm_utils_str_utf8safe_unescape_cp (str_safe); + str_free_5 = nm_utils_str_utf8safe_unescape_cp (str_safe); if (str) { - g_assert (str3 != str); - g_assert_cmpstr (str3, ==, str); + g_assert (str_free_5 != str); + g_assert_cmpstr (str_free_5, ==, str); } else - g_assert (!str3); - g_clear_pointer (&str3, g_free); + g_assert (!str_free_5); return; } - g_assert (str); - g_assert (str_safe != str); - g_assert (str_safe == str2); - g_assert ( strchr (str, '\\') - || !g_utf8_validate (str, -1, NULL) - || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) - && NM_STRCHAR_ANY (str, ch, (guchar) ch >= 127)) - || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) - && NM_STRCHAR_ANY (str, ch, (guchar) ch < ' '))); - g_assert (g_utf8_validate (str_safe, -1, NULL)); + if (!str_has_nul) { + g_assert (str); + g_assert (str_safe != str); + g_assert (str_safe == str_free_2); + g_assert ( strchr (str, '\\') + || !g_utf8_validate (str, -1, NULL) + || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) + && NM_STRCHAR_ANY (str, ch, (guchar) ch >= 127)) + || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) + && NM_STRCHAR_ANY (str, ch, (guchar) ch < ' '))); + g_assert (g_utf8_validate (str_safe, -1, NULL)); - str3 = g_strcompress (str_safe); - g_assert_cmpstr (str, ==, str3); - g_clear_pointer (&str3, g_free); + str_free_6 = g_strcompress (str_safe); + g_assert_cmpstr (str, ==, str_free_6); - str3 = nm_utils_str_utf8safe_unescape_cp (str_safe); - g_assert (str3 != str); - g_assert_cmpstr (str3, ==, str); - g_clear_pointer (&str3, g_free); + str_free_7 = nm_utils_str_utf8safe_unescape_cp (str_safe); + g_assert (str_free_7 != str); + g_assert_cmpstr (str_free_7, ==, str); - s = nm_utils_str_utf8safe_unescape (str_safe, &str3); - g_assert (str3 != str); - g_assert (s == str3); - g_assert_cmpstr (str3, ==, str); - g_clear_pointer (&str3, g_free); + s = nm_utils_str_utf8safe_unescape (str_safe, &str_free_8); + g_assert (str_free_8 != str); + g_assert (s == str_free_8); + g_assert_cmpstr (str_free_8, ==, str); + + g_assert_cmpstr (str_safe, ==, expected); + + return; + } + + g_assert_cmpstr (buf_safe, ==, expected); - g_assert_cmpstr (str_safe, ==, expected); } +#define do_test_utils_str_utf8safe(str, expected, flags) \ + _do_test_utils_str_utf8safe (""str"", NM_STRLEN (str), expected, flags) static void test_utils_str_utf8safe (void) { - do_test_utils_str_utf8safe (NULL, NULL, NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + _do_test_utils_str_utf8safe (NULL, 0, NULL, NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + do_test_utils_str_utf8safe ("", NULL, NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + do_test_utils_str_utf8safe ("\\", "\\\\", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + do_test_utils_str_utf8safe ("\\a", "\\\\a", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); do_test_utils_str_utf8safe ("\314", "\\314", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); do_test_utils_str_utf8safe ("\314\315x\315\315x", "\\314\\315x\\315\\315x", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); do_test_utils_str_utf8safe ("\314\315xx", "\\314\\315xx", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); @@ -6648,6 +6735,18 @@ test_utils_str_utf8safe (void) do_test_utils_str_utf8safe ("㈞abä㈞b", NULL, NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); do_test_utils_str_utf8safe ("abäb", "ab\\303\\244b", NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII); do_test_utils_str_utf8safe ("ab\ab", "ab\\007b", NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL); + + do_test_utils_str_utf8safe ("\0", "\\000", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + do_test_utils_str_utf8safe ("\0a\0", "\\000a\\000", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + do_test_utils_str_utf8safe ("\\\0", "\\\\\\000", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + do_test_utils_str_utf8safe ("\n\0", "\n\\000", NM_UTILS_STR_UTF8_SAFE_FLAG_NONE); + do_test_utils_str_utf8safe ("\n\0", "\\012\\000", NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL); + + do_test_utils_str_utf8safe_unescape ("\n\\0", "\n\0"); + do_test_utils_str_utf8safe_unescape ("\n\\01", "\n\01"); + do_test_utils_str_utf8safe_unescape ("\n\\012", "\n\012"); + do_test_utils_str_utf8safe_unescape ("\n\\.", "\n."); + do_test_utils_str_utf8safe_unescape ("\\n\\.3\\r", "\n.3\r"); } /*****************************************************************************/ diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 43981e3177..25661eaa7c 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -1077,6 +1077,231 @@ _str_append_escape (GString *s, char ch) g_string_append_c (s, '0' + ( ((guchar) ch) & 07)); } +gconstpointer +nm_utils_buf_utf8safe_unescape (const char *str, gsize *out_len, gpointer *to_free) +{ + GString *gstr; + gsize len; + const char *s; + + g_return_val_if_fail (to_free, NULL); + g_return_val_if_fail (out_len, NULL); + + if (!str) { + *out_len = 0; + *to_free = NULL; + return NULL; + } + + len = strlen (str); + + s = memchr (str, '\\', len); + if (!s) { + *out_len = len; + *to_free = NULL; + return str; + } + + gstr = g_string_new_len (NULL, len); + + g_string_append_len (gstr, str, s - str); + str = s; + + for (;;) { + char ch; + guint v; + + nm_assert (str[0] == '\\'); + + ch = (++str)[0]; + + if (ch == '\0') { + // error. Trailing '\\' + break; + } + + if (ch >= '0' && ch <= '9') { + v = ch - '0'; + ch = (++str)[0]; + if (ch >= '0' && ch <= '7') { + v = v * 8 + (ch - '0'); + ch = (++str)[0]; + if (ch >= '0' && ch <= '7') { + v = v * 8 + (ch - '0'); + ch = (++str)[0]; + } + } + ch = v; + } else { + switch (ch) { + case 'b': ch = '\b'; break; + case 'f': ch = '\f'; break; + case 'n': ch = '\n'; break; + case 'r': ch = '\r'; break; + case 't': ch = '\t'; break; + case 'v': ch = '\v'; break; + default: + /* Here we handle "\\\\", but all other unexpected escape sequences are really a bug. + * Take them literally, after removing the escape character */ + break; + } + str++; + } + + g_string_append_c (gstr, ch); + + s = strchr (str, '\\'); + if (!s) { + g_string_append (gstr, str); + break; + } + + g_string_append_len (gstr, str, s - str); + str = s; + } + + *out_len = gstr->len; + *to_free = gstr->str; + return g_string_free (gstr, FALSE); +} + +/** + * nm_utils_buf_utf8safe_escape: + * @buf: byte array, possibly in utf-8 encoding, may have NUL characters. + * @buflen: the length of @buf in bytes, or -1 if @buf is a NUL terminated + * string. + * @flags: #NMUtilsStrUtf8SafeFlags flags + * @to_free: (out): return the pointer location of the string + * if a copying was necessary. + * + * Based on the assumption, that @buf contains UTF-8 encoded bytes, + * this will return valid UTF-8 sequence, and invalid sequences + * will be escaped with backslash (C escaping, like g_strescape()). + * This is sanitize non UTF-8 characters. The result is valid + * UTF-8. + * + * The operation can be reverted with nm_utils_buf_utf8safe_unescape(). + * Note that if, and only if @buf contains no NUL bytes, the operation + * can also be reverted with g_strcompress(). + * + * Depending on @flags, valid UTF-8 characters are not escaped at all + * (except the escape character '\\'). This is the difference to g_strescape(), + * which escapes all non-ASCII characters. This allows to pass on + * valid UTF-8 characters as-is and can be directly shown to the user + * as UTF-8 -- with exception of the backslash escape character, + * invalid UTF-8 sequences, and other (depending on @flags). + * + * Returns: the escaped input buffer, as valid UTF-8. If no escaping + * is necessary, it returns the input @buf. Otherwise, an allocated + * string @to_free is returned which must be freed by the caller + * with g_free. The escaping can be reverted by g_strcompress(). + **/ +const char * +nm_utils_buf_utf8safe_escape (gconstpointer buf, gssize buflen, NMUtilsStrUtf8SafeFlags flags, char **to_free) +{ + const char *const str = buf; + const char *p = NULL; + const char *s; + gboolean nul_terminated = FALSE; + GString *gstr; + + g_return_val_if_fail (to_free, NULL); + + *to_free = NULL; + + if (buflen == 0) + return NULL; + + if (buflen < 0) { + if (!str) + return NULL; + buflen = strlen (str); + if (buflen == 0) + return str; + nul_terminated = TRUE; + } + + if ( g_utf8_validate (str, buflen, &p) + && nul_terminated) { + /* note that g_utf8_validate() does not allow NUL character inside @str. Good. + * We can treat @str like a NUL terminated string. */ + if (!NM_STRCHAR_ANY (str, ch, + ( ch == '\\' \ + || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) \ + && ch < ' ') \ + || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) \ + && ((guchar) ch) >= 127)))) + return str; + } + + gstr = g_string_sized_new (buflen + 5); + + s = str; + do { + buflen -= p - s; + nm_assert (buflen >= 0); + + for (; s < p; s++) { + char ch = s[0]; + + if (ch == '\\') + g_string_append (gstr, "\\\\"); + else if ( ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) \ + && ch < ' ') \ + || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) \ + && ((guchar) ch) >= 127)) + _str_append_escape (gstr, ch); + else + g_string_append_c (gstr, ch); + } + + if (buflen <= 0) + break; + + _str_append_escape (gstr, p[0]); + + buflen--; + if (buflen == 0) + break; + + s = &p[1]; + g_utf8_validate (s, buflen, &p); + } while (TRUE); + + *to_free = g_string_free (gstr, FALSE); + return *to_free; +} + +const char * +nm_utils_buf_utf8safe_escape_bytes (GBytes *bytes, NMUtilsStrUtf8SafeFlags flags, char **to_free) +{ + gconstpointer p; + gsize l; + + if (bytes) + p = g_bytes_get_data (bytes, &l); + else { + p = NULL; + l = 0; + } + + return nm_utils_buf_utf8safe_escape (p, l, flags, to_free); +} + +/*****************************************************************************/ + +const char * +nm_utils_str_utf8safe_unescape (const char *str, char **to_free) +{ + g_return_val_if_fail (to_free, NULL); + + if (!str || !strchr (str, '\\')) { + *to_free = NULL; + return str; + } + return (*to_free = g_strcompress (str)); +} + /** * nm_utils_str_utf8safe_escape: * @str: NUL terminated input string, possibly in utf-8 encoding @@ -1107,63 +1332,7 @@ _str_append_escape (GString *s, char ch) const char * nm_utils_str_utf8safe_escape (const char *str, NMUtilsStrUtf8SafeFlags flags, char **to_free) { - const char *p = NULL; - GString *s; - - g_return_val_if_fail (to_free, NULL); - - *to_free = NULL; - if (!str || !str[0]) - return str; - - if ( g_utf8_validate (str, -1, &p) - && !NM_STRCHAR_ANY (str, ch, - ( ch == '\\' \ - || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) \ - && ch < ' ') \ - || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) \ - && ((guchar) ch) >= 127)))) - return str; - - s = g_string_sized_new ((p - str) + strlen (p) + 5); - - do { - for (; str < p; str++) { - char ch = str[0]; - - if (ch == '\\') - g_string_append (s, "\\\\"); - else if ( ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) \ - && ch < ' ') \ - || ( NM_FLAGS_HAS (flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) \ - && ((guchar) ch) >= 127)) - _str_append_escape (s, ch); - else - g_string_append_c (s, ch); - } - - if (p[0] == '\0') - break; - _str_append_escape (s, p[0]); - - str = &p[1]; - g_utf8_validate (str, -1, &p); - } while (TRUE); - - *to_free = g_string_free (s, FALSE); - return *to_free; -} - -const char * -nm_utils_str_utf8safe_unescape (const char *str, char **to_free) -{ - g_return_val_if_fail (to_free, NULL); - - if (!str || !strchr (str, '\\')) { - *to_free = NULL; - return str; - } - return (*to_free = g_strcompress (str)); + return nm_utils_buf_utf8safe_escape (str, -1, flags, to_free); } /** diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 0670c6c2f6..a85497e854 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -472,6 +472,10 @@ typedef enum { NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII = 0x0002, } NMUtilsStrUtf8SafeFlags; +const char *nm_utils_buf_utf8safe_escape (gconstpointer buf, gssize buflen, NMUtilsStrUtf8SafeFlags flags, char **to_free); +const char *nm_utils_buf_utf8safe_escape_bytes (GBytes *bytes, NMUtilsStrUtf8SafeFlags flags, char **to_free); +gconstpointer nm_utils_buf_utf8safe_unescape (const char *str, gsize *out_len, gpointer *to_free); + const char *nm_utils_str_utf8safe_escape (const char *str, NMUtilsStrUtf8SafeFlags flags, char **to_free); const char *nm_utils_str_utf8safe_unescape (const char *str, char **to_free); From 331d44afa69f808a32f5fb2e3ea43fc11da862b1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 15:08:45 +0200 Subject: [PATCH 11/13] wifi: don't ignore trailing NUL byte when comparing SSID nm_utils_same_ssid() has a comment * Earlier versions of the Linux kernel added a NULL byte to the end of the * SSID to enable easy printing of the SSID on the console or in a terminal, * but this behavior was problematic (SSIDs are simply byte arrays, not strings) * and thus was changed. This function compensates for that behavior at the * cost of some compatibility with odd SSIDs that may legitimately have trailing * NULLs, even though that is functionally pointless. and the functionality was introduced by commit ccb13f0bdd0c8ac3ee85dd0a6064c9bc545585f1. There was only place left that calls nm_utils_same_ssid(). I really don't think this is the right approach, nor is it clear that this is still necessary. Also, it seems to only matter with WEXT, and we should not have such an ugly hack in all cases. --- src/devices/wifi/nm-wifi-ap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index d299d10234..f40f7191e3 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -1039,7 +1039,7 @@ nm_wifi_ap_check_compatible (NMWifiAP *self, if ( ssid && priv->ssid && !nm_utils_same_ssid (g_bytes_get_data (ssid, NULL), g_bytes_get_size (ssid), priv->ssid->data, priv->ssid->len, - TRUE)) + FALSE)) return FALSE; bssid = nm_setting_wireless_get_bssid (s_wireless); From 5cd4e6f3e68bc4564656f0bb00b5261a8f6adeb7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 12:15:38 +0200 Subject: [PATCH 12/13] wifi: don't use GBytesArray for NMWifiAP's ssid GBytes makes more sense, because it's immutable. Also, since at other places we use GBytes, having different types is combersome and requires needless conversions. Also: - avoid nm_utils_escape_ssid() instead of _nm_utils_ssid_to_string(). We use nm_utils_escape_ssid() when we want to log the SSID. However, it does not escape newlines, which is bad. - also no longer use nm_utils_same_ssid(). Since it no longer treated trailing NUL special, it is not different from g_bytes_equal(). - also, don't use nm_utils_ssid_to_utf8() for logging anymore. For logging, _nm_utils_ssid_escape_utf8safe() is better because it is loss-less escaping which can be unambigously reverted. --- libnm-core/nm-core-internal.h | 7 ++ libnm-core/nm-utils.c | 55 +++++++++++++ src/devices/wifi/nm-device-iwd.c | 40 ++++------ src/devices/wifi/nm-device-wifi.c | 80 +++++++++---------- src/devices/wifi/nm-wifi-ap.c | 67 +++++++--------- src/devices/wifi/nm-wifi-ap.h | 2 +- src/devices/wifi/nm-wifi-utils.c | 21 ++--- src/devices/wifi/nm-wifi-utils.h | 4 +- src/devices/wifi/tests/test-general.c | 26 +++--- src/platform/wifi/nm-wifi-utils-wext.c | 5 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 20 +++-- 11 files changed, 177 insertions(+), 150 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 337018485d..23e5dd71d8 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -328,6 +328,13 @@ gboolean _nm_dbus_error_has_name (GError *error, /*****************************************************************************/ +char *_nm_utils_ssid_to_string_arr (const guint8 *ssid, gsize len); +char *_nm_utils_ssid_to_string (GBytes *ssid); +char *_nm_utils_ssid_to_utf8 (GBytes *ssid); +gboolean _nm_utils_is_empty_ssid (GBytes *ssid); + +/*****************************************************************************/ + gboolean _nm_vpn_plugin_info_check_file (const char *filename, gboolean check_absolute, gboolean do_validate_filename, diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 606624e0e1..9565c0af39 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -336,6 +336,18 @@ nm_utils_ssid_to_utf8 (const guint8 *ssid, gsize len) return converted; } +char * +_nm_utils_ssid_to_utf8 (GBytes *ssid) +{ + const guint8 *p; + gsize l; + + g_return_val_if_fail (ssid, NULL); + + p = g_bytes_get_data (ssid, &l); + return nm_utils_ssid_to_utf8 (p, l); +} + /* Shamelessly ripped from the Linux kernel ieee80211 stack */ /** * nm_utils_is_empty_ssid: @@ -363,6 +375,18 @@ nm_utils_is_empty_ssid (const guint8 *ssid, gsize len) return TRUE; } +gboolean +_nm_utils_is_empty_ssid (GBytes *ssid) +{ + const guint8 *p; + gsize l; + + g_return_val_if_fail (ssid, FALSE); + + p = g_bytes_get_data (ssid, &l); + return nm_utils_is_empty_ssid (p, l); +} + #define ESSID_MAX_SIZE 32 /** @@ -404,6 +428,37 @@ nm_utils_escape_ssid (const guint8 *ssid, gsize len) return escaped; } +char * +_nm_utils_ssid_to_string_arr (const guint8 *ssid, gsize len) +{ + char *s_copy; + const char *s_cnst; + + if (len == 0) + return g_strdup ("(empty)"); + + s_cnst = nm_utils_buf_utf8safe_escape (ssid, len, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL, &s_copy); + nm_assert (s_cnst); + + if (nm_utils_is_empty_ssid (ssid, len)) + return g_strdup_printf ("\"%s\" (hidden)", s_cnst); + + return g_strdup_printf ("\"%s\"", s_cnst); +} + +char * +_nm_utils_ssid_to_string (GBytes *ssid) +{ + gconstpointer p; + gsize l; + + if (!ssid) + return g_strdup ("(none)"); + + p = g_bytes_get_data (ssid, &l); + return _nm_utils_ssid_to_string_arr (p, l); +} + /** * nm_utils_same_ssid: * @ssid1: (array length=len1): the first SSID to compare diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 4fbf0e5b39..84ee421740 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -473,7 +473,7 @@ is_connection_known_network (NMConnection *connection) { NMSettingWireless *s_wireless; GBytes *ssid; - gs_free char *str_ssid = NULL; + gs_free char *ssid_utf8 = NULL; s_wireless = nm_connection_get_setting_wireless (connection); if (!s_wireless) @@ -483,11 +483,9 @@ is_connection_known_network (NMConnection *connection) if (!ssid) return FALSE; - str_ssid = nm_utils_ssid_to_utf8 (g_bytes_get_data (ssid, NULL), - g_bytes_get_size (ssid)); - + ssid_utf8 = _nm_utils_ssid_to_utf8 (ssid); return nm_iwd_manager_is_known_network (nm_iwd_manager_get (), - str_ssid, + ssid_utf8, get_connection_iwd_security (connection)); } @@ -637,10 +635,9 @@ complete_connection (NMDevice *device, NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); NMSettingWireless *s_wifi; const char *setting_mac; - char *str_ssid = NULL; + gs_free char *ssid_utf8 = NULL; NMWifiAP *ap; - const GByteArray *ssid = NULL; - GByteArray *tmp_ssid = NULL; + GBytes *ssid; GBytes *setting_ssid = NULL; const char *perm_hw_addr; const char *mode; @@ -704,8 +701,7 @@ complete_connection (NMDevice *device, } ssid = nm_wifi_ap_get_ssid (ap); - - if (ssid == NULL) { + if (!ssid) { g_set_error_literal (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INVALID_CONNECTION, @@ -716,25 +712,18 @@ complete_connection (NMDevice *device, if (!nm_wifi_ap_complete_connection (ap, connection, nm_wifi_utils_is_manf_default_ssid (ssid), - error)) { - if (tmp_ssid) - g_byte_array_unref (tmp_ssid); + error)) return FALSE; - } - - str_ssid = nm_utils_ssid_to_utf8 (ssid->data, ssid->len); + ssid_utf8 = _nm_utils_ssid_to_utf8 (ssid); nm_utils_complete_generic (nm_device_get_platform (device), connection, NM_SETTING_WIRELESS_SETTING_NAME, existing_connections, - str_ssid, - str_ssid, + ssid_utf8, + ssid_utf8, NULL, TRUE); - g_free (str_ssid); - if (tmp_ssid) - g_byte_array_unref (tmp_ssid); /* 8021x networks can only be used if they've been provisioned on the IWD side and * thus are Known Networks. @@ -1251,7 +1240,7 @@ network_connect_cb (GObject *source, GAsyncResult *res, gpointer user_data) NMConnection *connection; NMSettingWireless *s_wifi; GBytes *ssid; - gs_free char *str_ssid = NULL; + gs_free char *ssid_utf8 = NULL; if (!_nm_dbus_proxy_call_finish (G_DBUS_PROXY (source), res, G_VARIANT_TYPE ("()"), @@ -1303,15 +1292,14 @@ network_connect_cb (GObject *source, GAsyncResult *res, gpointer user_data) if (!ssid) goto failed; - str_ssid = nm_utils_ssid_to_utf8 (g_bytes_get_data (ssid, NULL), - g_bytes_get_size (ssid)); + ssid_utf8 = _nm_utils_ssid_to_utf8 (ssid); _LOGI (LOGD_DEVICE | LOGD_WIFI, "Activation: (wifi) Stage 2 of 5 (Device Configure) successful. Connected to '%s'.", - str_ssid); + ssid_utf8); nm_device_activate_schedule_stage3_ip_config_start (device); - nm_iwd_manager_network_connected (nm_iwd_manager_get (), str_ssid, + nm_iwd_manager_network_connected (nm_iwd_manager_get (), ssid_utf8, get_connection_iwd_security (connection)); return; diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 4fc69061a4..2245fccf08 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -761,10 +761,9 @@ complete_connection (NMDevice *device, NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); NMSettingWireless *s_wifi; const char *setting_mac; - char *str_ssid = NULL; + gs_free char *ssid_utf8 = NULL; NMWifiAP *ap; - const GByteArray *ssid = NULL; - GByteArray *tmp_ssid = NULL; + GBytes *ssid = NULL; GBytes *setting_ssid = NULL; gboolean hidden = FALSE; const char *perm_hw_addr; @@ -836,19 +835,14 @@ complete_connection (NMDevice *device, if (ap) ssid = nm_wifi_ap_get_ssid (ap); + if (ssid == NULL) { /* The AP must be hidden. Connecting to a WiFi AP requires the SSID * as part of the initial handshake, so check the connection details * for the SSID. The AP object will still be used for encryption * settings and such. */ - setting_ssid = nm_setting_wireless_get_ssid (s_wifi); - if (setting_ssid) { - ssid = tmp_ssid = g_byte_array_new (); - g_byte_array_append (tmp_ssid, - g_bytes_get_data (setting_ssid, NULL), - g_bytes_get_size (setting_ssid)); - } + ssid = nm_setting_wireless_get_ssid (s_wifi); } if (ssid == NULL) { @@ -871,11 +865,8 @@ complete_connection (NMDevice *device, if (!nm_wifi_ap_complete_connection (ap, connection, nm_wifi_utils_is_manf_default_ssid (ssid), - error)) { - if (tmp_ssid) - g_byte_array_unref (tmp_ssid); + error)) return FALSE; - } } /* The kernel doesn't support Ad-Hoc WPA connections well at this time, @@ -888,24 +879,18 @@ complete_connection (NMDevice *device, NM_CONNECTION_ERROR_INVALID_SETTING, _("WPA Ad-Hoc disabled due to kernel bugs")); g_prefix_error (error, "%s: ", NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); - if (tmp_ssid) - g_byte_array_unref (tmp_ssid); return FALSE; } - str_ssid = nm_utils_ssid_to_utf8 (ssid->data, ssid->len); - + ssid_utf8 = _nm_utils_ssid_to_utf8 (ssid); nm_utils_complete_generic (nm_device_get_platform (device), connection, NM_SETTING_WIRELESS_SETTING_NAME, existing_connections, - str_ssid, - str_ssid, + ssid_utf8, + ssid_utf8, NULL, TRUE); - g_free (str_ssid); - if (tmp_ssid) - g_byte_array_unref (tmp_ssid); if (hidden) g_object_set (s_wifi, NM_SETTING_WIRELESS_HIDDEN, TRUE, NULL); @@ -1368,14 +1353,14 @@ request_wireless_scan (NMDeviceWifi *self, guint i; for (i = 0; i < ssids->len; i++) { - gs_free char *foo = NULL; - const guint8 *p; - gsize l; + gs_free char *ssid_str = NULL; + GBytes *ssid = ssids->pdata[i]; - p = g_bytes_get_data (ssids->pdata[i], &l); - foo = l > 0 ? nm_utils_ssid_to_utf8 (p, l) : NULL; - _LOGD (LOGD_WIFI, "wifi-scan: (%u) probe scanning SSID %s%s%s", - i, NM_PRINT_FMT_QUOTED (foo, "\"", foo, "\"", "*any*")); + ssid_str = g_bytes_get_size (ssid) > 0 + ? _nm_utils_ssid_to_string (ssid) + : NULL; + _LOGD (LOGD_WIFI, "wifi-scan: (%u) probe scanning SSID %s", + i, ssid_str ?: "*any*"); } } else _LOGD (LOGD_WIFI, "wifi-scan: no SSIDs to probe scan"); @@ -1550,7 +1535,7 @@ supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); NMDeviceState state; NMWifiAP *found_ap = NULL; - const GByteArray *ssid; + GBytes *ssid; g_return_if_fail (self != NULL); g_return_if_fail (properties != NULL); @@ -1579,15 +1564,19 @@ supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, /* Let the manager try to fill in the SSID from seen-bssids lists */ ssid = nm_wifi_ap_get_ssid (ap); - if (!ssid || nm_utils_is_empty_ssid (ssid->data, ssid->len)) { + if (!ssid || _nm_utils_is_empty_ssid (ssid)) { /* Try to fill the SSID from the AP database */ try_fill_ssid_for_hidden_ap (self, ap); ssid = nm_wifi_ap_get_ssid (ap); - if (ssid && (nm_utils_is_empty_ssid (ssid->data, ssid->len) == FALSE)) { + if ( ssid + && !_nm_utils_is_empty_ssid (ssid)) { + gs_free char *s = NULL; + /* Yay, matched it, no longer treat as hidden */ - _LOGD (LOGD_WIFI, "matched hidden AP %s => '%s'", - nm_wifi_ap_get_address (ap), nm_utils_escape_ssid (ssid->data, ssid->len)); + _LOGD (LOGD_WIFI, "matched hidden AP %s => %s", + nm_wifi_ap_get_address (ap), + (s = _nm_utils_ssid_to_string (ssid))); } else { /* Didn't have an entry for this AP in the database */ _LOGD (LOGD_WIFI, "failed to match hidden AP %s", @@ -2044,6 +2033,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, NMConnection *connection; NMSettingWireless *s_wifi; GBytes *ssid; + gs_free char *ssid_str = NULL; connection = nm_device_get_applied_connection (NM_DEVICE (self)); g_return_if_fail (connection); @@ -2055,11 +2045,11 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, g_return_if_fail (ssid); _LOGI (LOGD_DEVICE | LOGD_WIFI, - "Activation: (wifi) Stage 2 of 5 (Device Configure) successful. %s '%s'.", - priv->mode == NM_802_11_MODE_AP ? "Started Wi-Fi Hotspot" : - "Connected to wireless network", - ssid ? nm_utils_escape_ssid (g_bytes_get_data (ssid, NULL), - g_bytes_get_size (ssid)) : "(none)"); + "Activation: (wifi) Stage 2 of 5 (Device Configure) successful. %s %s", + priv->mode == NM_802_11_MODE_AP + ? "Started Wi-Fi Hotspot" + : "Connected to wireless network", + (ssid_str = _nm_utils_ssid_to_string (ssid))); nm_device_activate_schedule_stage3_ip_config_start (device); } else if (devstate == NM_DEVICE_STATE_ACTIVATED) periodic_update (self); @@ -2167,9 +2157,11 @@ supplicant_iface_notify_current_bss (NMSupplicantInterface *iface, if (new_ap != priv->current_ap) { const char *new_bssid = NULL; - const GByteArray *new_ssid = NULL; + GBytes *new_ssid = NULL; const char *old_bssid = NULL; - const GByteArray *old_ssid = NULL; + GBytes *old_ssid = NULL; + gs_free char *new_ssid_s = NULL; + gs_free char *old_ssid_s = NULL; /* Don't ever replace a "fake" current AP if we don't know about the * supplicant's current BSS yet. It'll get replaced when we receive @@ -2190,9 +2182,9 @@ supplicant_iface_notify_current_bss (NMSupplicantInterface *iface, _LOGD (LOGD_WIFI, "roamed from BSSID %s (%s) to %s (%s)", old_bssid ?: "(none)", - old_ssid ? nm_utils_escape_ssid (old_ssid->data, old_ssid->len) : "(none)", + (old_ssid_s = _nm_utils_ssid_to_string (old_ssid)), new_bssid ?: "(none)", - new_ssid ? nm_utils_escape_ssid (new_ssid->data, new_ssid->len) : "(none)"); + (new_ssid_s = _nm_utils_ssid_to_string (new_ssid))); set_current_ap (self, new_ap, TRUE); } diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index f40f7191e3..f6b6fc5882 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -58,7 +58,7 @@ struct _NMWifiAPPrivate { char *supplicant_path; /* D-Bus object path of this AP from wpa_supplicant */ /* Scanned or cached values */ - GByteArray * ssid; + GBytes * ssid; char * address; NM80211Mode mode; guint8 strength; @@ -95,7 +95,7 @@ nm_wifi_ap_get_supplicant_path (NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->supplicant_path; } -const GByteArray * +GBytes * nm_wifi_ap_get_ssid (const NMWifiAP *ap) { g_return_val_if_fail (NM_IS_WIFI_AP (ap), NULL); @@ -103,18 +103,6 @@ nm_wifi_ap_get_ssid (const NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->ssid; } -static GVariant * -nm_wifi_ap_get_ssid_as_variant (const NMWifiAP *self) -{ - const NMWifiAPPrivate *priv = NM_WIFI_AP_GET_PRIVATE (self); - - if (priv->ssid) { - return g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, - priv->ssid->data, priv->ssid->len, 1); - } else - return g_variant_new_array (G_VARIANT_TYPE_BYTE, NULL, 0); -} - gboolean nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, gsize len) { @@ -126,20 +114,22 @@ nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, gsize len) priv = NM_WIFI_AP_GET_PRIVATE (ap); /* same SSID */ - if ((ssid && priv->ssid) && (len == priv->ssid->len)) { - if (!memcmp (ssid, priv->ssid->data, len)) + if (priv->ssid) { + const guint8 *p; + gsize l; + + p = g_bytes_get_data (priv->ssid, &l); + if ( l == len + && !memcmp (p, ssid, len)) + return FALSE; + } else { + if (len == 0) return FALSE; } - if (priv->ssid) { - g_byte_array_free (priv->ssid, TRUE); - priv->ssid = NULL; - } - - if (ssid) { - priv->ssid = g_byte_array_new (); - g_byte_array_append (priv->ssid, ssid, len); - } + nm_clear_pointer (&priv->ssid, g_bytes_unref); + if (len > 0) + priv->ssid = g_bytes_new (ssid, len); _notify (ap, PROP_SSID); return TRUE; @@ -961,7 +951,7 @@ nm_wifi_ap_to_string (const NMWifiAP *self, const char *supplicant_id = "-"; const char *export_path; guint32 chan; - char b1[200]; + gs_free char *ssid_to_free = NULL; g_return_val_if_fail (NM_IS_WIFI_AP (self), NULL); @@ -977,10 +967,9 @@ nm_wifi_ap_to_string (const NMWifiAP *self, export_path = "/"; g_snprintf (str_buf, buf_len, - "%17s %-32s [ %c %3u %3u%% %c W:%04X R:%04X ] %3us sup:%s [nm:%s]", + "%17s %-35s [ %c %3u %3u%% %c W:%04X R:%04X ] %3us sup:%s [nm:%s]", priv->address ?: "(none)", - nm_sprintf_buf (b1, "%s%s%s", - NM_PRINT_FMT_QUOTED (priv->ssid, "\"", nm_utils_escape_ssid (priv->ssid->data, priv->ssid->len), "\"", "(none)")), + (ssid_to_free = _nm_utils_ssid_to_string (priv->ssid)), (priv->mode == NM_802_11_MODE_ADHOC ? '*' : (priv->hotspot @@ -1032,15 +1021,12 @@ nm_wifi_ap_check_compatible (NMWifiAP *self, return FALSE; ssid = nm_setting_wireless_get_ssid (s_wireless); - if ( (ssid && !priv->ssid) - || (priv->ssid && !ssid)) - return FALSE; - - if ( ssid && priv->ssid && - !nm_utils_same_ssid (g_bytes_get_data (ssid, NULL), g_bytes_get_size (ssid), - priv->ssid->data, priv->ssid->len, - FALSE)) - return FALSE; + if (ssid != priv->ssid) { + if (!ssid || !priv->ssid) + return FALSE; + if (!g_bytes_equal (ssid, priv->ssid)) + return FALSE; + } bssid = nm_setting_wireless_get_bssid (s_wireless); if (bssid && (!priv->address || !nm_utils_hwaddr_matches (bssid, -1, priv->address, -1))) @@ -1126,7 +1112,8 @@ get_property (GObject *object, guint prop_id, g_value_set_uint (value, priv->rsn_flags); break; case PROP_SSID: - g_value_take_variant (value, nm_wifi_ap_get_ssid_as_variant (self)); + g_value_take_variant (value, + nm_utils_gbytes_to_variant_ay (priv->ssid)); break; case PROP_FREQUENCY: g_value_set_uint (value, priv->freq); @@ -1334,7 +1321,7 @@ finalize (GObject *object) g_free (priv->supplicant_path); if (priv->ssid) - g_byte_array_free (priv->ssid, TRUE); + g_bytes_unref (priv->ssid); g_free (priv->address); G_OBJECT_CLASS (nm_wifi_ap_parent_class)->finalize (object); diff --git a/src/devices/wifi/nm-wifi-ap.h b/src/devices/wifi/nm-wifi-ap.h index 4fdeee935c..57db4443a6 100644 --- a/src/devices/wifi/nm-wifi-ap.h +++ b/src/devices/wifi/nm-wifi-ap.h @@ -72,7 +72,7 @@ gboolean nm_wifi_ap_complete_connection (NMWifiAP *self, GError **error); const char * nm_wifi_ap_get_supplicant_path (NMWifiAP *ap); -const GByteArray *nm_wifi_ap_get_ssid (const NMWifiAP *ap); +GBytes *nm_wifi_ap_get_ssid (const NMWifiAP *ap); gboolean nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, gsize len); diff --git a/src/devices/wifi/nm-wifi-utils.c b/src/devices/wifi/nm-wifi-utils.c index 2fe81e323a..c9806a53cb 100644 --- a/src/devices/wifi/nm-wifi-utils.c +++ b/src/devices/wifi/nm-wifi-utils.c @@ -525,7 +525,7 @@ verify_adhoc (NMSettingWirelessSecurity *s_wsec, } gboolean -nm_wifi_utils_complete_connection (const GByteArray *ap_ssid, +nm_wifi_utils_complete_connection (GBytes *ap_ssid, const char *bssid, NM80211Mode ap_mode, guint32 ap_flags, @@ -538,7 +538,7 @@ nm_wifi_utils_complete_connection (const GByteArray *ap_ssid, NMSettingWireless *s_wifi; NMSettingWirelessSecurity *s_wsec; NMSetting8021x *s_8021x; - GBytes *ssid, *ap_ssid_bytes; + GBytes *ssid; const char *mode, *key_mgmt, *auth_alg, *leap_username; gboolean adhoc = FALSE; @@ -548,20 +548,17 @@ nm_wifi_utils_complete_connection (const GByteArray *ap_ssid, s_8021x = nm_connection_get_setting_802_1x (connection); /* Fill in missing SSID */ - ap_ssid_bytes = ap_ssid ? g_bytes_new (ap_ssid->data, ap_ssid->len) : NULL; ssid = nm_setting_wireless_get_ssid (s_wifi); if (!ssid) - g_object_set (G_OBJECT (s_wifi), NM_SETTING_WIRELESS_SSID, ap_ssid_bytes, NULL); - else if (!ap_ssid_bytes || !g_bytes_equal (ssid, ap_ssid_bytes)) { + g_object_set (G_OBJECT (s_wifi), NM_SETTING_WIRELESS_SSID, ap_ssid, NULL); + else if (!ap_ssid || !g_bytes_equal (ssid, ap_ssid)) { g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, _("connection does not match access point")); g_prefix_error (error, "%s.%s: ", NM_SETTING_WIRELESS_SETTING_NAME, NM_SETTING_WIRELESS_SSID); - g_bytes_unref (ap_ssid_bytes); return FALSE; } - g_bytes_unref (ap_ssid_bytes); if (lock_bssid && !nm_setting_wireless_get_bssid (s_wifi)) g_object_set (G_OBJECT (s_wifi), NM_SETTING_WIRELESS_BSSID, bssid, NULL); @@ -783,8 +780,10 @@ nm_wifi_utils_level_to_quality (int val) } gboolean -nm_wifi_utils_is_manf_default_ssid (const GByteArray *ssid) +nm_wifi_utils_is_manf_default_ssid (GBytes *ssid) { + const guint8 *ssid_p; + gsize ssid_l; int i; /* * List of manufacturer default SSIDs that are often unchanged by users. @@ -806,9 +805,11 @@ nm_wifi_utils_is_manf_default_ssid (const GByteArray *ssid) "TURBONETT", }; + ssid_p = g_bytes_get_data (ssid, &ssid_l); + for (i = 0; i < G_N_ELEMENTS (manf_defaults); i++) { - if (ssid->len == strlen (manf_defaults[i])) { - if (memcmp (manf_defaults[i], ssid->data, ssid->len) == 0) + if (ssid_l == strlen (manf_defaults[i])) { + if (memcmp (manf_defaults[i], ssid_p, ssid_l) == 0) return TRUE; } } diff --git a/src/devices/wifi/nm-wifi-utils.h b/src/devices/wifi/nm-wifi-utils.h index 3a982bb99b..88dc37911f 100644 --- a/src/devices/wifi/nm-wifi-utils.h +++ b/src/devices/wifi/nm-wifi-utils.h @@ -27,7 +27,7 @@ #include "nm-setting-wireless-security.h" #include "nm-setting-8021x.h" -gboolean nm_wifi_utils_complete_connection (const GByteArray *ssid, +gboolean nm_wifi_utils_complete_connection (GBytes *ssid, const char *bssid, NM80211Mode mode, guint32 flags, @@ -39,6 +39,6 @@ gboolean nm_wifi_utils_complete_connection (const GByteArray *ssid, guint32 nm_wifi_utils_level_to_quality (int val); -gboolean nm_wifi_utils_is_manf_default_ssid (const GByteArray *ssid); +gboolean nm_wifi_utils_is_manf_default_ssid (GBytes *ssid); #endif /* __NM_WIFI_UTILS_H__ */ diff --git a/src/devices/wifi/tests/test-general.c b/src/devices/wifi/tests/test-general.c index a4507813d3..f752bbfcd9 100644 --- a/src/devices/wifi/tests/test-general.c +++ b/src/devices/wifi/tests/test-general.c @@ -74,8 +74,7 @@ complete_connection (const char *ssid, NMConnection *src, GError **error) { - GByteArray *tmp; - gboolean success; + gs_unref_bytes GBytes *ssid_b = NULL; NMSettingWireless *s_wifi; /* Add a wifi setting if one doesn't exist */ @@ -85,20 +84,17 @@ complete_connection (const char *ssid, nm_connection_add_setting (src, NM_SETTING (s_wifi)); } - tmp = g_byte_array_sized_new (strlen (ssid)); - g_byte_array_append (tmp, (const guint8 *) ssid, strlen (ssid)); + ssid_b = g_bytes_new (ssid, strlen (ssid)); - success = nm_wifi_utils_complete_connection (tmp, - bssid, - mode, - flags, - wpa_flags, - rsn_flags, - src, - lock_bssid, - error); - g_byte_array_free (tmp, TRUE); - return success; + return nm_wifi_utils_complete_connection (ssid_b, + bssid, + mode, + flags, + wpa_flags, + rsn_flags, + src, + lock_bssid, + error); } typedef struct { diff --git a/src/platform/wifi/nm-wifi-utils-wext.c b/src/platform/wifi/nm-wifi-utils-wext.c index 8045b6d4f7..e52ae5a362 100644 --- a/src/platform/wifi/nm-wifi-utils-wext.c +++ b/src/platform/wifi/nm-wifi-utils-wext.c @@ -41,6 +41,7 @@ #include "nm-wifi-utils-private.h" #include "nm-utils.h" #include "platform/nm-platform-utils.h" +#include "nm-core-internal.h" typedef struct { NMWifiUtils parent; @@ -506,11 +507,13 @@ wifi_wext_set_mesh_ssid (NMWifiUtils *data, const guint8 *ssid, gsize len) return TRUE; if (errno != ENODEV) { + gs_free char *ssid_str = NULL; + errsv = errno; _LOGE (LOGD_PLATFORM | LOGD_WIFI | LOGD_OLPC, "(%s): error setting SSID to '%s': %s", ifname, - ssid ? nm_utils_escape_ssid (ssid, len) : "(null)", + (ssid_str = _nm_utils_ssid_to_string_arr (ssid, len)), strerror (errsv)); } diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 616e5f8ba0..4d2a16bc84 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -4013,7 +4013,7 @@ wireless_connection_from_ifcfg (const char *file, NMSetting8021x *s_8021x = NULL; GBytes *ssid; NMSetting *security_setting = NULL; - char *printable_ssid = NULL; + gs_free char *ssid_utf8 = NULL; const char *mode; gboolean adhoc = FALSE; GError *local = NULL; @@ -4033,12 +4033,6 @@ wireless_connection_from_ifcfg (const char *file, nm_connection_add_setting (connection, wireless_setting); ssid = nm_setting_wireless_get_ssid (NM_SETTING_WIRELESS (wireless_setting)); - if (ssid) { - printable_ssid = nm_utils_ssid_to_utf8 (g_bytes_get_data (ssid, NULL), - g_bytes_get_size (ssid)); - } else - printable_ssid = g_strdup ("unmanaged"); - mode = nm_setting_wireless_get_mode (NM_SETTING_WIRELESS (wireless_setting)); if (mode && !strcmp (mode, "adhoc")) adhoc = TRUE; @@ -4046,7 +4040,6 @@ wireless_connection_from_ifcfg (const char *file, /* Wireless security */ security_setting = make_wireless_security_setting (ifcfg, file, ssid, adhoc, &s_8021x, &local); if (local) { - g_free (printable_ssid); g_object_unref (connection); g_propagate_error (error, local); return NULL; @@ -4057,11 +4050,16 @@ wireless_connection_from_ifcfg (const char *file, nm_connection_add_setting (connection, NM_SETTING (s_8021x)); } + if (ssid) + ssid_utf8 = _nm_utils_ssid_to_utf8 (ssid); + /* Connection */ - con_setting = make_connection_setting (file, ifcfg, + con_setting = make_connection_setting (file, + ifcfg, NM_SETTING_WIRELESS_SETTING_NAME, - printable_ssid, NULL); - g_free (printable_ssid); + nm_str_not_empty (ssid_utf8) ?: "unmanaged", + NULL); + if (!con_setting) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Failed to create connection setting."); From f1bc0f0bf2a7ab187f88b70f04a8e6b04e7f8f37 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 12 Aug 2018 19:19:21 +0200 Subject: [PATCH 13/13] wifi: refactor nm_wifi_ap_set_ssid() to accept GBytes - have two variants of functions to set the SSID of an access point: one that passes SSID as GBytes, and one that passes it as plain data with length. Accepting a GBytes allows to share the immutable GBytes instance. - both functions now also support clearing the SSID. In nm_wifi_ap_update_from_properties(), if the GVariant specifies a "SSID", we always update the access point. We already support chaging the SSID, so why not support changing it to *no* SSID (hidden). --- src/devices/wifi/nm-device-iwd.c | 7 ++- src/devices/wifi/nm-device-wifi.c | 6 +-- src/devices/wifi/nm-wifi-ap.c | 89 ++++++++++++++++++++++--------- src/devices/wifi/nm-wifi-ap.h | 6 ++- 4 files changed, 73 insertions(+), 35 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 84ee421740..3a6855cbad 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -296,8 +296,11 @@ get_ordered_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) props = g_variant_new ("a{sv}", &builder); ap = nm_wifi_ap_new_from_properties (path, props); - if (name[0] != '\0') - nm_wifi_ap_set_ssid (ap, (const guint8 *) name, strlen (name)); + + nm_wifi_ap_set_ssid_arr (ap, + (const guint8 *) name, + NM_MIN (32, strlen (name))); + nm_wifi_ap_set_strength (ap, nm_wifi_utils_level_to_quality (signal / 100)); nm_wifi_ap_set_freq (ap, 2417); nm_wifi_ap_set_max_bitrate (ap, 65000); diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 2245fccf08..6c5a4723fb 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1515,11 +1515,7 @@ try_fill_ssid_for_hidden_ap (NMDeviceWifi *self, s_wifi = nm_connection_get_setting_wireless (connection); if (s_wifi) { if (nm_settings_connection_has_seen_bssid (NM_SETTINGS_CONNECTION (connection), bssid)) { - GBytes *ssid = nm_setting_wireless_get_ssid (s_wifi); - - nm_wifi_ap_set_ssid (ap, - g_bytes_get_data (ssid, NULL), - g_bytes_get_size (ssid)); + nm_wifi_ap_set_ssid (ap, nm_setting_wireless_get_ssid (s_wifi)); break; } } diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index f6b6fc5882..7cfc659875 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -104,32 +104,66 @@ nm_wifi_ap_get_ssid (const NMWifiAP *ap) } gboolean -nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, gsize len) +nm_wifi_ap_set_ssid_arr (NMWifiAP *ap, + const guint8 *ssid, + gsize ssid_len) { NMWifiAPPrivate *priv; + const guint8 *my_data; + gsize my_len; g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); - g_return_val_if_fail (ssid == NULL || len > 0, FALSE); + + if (ssid_len > 32) + g_return_val_if_reached (FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); - /* same SSID */ - if (priv->ssid) { - const guint8 *p; - gsize l; - - p = g_bytes_get_data (priv->ssid, &l); - if ( l == len - && !memcmp (p, ssid, len)) - return FALSE; - } else { - if (len == 0) - return FALSE; + if (priv->ssid) + my_data = g_bytes_get_data (priv->ssid, &my_len); + else { + my_data = NULL; + my_len = 0; } + if ( my_len == ssid_len + && memcmp (ssid, my_data, ssid_len) == 0) + return FALSE; + nm_clear_pointer (&priv->ssid, g_bytes_unref); - if (len > 0) - priv->ssid = g_bytes_new (ssid, len); + if (ssid_len > 0) + priv->ssid = g_bytes_new (ssid, ssid_len); + + _notify (ap, PROP_SSID); + return TRUE; +} + +gboolean +nm_wifi_ap_set_ssid (NMWifiAP *ap, GBytes *ssid) +{ + NMWifiAPPrivate *priv; + gsize l; + + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); + + if (ssid) { + l = g_bytes_get_size (ssid); + if (l == 0 || l > 32) + g_return_val_if_reached (FALSE); + } + + priv = NM_WIFI_AP_GET_PRIVATE (ap); + + if (ssid == priv->ssid) + return FALSE; + if ( ssid + && priv->ssid + && g_bytes_equal (ssid, priv->ssid)) + return FALSE; + + nm_clear_pointer (&priv->ssid, g_bytes_unref); + if (ssid) + priv->ssid = g_bytes_ref (ssid); _notify (ap, PROP_SSID); return TRUE; @@ -804,10 +838,16 @@ nm_wifi_ap_update_from_properties (NMWifiAP *ap, len = MIN (32, len); /* Stupid ieee80211 layer uses */ - if ( bytes && len - && !(((len == 8) || (len == 9)) && !memcmp (bytes, "", 8)) - && !nm_utils_is_empty_ssid (bytes, len)) - changed |= nm_wifi_ap_set_ssid (ap, bytes, len); + if ( bytes + && len + && !( NM_IN_SET (len, 8, 9) + && memcmp (bytes, "", len) == 0) + && !nm_utils_is_empty_ssid (bytes, len)) { + /* good */ + } else + len = 0; + + changed |= nm_wifi_ap_set_ssid_arr (ap, bytes, len); g_variant_unref (v); } @@ -1189,7 +1229,6 @@ nm_wifi_ap_new_fake_from_connection (NMConnection *connection) NMWifiAPPrivate *priv; NMSettingWireless *s_wireless; NMSettingWirelessSecurity *s_wireless_sec; - GBytes *ssid; const char *mode, *band, *key_mgmt; guint32 channel; NM80211ApSecurityFlags flags; @@ -1200,14 +1239,12 @@ nm_wifi_ap_new_fake_from_connection (NMConnection *connection) s_wireless = nm_connection_get_setting_wireless (connection); g_return_val_if_fail (s_wireless != NULL, NULL); - ssid = nm_setting_wireless_get_ssid (s_wireless); - g_return_val_if_fail (ssid != NULL, NULL); - g_return_val_if_fail (g_bytes_get_size (ssid) > 0, NULL); - ap = (NMWifiAP *) g_object_new (NM_TYPE_WIFI_AP, NULL); priv = NM_WIFI_AP_GET_PRIVATE (ap); priv->fake = TRUE; - nm_wifi_ap_set_ssid (ap, g_bytes_get_data (ssid, NULL), g_bytes_get_size (ssid)); + + nm_wifi_ap_set_ssid (ap, + nm_setting_wireless_get_ssid (s_wireless)); // FIXME: bssid too? diff --git a/src/devices/wifi/nm-wifi-ap.h b/src/devices/wifi/nm-wifi-ap.h index 57db4443a6..7462e9d1c5 100644 --- a/src/devices/wifi/nm-wifi-ap.h +++ b/src/devices/wifi/nm-wifi-ap.h @@ -73,9 +73,11 @@ gboolean nm_wifi_ap_complete_connection (NMWifiAP *self, const char * nm_wifi_ap_get_supplicant_path (NMWifiAP *ap); GBytes *nm_wifi_ap_get_ssid (const NMWifiAP *ap); -gboolean nm_wifi_ap_set_ssid (NMWifiAP *ap, +gboolean nm_wifi_ap_set_ssid_arr (NMWifiAP *ap, const guint8 *ssid, - gsize len); + gsize ssid_len); +gboolean nm_wifi_ap_set_ssid (NMWifiAP *ap, + GBytes *ssid); const char * nm_wifi_ap_get_address (const NMWifiAP *ap); gboolean nm_wifi_ap_set_address (NMWifiAP *ap, const char *addr);