GNOME.org
 

[network-manager-applet] core: fix clearing secrets when updating connections

commit ebfd3cdb9d4aba32df24b7f09e32d939bcc64d36
Author: Dan Williams <dcbw redhat com>
Date:   Tue Feb 9 23:24:08 2010 -0800

    core: fix clearing secrets when updating connections
    
    It was never a problem for wifi/wired or DSL since their various
    configurations require valid secrets before you can hit Apply.  But
    for Mobile Broadband PINs the libnm-glib settings interface
    refactor broke clearing secrets so you could never remove a PIN
    from a 3G connection after you added one.
    
    The problem was that empty keys were never deleted from the keyring
    when updating the connection.  Make that happen.  But we have to be
    careful because sometimes in the applet the connection won't have
    secrets when it gets updated (timestamps, seen BSSIDs), so we don't
    want to blow away keyring items we simply haven't asked for yet in
    those two cases.

 src/applet-device-wifi.c                 |   30 +++++--------
 src/applet.c                             |   22 +++-------
 src/gconf-helpers/gconf-helpers.c        |   70 +++++++++++++++++++++++++++--
 src/gconf-helpers/gconf-helpers.h        |    3 +-
 src/gconf-helpers/gconf-upgrade.c        |    2 +-
 src/gconf-helpers/nma-gconf-connection.c |   25 ++++++++---
 src/gconf-helpers/nma-gconf-connection.h |    3 +
 7 files changed, 107 insertions(+), 48 deletions(-)
---
diff --git a/src/applet-device-wifi.c b/src/applet-device-wifi.c
index b24ff80..48045f2 100644
--- a/src/applet-device-wifi.c
+++ b/src/applet-device-wifi.c
@@ -820,31 +820,26 @@ out:
 	g_slist_free (connections);
 }
 
-static gboolean
+static void
 add_seen_bssid (NMSettingsConnectionInterface *connection, NMAccessPoint *ap)
 {
 	NMSettingWireless *s_wireless;
 	const char *bssid;
 
+	if (!NMA_GCONF_CONNECTION (connection))
+		return;
+
 	s_wireless = NM_SETTING_WIRELESS (nm_connection_get_setting (NM_CONNECTION (connection), NM_TYPE_SETTING_WIRELESS));
 	if (!s_wireless)
-		return FALSE;
+		return;
 
 	bssid = nm_access_point_get_hw_address (ap);
 	if (!bssid || !utils_ether_addr_valid (ether_aton (bssid)))
-		return FALSE;
-
-	return nm_setting_wireless_add_seen_bssid (s_wireless, bssid);
-}
+		return;
 
-static void
-bssid_update_cb (NMSettingsConnectionInterface *connection,
-                 GError *error,
-                 gpointer user_data)
-{
-	if (error) {
-		g_warning ("%s: failed to update connection seen BSSIDs: (%d) %s",
-		           __func__, error->code, error->message);
+	if (nm_setting_wireless_add_seen_bssid (s_wireless, bssid)) {
+		/* Ignore secrets since we don't have any here and we're just adding a BSSID */
+		nma_gconf_connection_update (NMA_GCONF_CONNECTION (connection), TRUE);
 	}
 }
 
@@ -877,8 +872,7 @@ notify_active_ap_changed_cb (NMDeviceWifi *device,
 	if (!ssid || !nm_utils_same_ssid (nm_setting_wireless_get_ssid (s_wireless), ssid, TRUE))
 		return;
 
-	if (add_seen_bssid (connection, new))
-		nm_settings_connection_interface_update (connection, bssid_update_cb, NULL);
+	add_seen_bssid (connection, new);
 
 	applet_schedule_update_icon (applet);
 }
@@ -1211,8 +1205,8 @@ wireless_device_state_changed (NMDevice *device,
 
 		/* Save this BSSID to seen-bssids list */
 		connection = applet_get_exported_connection_for_device (device, applet);
-		if (connection && add_seen_bssid (connection, new))
-			nm_settings_connection_interface_update (connection, bssid_update_cb, NULL);
+		if (connection)
+			add_seen_bssid (connection, new);
 	}
 
 	msg = g_strdup_printf (_("You are now connected to the wireless network '%s'."),
diff --git a/src/applet.c b/src/applet.c
index d69f6f8..10a8b28 100644
--- a/src/applet.c
+++ b/src/applet.c
@@ -762,18 +762,6 @@ applet_is_any_vpn_activating (NMApplet *applet)
 	}
 	return FALSE;
 }
-static void
-save_timestamp_cb (NMSettingsConnectionInterface *connection,
-                   GError *error,
-                   gpointer user_data)
-{
-	if (error) {
-		g_warning ("Error saving connection %s timestamp: (%d) %s",
-		           nm_connection_get_path (NM_CONNECTION (connection)),
-		           error->code,
-		           error->message);
-	}
-}
 
 static void
 update_connection_timestamp (NMActiveConnection *active,
@@ -788,14 +776,15 @@ update_connection_timestamp (NMActiveConnection *active,
 
 	gconf_connection = nm_settings_interface_get_connection_by_path (NM_SETTINGS_INTERFACE (applet->gconf_settings),
 	                                                                 nm_connection_get_path (connection));
-	if (!gconf_connection)
+	if (!gconf_connection || !NMA_IS_GCONF_CONNECTION (gconf_connection))
 		return;
 
 	s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION));
 	g_assert (s_con);
 
 	g_object_set (s_con, NM_SETTING_CONNECTION_TIMESTAMP, (guint64) time (NULL), NULL);
