fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-01 11:50:18 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

wifi: parse RequestScan D-Bus arguments before authenticating request

Browse source 
It feels better to first parse input arguments before authenticating.
One argument for otherwise would be that we shouldn't reveal any
information about the request before authenticating it. Meaning: every
request (even with invalid arguments) should fail with
permission-denied.

However, I prefer this for minor reasons:

- what makes a valid request is no secret. And if somebody makes an
invalid request, it should fail with invalid-arguments first.

- we possibly can short cut the expensive authentication process, where
we ask PolicyKit.

- by extracting the options variant early and only pass on the SSIDs
array, we handle the encoding of the options array earlier and where
it belongs: closer to the D-Bus request that defines the meaning of
the argument.

Also, change the failure reason to return invalid-argument.
This commit is contained in:
Thomas Haller 2020-03-20 12:43:43 +01:00
parent c1f473d342
commit cf30e15aba
1 changed files with 27 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

54
src/devices/wifi/nm-device-wifi.c
View file

@ -1146,7 +1146,7 @@ ssids_options_to_ptrarray (GVariant *value, GError **error)
if (num_ssids > 32) {
g_set_error_literal (error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_NOT_ALLOWED,
NM_DEVICE_ERROR_INVALID_ARGUMENT,
"too many SSIDs requested to scan");
return NULL;
}
@ -1163,7 +1163,7 @@ ssids_options_to_ptrarray (GVariant *value, GError **error)
if (len > 32) {
g_set_error (error,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_NOT_ALLOWED,
NM_DEVICE_ERROR_INVALID_ARGUMENT,
"SSID at index %d more than 32 bytes", (int) i);
return NULL;
}
@ -1189,8 +1189,7 @@ dbus_request_scan_cb (NMDevice *device,
gpointer user_data)
{
NMDeviceWifi *self = NM_DEVICE_WIFI (device);
gs_unref_variant GVariant *scan_options = user_data;
gs_unref_ptrarray GPtrArray *ssids = NULL;
gs_unref_ptrarray GPtrArray *ssids = user_data;
if (error) {
g_dbus_method_invocation_return_gerror (context, error);
@ -1205,28 +1204,6 @@ dbus_request_scan_cb (NMDevice *device,
return;
}
if (scan_options) {
gs_unref_variant GVariant *val = g_variant_lookup_value (scan_options, "ssids", NULL);
if (val) {
gs_free_error GError *ssid_error = NULL;
if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("aay"))) {
g_dbus_method_invocation_return_error_literal (context,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_NOT_ALLOWED,
"Invalid 'ssid' scan option");
return;
}
ssids = ssids_options_to_ptrarray (val, &ssid_error);
if (ssid_error) {
g_dbus_method_invocation_return_gerror (context, ssid_error);
return;
}
}
}
request_wireless_scan (self, FALSE, FALSE, ssids);
g_dbus_method_invocation_return_value (context, NULL);
}
@ -1239,6 +1216,29 @@ _nm_device_wifi_request_scan (NMDeviceWifi *self,
NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
NMDevice *device = NM_DEVICE (self);
gint64 last_scan;
gs_unref_ptrarray GPtrArray *ssids = NULL;
if (options) {
gs_unref_variant GVariant *val = g_variant_lookup_value (options, "ssids", NULL);
if (val) {
gs_free_error GError *ssid_error = NULL;
if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("aay"))) {
g_dbus_method_invocation_return_error_literal (invocation,
NM_DEVICE_ERROR,
NM_DEVICE_ERROR_INVALID_ARGUMENT,
"Invalid 'ssid' scan option");
return;
}
ssids = ssids_options_to_ptrarray (val, &ssid_error);
if (ssid_error) {
g_dbus_method_invocation_return_gerror (invocation, ssid_error);
return;
}
}
}
if ( !priv->enabled
|| !priv->sup_iface
@ -1281,7 +1281,7 @@ _nm_device_wifi_request_scan (NMDeviceWifi *self,
NM_AUTH_PERMISSION_WIFI_SCAN,
TRUE,
dbus_request_scan_cb,
options ? g_variant_ref (options) : NULL);
g_steal_pointer (&ssids));
}
static gboolean