Re: [PATCH] wifi: add the on-demand WiFi scan support

[Dan Williams <dcbw redhat com>]

On Mon, 2012-05-28 at 16:31 +0800, Gary Ching-Pang Lin wrote:
> A new dbus method was added to request the wifi device to scan the
> access points.

Thanks!  Looking over the patch, I think we can do it more easily with a
small change to the core NM code.  Basically, the disconnect request and
the wifi scan request bits both need to authenticate the caller with a
specific permission, and then perform some action later.  The manager
handles all that already, so what I propose is to change the NMDevice
signal 'disconnect-request' into a generic 'auth-request' signal.  The
arguments the device passes in that signal would be the PK permission to
check, a callback function, and possibly some user data.  The manager
would then process the requests (cleanly handling device removal of
course) and then call the callback.  I'm happy to do this part, and then
you can rebase your patch onto that common functionality.  Does that
sound OK?

Dan

> ---
>  introspection/nm-device-wifi.xml |   13 ++++
>  src/nm-device-wifi.c             |  130 ++++++++++++++++++++++++++++++++++++++
>  src/nm-device-wifi.h             |    1 +
>  3 files changed, 144 insertions(+), 0 deletions(-)
> 
> diff --git a/introspection/nm-device-wifi.xml b/introspection/nm-device-wifi.xml
> index fb50762..531fc89 100644
> --- a/introspection/nm-device-wifi.xml
> +++ b/introspection/nm-device-wifi.xml
> @@ -14,6 +14,19 @@
>        </tp:docstring>
>      </method>
>  
> +    <method name="RequestScan">
> +      <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="impl_device_request_scan"/>
> +      <annotation name="org.freedesktop.DBus.GLib.Async" value=""/>
> +      <arg name="options" type="a{sv}" direction="in">
> +        <tp:docstring>
> +          Options of scan
> +        </tp:docstring>
> +      </arg>
> +      <tp:docstring>
> +        Request the device to scan
> +      </tp:docstring>
> +    </method>
> +
>      <property name="HwAddress" type="s" access="read">
>        <tp:docstring>
>          The active hardware address of the device.
> diff --git a/src/nm-device-wifi.c b/src/nm-device-wifi.c
> index ad1cfe3..417969e 100644
> --- a/src/nm-device-wifi.c
> +++ b/src/nm-device-wifi.c
> @@ -45,6 +45,7 @@
>  #include "nm-marshal.h"
>  #include "NetworkManagerUtils.h"
>  #include "nm-activation-request.h"
> +#include "nm-dbus-manager.h"
>  #include "nm-supplicant-manager.h"
>  #include "nm-supplicant-interface.h"
>  #include "nm-supplicant-config.h"
> @@ -57,6 +58,7 @@
>  #include "nm-setting-ip6-config.h"
>  #include "nm-system.h"
>  #include "nm-settings-connection.h"
> +#include "nm-manager-auth.h"
>  #include "nm-enum-types.h"
>  #include "wifi-utils.h"
>  
> @@ -64,6 +66,10 @@ static gboolean impl_device_get_access_points (NMDeviceWifi *device,
>                                                 GPtrArray **aps,
>                                                 GError **err);
>  
> +static void impl_device_request_scan (NMDeviceWifi *device,
> +                                      GHashTable *options,
> +                                      DBusGMethodInvocation *context);
> +
>  #include "nm-device-wifi-glue.h"
>  
> 
> @@ -150,6 +156,10 @@ struct _NMDeviceWifiPrivate {
>  	guint             periodic_source_id;
>  	guint             link_timeout_id;
>  
> +	NMDBusManager *   dbus_mgr;
> +	GSList *          auth_chains;
> +	glong             request_scan_time;
> +
>  	NMDeviceWifiCapabilities capabilities;
>  };
>  
> @@ -330,6 +340,12 @@ constructor (GType type,
>  	}
>  	priv->ipw_rfkill_state = nm_device_wifi_get_ipw_rfkill_state (self);
>  
> +	priv->dbus_mgr = nm_dbus_manager_get ();
> +	g_assert (priv->dbus_mgr);
> +
> +	priv->auth_chains = NULL;
> +	priv->request_scan_time = 0;
> +
>  	return object;
>  }
>  
> @@ -1441,6 +1457,112 @@ impl_device_get_access_points (NMDeviceWifi *self,
>  	return TRUE;
>  }
>  
> +static GError *
> +request_scan_check_error (GError *auth_error,
> +                          NMAuthCallResult result)
> +{
> +	if (auth_error) {
> +		nm_log_dbg (LOGD_WIFI, "request scan failed: %s", auth_error->message);
> +		return g_error_new (NM_WIFI_ERROR,
> +		                    NM_WIFI_ERROR_PERMISSION_DENIED,
> +		                    "request scan failed: %s",
> +		                    auth_error->message);
> +	} else if (result != NM_AUTH_CALL_RESULT_YES) {
> +		return g_error_new (NM_WIFI_ERROR,
> +		                    NM_WIFI_ERROR_PERMISSION_DENIED,
> +		                    "Not authorized to request scan");
> +	}
> +	return NULL;
> +}
> +
> +static void
> +do_request_scan (NMAuthChain *chain,
> +                 GError *auth_error,
> +                 DBusGMethodInvocation *context,
> +                 gpointer user_data)
> +
> +{
> +	NMDeviceWifi *self = NM_DEVICE_WIFI (user_data);
> +	NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
> +	NMAuthCallResult result;
> +	GError *error = NULL;
> +	GTimeVal now;
> +
> +	priv->auth_chains = g_slist_remove (priv->auth_chains, chain);
> +
> +	result = GPOINTER_TO_UINT (nm_auth_chain_get_data (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL));
> +	error = request_scan_check_error (auth_error, result);
> +	if (!error) {
> +		g_get_current_time (&now);
> +		cancel_pending_scan (self);
> +		request_wireless_scan (self);
> +		priv->request_scan_time = now.tv_sec;
> +
> +		dbus_g_method_return (context);
> +	} else {
> +		dbus_g_method_return_error (context, error);
> +	}
> +
> +	g_clear_error (&error);
> +	nm_auth_chain_unref (chain);
> +}
> +
> +static void
> +impl_device_request_scan (NMDeviceWifi *self,
> +                          GHashTable *options,
> +                          DBusGMethodInvocation *context)
> +{
> +	NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
> +	gulong sender_uid = G_MAXULONG;
> +	char *error_desc = NULL;
> +	GError *error = NULL;
> +	NMAuthChain *chain;
> +	GTimeVal now;
> +
> +	if (!priv->enabled)
> +		return;
> +
> +	g_get_current_time (&now);
> +	if (now.tv_sec - priv->request_scan_time < 10) {
> +		dbus_g_method_return (context);
> +		return;
> +	}
> +
> +	/* Need to check the caller's permissions and stuff before we can
> +	 * start the scan.
> +	 */
> +	if (!nm_auth_get_caller_uid (context,
> +		                     priv->dbus_mgr,
> +	                             &sender_uid,
> +	                             &error_desc)) {
> +		error = g_error_new_literal (NM_WIFI_ERROR,
> +		                             NM_WIFI_ERROR_PERMISSION_DENIED,
> +		                             error_desc);
> +		dbus_g_method_return_error (context, error);
> +		g_error_free (error);
> +		g_free (error_desc);
> +		return;
> +	}
> +
> +	/* Yay for root */
> +	if (0 == sender_uid) {
> +		cancel_pending_scan (self);
> +		request_wireless_scan (self);
> +		priv->request_scan_time = now.tv_sec;
> +		dbus_g_method_return (context);
> +		return;
> +	}
> +
> +	/* Otherwise validate the user request */
> +	chain = nm_auth_chain_new (context, NULL, do_request_scan, self);
> +	g_assert (chain);
> +	priv->auth_chains = g_slist_append (priv->auth_chains, chain);
> +
> +	nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_NETWORK_CONTROL, TRUE);
> +
> +	return;
> +}
> +
>  static gboolean
>  scanning_allowed (NMDeviceWifi *self)
>  {
> @@ -3399,6 +3521,14 @@ dispose (GObject *object)
>  		priv->ipw_rfkill_id = 0;
>  	}
>  
> +	g_slist_foreach (priv->auth_chains, (GFunc) nm_auth_chain_unref, NULL);
> +	g_slist_free (priv->auth_chains);
> +
> +	if (priv->dbus_mgr) {
> +		g_object_unref (priv->dbus_mgr);
> +		priv->dbus_mgr = NULL;
> +	}
> +
>  	G_OBJECT_CLASS (nm_device_wifi_parent_class)->dispose (object);
>  }
>  
> diff --git a/src/nm-device-wifi.h b/src/nm-device-wifi.h
> index 1e665fc..1541121 100644
> --- a/src/nm-device-wifi.h
> +++ b/src/nm-device-wifi.h
> @@ -46,6 +46,7 @@ typedef enum {
>  	NM_WIFI_ERROR_CONNECTION_INVALID,          /*< nick=ConnectionInvalid >*/
>  	NM_WIFI_ERROR_CONNECTION_INCOMPATIBLE,     /*< nick=ConnectionIncompatible >*/
>  	NM_WIFI_ERROR_ACCESS_POINT_NOT_FOUND,      /*< nick=AccessPointNotFound >*/
> +	NM_WIFI_ERROR_PERMISSION_DENIED,           /*< nick=PermissionDenied >*/
>  } NMWifiError;
>  
>  #define NM_DEVICE_WIFI_HW_ADDRESS          "hw-address"