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

[Gary Ching-Pang Lin <chingpang gmail com>]

2012/6/2 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?
Sure! This is a good solution since there will be no duplicate nm-auth bit in
the wifi device. I'll rebase my patch with the signal.

Gary Lin
>
> 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"
>
>