-	nm_settings_connection_interface_update (gconf_connection, save_timestamp_cb, NULL);
+	/* Ignore secrets since we're just updating the timestamp */
+	nma_gconf_connection_update (NMA_GCONF_CONNECTION (gconf_connection), TRUE);
 }
 
 static char *
@@ -2620,7 +2609,7 @@ periodic_update_active_connection_timestamps (gpointer user_data)
 
 		path = nm_active_connection_get_connection (active);
 		connection = nm_settings_interface_get_connection_by_path (NM_SETTINGS_INTERFACE (applet->gconf_settings), path);
-		if (!connection)
+		if (!connection || !NMA_IS_GCONF_CONNECTION (connection))
 			continue;
 
 		devices = nm_active_connection_get_devices (active);
@@ -2640,7 +2629,8 @@ periodic_update_active_connection_timestamps (gpointer user_data)
 				g_assert (s_con);
 
 				g_object_set (s_con, NM_SETTING_CONNECTION_TIMESTAMP, (guint64) time (NULL), NULL);
-				nm_settings_connection_interface_update (connection, save_timestamp_cb, NULL);
+				/* Ignore secrets since we're just updating the timestamp */
+				nma_gconf_connection_update (NMA_GCONF_CONNECTION (connection), TRUE);
 				break;
 			}
 		}
diff --git a/src/gconf-helpers/gconf-helpers.c b/src/gconf-helpers/gconf-helpers.c
index a299ccc..3170e68 100644
--- a/src/gconf-helpers/gconf-helpers.c
+++ b/src/gconf-helpers/gconf-helpers.c
@@ -1866,6 +1866,48 @@ nm_gconf_add_keyring_item (const char *connection_uuid,
 	g_free (display_name);
 }
 
+static void
+delete_done (GnomeKeyringResult result, gpointer user_data)
+{
+}
+
+static void
+keyring_delete_item (const char *connection_uuid,
+                     const char *setting_name,
+                     const char *setting_key)
+{
+	GList *found_list = NULL;
+	GnomeKeyringResult ret;
+	GList *iter;
+
+	pre_keyring_callback ();
+
+	ret = gnome_keyring_find_itemsv_sync (GNOME_KEYRING_ITEM_GENERIC_SECRET,
+	                                      &found_list,
+	                                      KEYRING_UUID_TAG,
+	                                      GNOME_KEYRING_ATTRIBUTE_TYPE_STRING,
+	                                      connection_uuid,
+	                                      KEYRING_SN_TAG,
+	                                      GNOME_KEYRING_ATTRIBUTE_TYPE_STRING,
+	                                      setting_name,
+	                                      KEYRING_SK_TAG,
+	                                      GNOME_KEYRING_ATTRIBUTE_TYPE_STRING,
+	                                      setting_key,
+	                                      NULL);
+	if (ret == GNOME_KEYRING_RESULT_OK) {
+		for (iter = found_list; iter != NULL; iter = g_list_next (iter)) {
+			GnomeKeyringFound *found = (GnomeKeyringFound *) iter->data;
+
+			gnome_keyring_item_delete (found->keyring,
+			                           found->item_id,
+			                           delete_done,
+			                           NULL,
+			                           NULL);
+		}
+		gnome_keyring_found_list_free (found_list);
+	}
+}
+
 typedef struct CopyOneSettingValueInfo {
 	NMConnection *connection;
 	GConfClient *client;
@@ -1912,6 +1954,14 @@ write_one_secret_to_keyring (NMSetting *setting,
 		                           setting_name,
 		                           key,
 		                           secret);
+	} else {
+		/* We have to be careful about this, since if the connection we're
+		 * given doesn't include secrets we'll blow anything in the keyring
+		 * away here.  We rely on the caller knowing whether or not to do this.
+		 */
+		keyring_delete_item (info->connection_uuid,
+		                     setting_name,
+		                     key);
 	}
 }
 
