GNOME.org
 

[PATCH v2] Re: Change the state of iface from disconnected to connected using nmcli

On Wednesday 16 of May 2012 18:02:12 Dan Williams wrote:
> On Mon, 2012-05-14 at 17:36 +0200, Jirka Klimes wrote:
> > On Thursday 22 of March 2012 11:10:56 Dan Williams wrote:
> > > On Sun, 2012-03-18 at 23:43 +0530, Abhijeet Rastogi wrote:
> > > > Hi,
> > > > 
> > > > 
> > > > This is my first post here. I have tried googling and looking at the
> > > > manpage but no help.
> > > > 
> > > > 
> > > > We can use
> > > > 
> > > > 
> > > > $nmcli dev disconnect iface eth0
> > > > 
> > > > 
> > > > to change the state of eth0 to disconnect. Is there a command to
> > > > change it's state back to connected? I tried looking at the manpage &
> > > > I can't seem to find  any option for that.
> > > 
> > > At the moment, you need to tell NM to reconnect to something, eg 'nmcli
> > > con up uuid <uuid>'.  Disconnect places the device into manual mode
> > > which requires user action to return to automatic mode.  I suppose we
> > > can add an 'autoconnect' property to each device, which Disconnect()
> > > would set to false, but which could be changed via the D-Bus properties
> > > interface (authenticated of course) back to TRUE.  This property would
> > > follow the internal 'autoconnect_inhibit' device member variable and the
> > > NMPolicy would listen for changes to this variable and retrigger an auto
> > > connection check if it changes back to TRUE.  Patches accepted for that.
> > > 
> > > Dan
> > 
> > Attached are patches adding 'Autoconnect' property to NMDevice for core NM
> > and libnm-glib as well.
> 
> For the API docs, I'd say:
> 
> "If TRUE, indicates the device is allowed to autoconnect.  If FALSE,
> manual intervention is required before the device will automatically
> connect to a known network, such as activating a connection using the
> device, or setting this property to TRUE."
> 
Changed the text in xml.

> There's a redundant NM_DEVICE_GET_PRIVATE() in
> nm_device_get_autoconnect().
> 
Removed.

> The other thing we need to do is authenticate the property write call,
> which we've got some code for in nm-manager.c::prop_filter().  It's
> going to get a bit complicated since that function only works on the
> manager object, so we'll have to have something that's a bit more
> involved than just strcmp(propiface, NM_DBUS_IFACE).  That said, it
> shouldn't be too bad.  We basically want to enforce the same permissions
> as manager_device_disconnect_request() does.
> 
After some tries it seems to me that the easiest way is to tweak the 
prop_filter() and use it.
I have added a check for .Device interface and find the right NMDevice object
according to object path extracted from the D-Bus message.
NM_AUTH_PERMISSION_NETWORK_CONTROL permission is used.
Does it look right?

I'm sending authorization changes as a separate patch, so that it is easier to
see what changed.
The changes were made to the previous "core" patch.
"libnm-glib" didn't change.

> The rest of it looks good!
> 
> Dan
> 
> And while we're at it, to reduce confusion, we should just rename
> "autoconnect_inhibit" to autoconnect.  Something like the attached patch
> applied on top of yours?
Changed to "autoconnect" variable.

Thanks for the review.
Jirka
>From 2c0a08b6298e6a3057b39a20256d67c601b76e4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= <jklimes redhat com>
Date: Mon, 14 May 2012 15:32:54 +0200
Subject: [PATCH 1/3] core: add "Autoconnect" property to NMDevice
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It is bound to autoconnect_inhibit private variable (has opposite meaning).
While 'Autoconnect' is TRUE (default value) the device can automatically
activate a connection. If it is changed to FALSE, the device will not
auto-activate until 'Autoconnect' is TRUE again.
Disconnect() method sets 'Autoconnect' to FALSE. NMPolicy monitors the property
and schedules auto activation when FALSE->TRUE transition is made.

Signed-off-by: Jiří Klimeš <jklimes redhat com>
---
 introspection/nm-device.xml |    8 ++++++++
 src/nm-device.c             |   40 +++++++++++++++++++++++++++-------------
 src/nm-device.h             |    5 +++--
 src/nm-manager.c            |    3 ++-
 src/nm-policy.c             |   12 +++++++++++-
 5 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/introspection/nm-device.xml b/introspection/nm-device.xml
index d12d477..5e58a47 100644
--- a/introspection/nm-device.xml
+++ b/introspection/nm-device.xml
@@ -91,6 +91,14 @@
         Whether or not this device is managed by NetworkManager.
       </tp:docstring>
     </property>
+    <property name="Autoconnect" type="b" access="readwrite">
+      <tp:docstring>
+        If TRUE, indicates the device is allowed to autoconnect.  If FALSE,
+        manual intervention is required before the device will automatically
+        connect to a known network, such as activating a connection using the
+        device, or setting this property to TRUE.
+      </tp:docstring>
+    </property>
     <property name="FirmwareMissing" type="b" access="read">
       <tp:docstring>
         If TRUE, indicates the device is likely missing firmware necessary for
diff --git a/src/nm-device.c b/src/nm-device.c
index 2f69be6..d63dcb9 100644
--- a/src/nm-device.c
+++ b/src/nm-device.c
@@ -111,6 +111,7 @@ enum {
 	PROP_ACTIVE_CONNECTION,
 	PROP_DEVICE_TYPE,
 	PROP_MANAGED,
+	PROP_AUTOCONNECT,
 	PROP_FIRMWARE_MISSING,
 	PROP_TYPE_DESC,
 	PROP_RFKILL_TYPE,
@@ -222,8 +223,8 @@ typedef struct {
 	/* IP6 config from DHCP */
 	NMIP6Config *   dhcp6_ip6_config;
 
-	/* inhibit autoconnect feature */
-	gboolean	autoconnect_inhibit;
+	/* allow autoconnect feature */
+	gboolean        autoconnect;
 
 	/* master interface for bridge, bond, vlan, etc */
 	NMDevice *	master;
@@ -743,10 +744,10 @@ nm_device_autoconnect_allowed (NMDevice *self)
 	g_value_take_object (&instance, self);
 
 	g_value_init (&retval, G_TYPE_BOOLEAN);
-	if (priv->autoconnect_inhibit)
-		g_value_set_boolean (&retval, FALSE);
-	else
+	if (priv->autoconnect)
 		g_value_set_boolean (&retval, TRUE);
+	else
+		g_value_set_boolean (&retval, FALSE);
 
 	/* Use g_signal_emitv() rather than g_signal_emit() to avoid the return
 	 * value being changed if no handlers are connected */
@@ -3145,7 +3146,7 @@ nm_device_disconnect (NMDevice *device, GError **error)
 		return FALSE;
 	}
 
-	priv->autoconnect_inhibit = TRUE;	
+	priv->autoconnect = FALSE;
 	nm_device_state_changed (device,
 	                         NM_DEVICE_STATE_DISCONNECTED,
 	                         NM_DEVICE_STATE_REASON_USER_REQUESTED);
@@ -3759,6 +3760,9 @@ set_property (GObject *object, guint prop_id,
 	case PROP_MANAGED:
 		priv->managed = g_value_get_boolean (value);
 		break;
+	case PROP_AUTOCONNECT:
+		priv->autoconnect = g_value_get_boolean (value);
+		break;
 	case PROP_FIRMWARE_MISSING:
 		priv->firmware_missing = g_value_get_boolean (value);
 		break;
@@ -3867,6 +3871,9 @@ get_property (GObject *object, guint prop_id,
 	case PROP_MANAGED:
 		g_value_set_boolean (value, priv->managed);
 		break;
+	case PROP_AUTOCONNECT:
+		g_value_set_boolean (value, priv->autoconnect);
+		break;
 	case PROP_FIRMWARE_MISSING:
 		g_value_set_boolean (value, priv->firmware_missing);
 		break;
@@ -4028,6 +4035,14 @@ nm_device_class_init (NMDeviceClass *klass)
 		                       G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
 
 	g_object_class_install_property
+		(object_class, PROP_AUTOCONNECT,
+		 g_param_spec_boolean (NM_DEVICE_AUTOCONNECT,
+		                       "Autoconnect",
+		                       "Autoconnect",
+		                       TRUE,
+		                       G_PARAM_READWRITE));
+
+	g_object_class_install_property
 		(object_class, PROP_FIRMWARE_MISSING,
 		 g_param_spec_boolean (NM_DEVICE_FIRMWARE_MISSING,
 		                       "FirmwareMissing",
@@ -4335,7 +4350,7 @@ nm_device_state_changed (NMDevice *device,
 			nm_device_deactivate (device, reason);
 		break;
 	default:
-		priv->autoconnect_inhibit = FALSE;
+		priv->autoconnect = TRUE;
 		break;
 	}
 
@@ -4655,12 +4670,11 @@ nm_device_set_dhcp_anycast_address (NMDevice *device, guint8 *addr)
 	}
 }
 
-
-void
-nm_device_clear_autoconnect_inhibit (NMDevice *device)
+gboolean
+nm_device_get_autoconnect (NMDevice *device)
 {
-	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
-	g_return_if_fail (priv);
-	priv->autoconnect_inhibit = FALSE;
+	g_return_val_if_fail (NM_IS_DEVICE (device), FALSE);
+
+	return NM_DEVICE_GET_PRIVATE (device)->autoconnect;
 }
 
diff --git a/src/nm-device.h b/src/nm-device.h
index f4495b9..23e780b 100644
--- a/src/nm-device.h
+++ b/src/nm-device.h
@@ -15,7 +15,7 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
- * Copyright (C) 2005 - 2011 Red Hat, Inc.
+ * Copyright (C) 2005 - 2012 Red Hat, Inc.
  * Copyright (C) 2006 - 2008 Novell, Inc.
  */
 
@@ -52,6 +52,7 @@
 #define NM_DEVICE_ACTIVE_CONNECTION "active-connection"
 #define NM_DEVICE_DEVICE_TYPE      "device-type" /* ugh */
 #define NM_DEVICE_MANAGED          "managed"
+#define NM_DEVICE_AUTOCONNECT      "autoconnect"
 #define NM_DEVICE_FIRMWARE_MISSING "firmware-missing"
 #define NM_DEVICE_TYPE_DESC        "type-desc"    /* Internal only */
 #define NM_DEVICE_RFKILL_TYPE      "rfkill-type"  /* Internal only */
@@ -244,7 +245,7 @@ void nm_device_set_managed (NMDevice *device,
                             gboolean managed,
                             NMDeviceStateReason reason);
 
-void nm_device_clear_autoconnect_inhibit (NMDevice *device);
+gboolean nm_device_get_autoconnect (NMDevice *device);
 
 void nm_device_handle_autoip4_event (NMDevice *self,
                                      const char *event,
diff --git a/src/nm-manager.c b/src/nm-manager.c
index e887e57..b669bb8 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -3248,7 +3248,8 @@ do_sleep_wake (NMManager *self)
 					nm_device_set_enabled (device, enabled);
 			}
 
-			nm_device_clear_autoconnect_inhibit (device);
+			g_object_set (G_OBJECT (device), NM_DEVICE_AUTOCONNECT, TRUE, NULL);
+
 			if (nm_device_spec_match_list (device, unmanaged_specs))
 				nm_device_set_managed (device, FALSE, NM_DEVICE_STATE_REASON_NOW_UNMANAGED);
 			else
diff --git a/src/nm-policy.c b/src/nm-policy.c
index babf164..16cd117 100644
--- a/src/nm-policy.c
+++ b/src/nm-policy.c
@@ -15,7 +15,7 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
- * Copyright (C) 2004 - 2011 Red Hat, Inc.
+ * Copyright (C) 2004 - 2012 Red Hat, Inc.
  * Copyright (C) 2007 - 2008 Novell, Inc.
  */
 
@@ -1127,6 +1127,15 @@ device_ip_config_changed (NMDevice *device,
 }
 
 static void
+device_autoconnect_changed (NMDevice *device,
+                            GParamSpec *pspec,
+                            gpointer user_data)
+{
+	if (nm_device_get_autoconnect (device))
+		schedule_activate_check ((NMPolicy *) user_data, device, 0);
+}
+
+static void
 wireless_networks_changed (NMDevice *device, GObject *ap, gpointer user_data)
 {
 	schedule_activate_check ((NMPolicy *) user_data, device, 0);
@@ -1169,6 +1178,7 @@ device_added (NMManager *manager, NMDevice *device, gpointer user_data)
 	_connect_device_signal (policy, device, "state-changed", device_state_changed);
 	_connect_device_signal (policy, device, "notify::" NM_DEVICE_IP4_CONFIG, device_ip_config_changed);
 	_connect_device_signal (policy, device, "notify::" NM_DEVICE_IP6_CONFIG, device_ip_config_changed);
+	_connect_device_signal (policy, device, "notify::" NM_DEVICE_AUTOCONNECT, device_autoconnect_changed);
 
 	switch (nm_device_get_device_type (device)) {
 	case NM_DEVICE_TYPE_WIFI:
-- 
1.7.7.6


>From 2a47c8979d979581b15e31d4b9ac559fc71627d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= <jklimes redhat com>
Date: Thu, 17 May 2012 17:01:10 +0200
Subject: [PATCH 2/3] core: authenticate Set() D-Bus call for NMDevice
 "Autoconnect" property
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


Signed-off-by: Jiří Klimeš <jklimes redhat com>
---
 src/nm-manager.c |   47 ++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/src/nm-manager.c b/src/nm-manager.c
index b669bb8..ec717a1 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -3769,7 +3769,8 @@ firmware_dir_changed (GFileMonitor *monitor,
 	}
 }
 
-#define PERM_DENIED_ERROR "org.freedesktop.NetworkManager.PermissionDenied"
+#define NM_PERM_DENIED_ERROR "org.freedesktop.NetworkManager.PermissionDenied"
+#define DEV_PERM_DENIED_ERROR "org.freedesktop.NetworkManager.Device.PermissionDenied"
 
 static void
 prop_set_auth_done_cb (NMAuthChain *chain,
@@ -3783,8 +3784,11 @@ prop_set_auth_done_cb (NMAuthChain *chain,
 	DBusConnection *dbus_connection;
 	NMAuthCallResult result;
 	DBusMessage *reply, *request;
-	const char *permission, *prop;
+	const char *permission, *prop, *objpath;
 	gboolean set_enabled = TRUE;
+	gboolean is_device = FALSE;
+	size_t objpath_len;
+	size_t devpath_len;
 
 	priv->auth_chains = g_slist_remove (priv->auth_chains, chain);
 
@@ -3792,9 +3796,16 @@ prop_set_auth_done_cb (NMAuthChain *chain,
 	permission = nm_auth_chain_get_data (chain, "permission");
 	prop = nm_auth_chain_get_data (chain, "prop");
 	set_enabled = GPOINTER_TO_UINT (nm_auth_chain_get_data (chain, "enabled"));
+	objpath = nm_auth_chain_get_data (chain, "objectpath");
+
+	objpath_len = strlen (objpath);
+	devpath_len = strlen (NM_DBUS_PATH "/Devices");
+	if (   strncmp (objpath, NM_DBUS_PATH "/Devices", devpath_len) == 0
+	    && objpath_len > devpath_len)
+		is_device = TRUE;
 
 	if (error) {
-		reply = dbus_message_new_error (request, PERM_DENIED_ERROR,
+		reply = dbus_message_new_error (request, is_device ? DEV_PERM_DENIED_ERROR : NM_PERM_DENIED_ERROR,
 		                                "Not authorized to perform this operation");
 	} else {
 		/* Caller has had a chance to obtain authorization, so we only need to
@@ -3802,10 +3813,16 @@ prop_set_auth_done_cb (NMAuthChain *chain,
 		 */
 		result = GPOINTER_TO_UINT (nm_auth_chain_get_data (chain, permission));
 		if (result != NM_AUTH_CALL_RESULT_YES) {
-			reply = dbus_message_new_error (request, PERM_DENIED_ERROR,
-				                            "Not authorized to perform this operation");
+			reply = dbus_message_new_error (request, is_device ? DEV_PERM_DENIED_ERROR : NM_PERM_DENIED_ERROR,
+			                                "Not authorized to perform this operation");
 		} else {
-			g_object_set (self, prop, set_enabled, NULL);
+			if (is_device) {
+				/* Find the device */
+				NMDevice *device = nm_manager_get_device_by_path (self, objpath);
+				if (device)
+					g_object_set (device, prop, set_enabled, NULL);
+			} else
+				g_object_set (self, prop, set_enabled, NULL);
 			reply = dbus_message_new_method_return (request);
 		}
 	}
@@ -3834,6 +3851,7 @@ prop_filter (DBusConnection *connection,
 	const char *propiface = NULL;
 	const char *propname = NULL;
 	const char *sender = NULL;
+	const char *objpath = NULL;
 	const char *glib_propname = NULL, *permission = NULL;
 	DBusError dbus_error;
 	gulong uid = G_MAXULONG;
@@ -3855,7 +3873,7 @@ prop_filter (DBusConnection *connection,
 	if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_STRING)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 	dbus_message_iter_get_basic (&iter, &propiface);
-	if (!propiface || strcmp (propiface, NM_DBUS_INTERFACE))
+	if (!propiface || (strcmp (propiface, NM_DBUS_INTERFACE) && strcmp (propiface, NM_DBUS_INTERFACE_DEVICE)))
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 	dbus_message_iter_next (&iter);
 
@@ -3874,6 +3892,9 @@ prop_filter (DBusConnection *connection,
 	} else if (!strcmp (propname, "WimaxEnabled")) {
 		glib_propname = NM_MANAGER_WIMAX_ENABLED;
 		permission = NM_AUTH_PERMISSION_ENABLE_DISABLE_WIMAX;
+	} else if (!strcmp (propname, "Autoconnect")) {
+		glib_propname = NM_DEVICE_AUTOCONNECT;
+		permission = NM_AUTH_PERMISSION_NETWORK_CONTROL;
 	} else
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
@@ -3887,15 +3908,22 @@ prop_filter (DBusConnection *connection,
 
 	sender = dbus_message_get_sender (message);
 	if (!sender) {
-		reply = dbus_message_new_error (message, PERM_DENIED_ERROR,
+		reply = dbus_message_new_error (message, NM_PERM_DENIED_ERROR,
 		                                "Could not determine D-Bus requestor");
 		goto out;
 	}
 
+	objpath = dbus_message_get_path (message);
+	if (!objpath) {
+		reply = dbus_message_new_error (message, NM_PERM_DENIED_ERROR,
+		                                "Could not determine D-Bus object path");
+		goto out;
+	}
+
 	dbus_error_init (&dbus_error);
 	uid = dbus_bus_get_unix_user (connection, sender, &dbus_error);
 	if (dbus_error_is_set (&dbus_error)) {
-		reply = dbus_message_new_error (message, PERM_DENIED_ERROR,
+		reply = dbus_message_new_error (message, NM_PERM_DENIED_ERROR,
 		                                "Could not determine the user ID of the requestor");
 		dbus_error_free (&dbus_error);
 		goto out;
@@ -3910,6 +3938,7 @@ prop_filter (DBusConnection *connection,
 		nm_auth_chain_set_data (chain, "permission", g_strdup (permission), g_free);
 		nm_auth_chain_set_data (chain, "enabled", GUINT_TO_POINTER (set_enabled), NULL);
 		nm_auth_chain_set_data (chain, "message", dbus_message_ref (message), (GDestroyNotify) dbus_message_unref);
+		nm_auth_chain_set_data (chain, "objectpath", g_strdup (objpath), g_free);
 		nm_auth_chain_add_call (chain, permission, TRUE);
 	} else {
 		/* Yay for root */
-- 
1.7.7.6
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]