@@ -2469,7 +2519,8 @@ remove_leftovers (CopyOneSettingValueInfo *info)
 void
 nm_gconf_write_connection (NMConnection *connection,
                            GConfClient *client,
-                           const char *dir)
+                           const char *dir,
+                           gboolean ignore_secrets)
 {
 	NMSettingConnection *s_con;
 	CopyOneSettingValueInfo info;
@@ -2488,15 +2539,24 @@ nm_gconf_write_connection (NMConnection *connection,
 	info.dir = dir;
 	info.connection_uuid = nm_setting_connection_get_uuid (s_con);
 	info.connection_name = nm_setting_connection_get_id (s_con);
+
 	nm_connection_for_each_setting_value (connection,
 	                                      copy_one_setting_value_to_gconf,
 	                                      &info);
 	remove_leftovers (&info);
 
-	/* write secrets */
-	nm_connection_for_each_setting_value (connection,
-	                                      write_one_secret_to_keyring,
-	                                      &info);
+	/* Write/clear secrets; the caller must know whether or not to do this
+	 * based on how the connection was updated; if only something like the
+	 * BSSID or timestamp is getting updated, then you want to ignore
+	 * secrets, since the secrets could not possibly have changed.  On the
+	 * other hand, if the user cleared out a secret in the connection editor,
+	 * you want to ensure that secret gets deleted from the keyring.
+	 */
+	if (ignore_secrets == FALSE) {
+		nm_connection_for_each_setting_value (connection,
+		                                      write_one_secret_to_keyring,
+		                                      &info);
+	}
 
 	/* Update ignore CA cert status */
 	ignore = GPOINTER_TO_UINT (g_object_get_data (G_OBJECT (connection), IGNORE_CA_CERT_TAG));
diff --git a/src/gconf-helpers/gconf-helpers.h b/src/gconf-helpers/gconf-helpers.h
index 17d1ce8..942acc8 100644
--- a/src/gconf-helpers/gconf-helpers.h
+++ b/src/gconf-helpers/gconf-helpers.h
@@ -241,7 +241,8 @@ nm_gconf_read_connection (GConfClient *client,
 void
 nm_gconf_write_connection (NMConnection *connection,
                            GConfClient *client,
-                           const char *dir);
+                           const char *dir,
+                           gboolean ignore_secrets);
 
 void
 nm_gconf_add_keyring_item (const char *connection_uuid,
diff --git a/src/gconf-helpers/gconf-upgrade.c b/src/gconf-helpers/gconf-upgrade.c
index 0651a37..5e51cd9 100644
--- a/src/gconf-helpers/gconf-upgrade.c
+++ b/src/gconf-helpers/gconf-upgrade.c
@@ -836,7 +836,7 @@ nm_gconf_write_0_6_connection (NMConnection *connection, GConfClient *client, in
 	char *dir;
 
 	dir = g_strdup_printf ("%s/%d", GCONF_PATH_CONNECTIONS, n);
-	nm_gconf_write_connection (connection, client, dir);
+	nm_gconf_write_connection (connection, client, dir, FALSE);
 	g_free (dir);
 }
 
diff --git a/src/gconf-helpers/nma-gconf-connection.c b/src/gconf-helpers/nma-gconf-connection.c
index da76a9b..753cc96 100644
--- a/src/gconf-helpers/nma-gconf-connection.c
+++ b/src/gconf-helpers/nma-gconf-connection.c
@@ -426,6 +426,20 @@ clear_keyring_items (NMAGConfConnection *self)
 	}
 }
 
+void
+nma_gconf_connection_update (NMAGConfConnection *self,
+                             gboolean ignore_secrets)
+{
+	NMAGConfConnectionPrivate *priv = NMA_GCONF_CONNECTION_GET_PRIVATE (self);
+
+	nm_gconf_write_connection (NM_CONNECTION (self),
+	                           priv->client,
+	                           priv->dir,
+	                           ignore_secrets);
+	gconf_client_notify (priv->client, priv->dir);
+	gconf_client_suggest_sync (priv->client, NULL);
+}
+
 /******************************************************/
 
 static gboolean
@@ -433,13 +447,10 @@ update (NMSettingsConnectionInterface *connection,
 	    NMSettingsConnectionInterfaceUpdateFunc callback,
 	    gpointer user_data)
 {
-	NMAGConfConnectionPrivate *priv = NMA_GCONF_CONNECTION_GET_PRIVATE (connection);
-
-	nm_gconf_write_connection (NM_CONNECTION (connection),
-	                           priv->client,
-	                           priv->dir);
-	gconf_client_notify (priv->client, priv->dir);
-	gconf_client_suggest_sync (priv->client, NULL);
+	/* Always update secrets since it's assumed that secrets are included in
+	 * the new connection data.
+	 */
+	nma_gconf_connection_update (NMA_GCONF_CONNECTION (connection), FALSE);
 
 	return parent_settings_connection_iface->update (connection, callback, user_data);
 }
diff --git a/src/gconf-helpers/nma-gconf-connection.h b/src/gconf-helpers/nma-gconf-connection.h
index 224eb7e..9571c38 100644
--- a/src/gconf-helpers/nma-gconf-connection.h
+++ b/src/gconf-helpers/nma-gconf-connection.h
@@ -74,6 +74,9 @@ gboolean nma_gconf_connection_gconf_changed (NMAGConfConnection *self);
 
 const char *nma_gconf_connection_get_gconf_path (NMAGConfConnection *self);
 
+void nma_gconf_connection_update (NMAGConfConnection *self,
+                                  gboolean ignore_secrets);
+
 G_END_DECLS
 
 #endif /* NMA_GCONF_CONNECTION_H */
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]