gnome-keyring r1634 - in trunk: . pkcs11/gck pkcs11/user-store

[nnielsen svn gnome org]

Author: nnielsen
Date: Sat Feb 28 03:05:56 2009
New Revision: 1634
URL: http://svn.gnome.org/viewvc/gnome-keyring?rev=1634&view=rev

Log:
Fix problems loading private objects prematurely from the user store
without first logging
in.

Modified:
   trunk/ChangeLog
   trunk/pkcs11/gck/gck-data-file.c
   trunk/pkcs11/user-store/gck-user-storage.c

Modified: trunk/pkcs11/gck/gck-data-file.c
==============================================================================
--- trunk/pkcs11/gck/gck-data-file.c	(original)
+++ trunk/pkcs11/gck/gck-data-file.c	Sat Feb 28 03:05:56 2009
@@ -64,8 +64,7 @@
 	guint sections;
 	gboolean incomplete;
 	
-	/* Stuff added/notseen on this read */
-	GHashTable *added;
+	/* Stuff notseen on this read */
 	GHashTable *checks;
 };
 
@@ -535,6 +534,7 @@
 {
 	GHashTable *attributes;
 	const gchar *identifier;
+	gboolean added;
 	CK_ATTRIBUTE_PTR at;
 	CK_ATTRIBUTE attr;
 	gpointer key, value;
@@ -557,6 +557,8 @@
 
 	for (i = 0; i < n_entries; ++i) {
 		
+		added = FALSE;
+		
 		/* The attributes */
 		if (!egg_buffer_get_string (buffer, *offset, offset, &str, (EggBufferAllocator)g_realloc))
 			return GCK_DATA_FAILURE;
@@ -571,6 +573,7 @@
 		
 		/* Lookup or create a new table for it */
 		if (!g_hash_table_lookup_extended (entries, str, &key, &value)) {
+			added = TRUE;
 			value = attributes_new ();
 			key = g_strdup (str);
 			g_hash_table_replace (entries, key, value);
@@ -598,11 +601,15 @@
 				
 			at = attribute_dup (&attr);
 			g_hash_table_replace (attributes, &(at->type), at);
-			
+
 			/* Only emit the changed signal if we haven't just added this one */
-			if (!g_hash_table_lookup (self->added, identifier))
+			if (added == FALSE)
 				g_signal_emit (self, signals[ENTRY_CHANGED], 0, identifier, attr.type);
 		}
+		
+		/* A new entry was loaded */
+		if (added == TRUE)
+			g_signal_emit (self, signals[ENTRY_ADDED], 0, identifier);
 	}
 
 	return GCK_DATA_SUCCESS;
@@ -702,10 +709,18 @@
 
 	if (!g_hash_table_remove (self->identifiers, key))
 		g_assert_not_reached ();
-	if (entries != NULL && !g_hash_table_remove (entries, key))
-		g_return_if_reached ();
 	
-	g_signal_emit (self, signals[ENTRY_REMOVED], 0, key);
+	if (entries != NULL) {
+		if (!g_hash_table_remove (entries, key))
+			g_return_if_reached ();
+		
+		/* 
+		 * Note that we only fire the removed signal when the identifier 
+		 * was accessible. We don't fire removed for private items in 
+		 * a locked file.
+		 */
+		g_signal_emit (self, signals[ENTRY_REMOVED], 0, key);
+	}
 }
 
 static GckDataResult
@@ -738,11 +753,6 @@
 			break;
 		}
 		
-		/* Lookup the section it's currently in */
-		section = GPOINTER_TO_UINT (g_hash_table_lookup (self->identifiers, identifier));
-		if (section == 0 || section != value) 
-			g_hash_table_replace (self->added, g_strdup (identifier), UNUSED_VALUE);
-
 		section = value;
 		g_hash_table_replace (self->identifiers, identifier, GUINT_TO_POINTER (section));
 		
@@ -988,13 +998,6 @@
 }
 
 static void
-emit_each_added_identifier (gpointer key, gpointer value, gpointer data)
-{
-	GckDataFile *self = GCK_DATA_FILE (data);
-	g_signal_emit (self, signals[ENTRY_ADDED], 0, key);
-}
-
-static void
 dump_attributes (gpointer key, gpointer value, gpointer user_data)
 {
 	CK_ATTRIBUTE_PTR attr = value;
@@ -1047,7 +1050,6 @@
 	
 	self->unknowns = NULL;
 	
-	self->added = NULL;
 	self->checks = NULL;
 }
 
@@ -1060,7 +1062,6 @@
 	g_hash_table_destroy (self->identifiers);
 	self->identifiers = NULL;
 	
-	g_assert (self->added == NULL);
 	g_assert (self->checks == NULL);
 	
 	g_assert (self->publics);
@@ -1143,7 +1144,6 @@
 
 	/* Reads are not reentrant for a single data file */
 	g_return_val_if_fail (self->checks == NULL, GCK_DATA_FAILURE);
-	g_return_val_if_fail (self->added == NULL, GCK_DATA_FAILURE);
 
 	self->sections = 0;
 
@@ -1153,18 +1153,9 @@
 
 	/* Setup a hash table to monitor the actual data read */
 	self->checks = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
-	self->added = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 	g_hash_table_foreach (self->identifiers, copy_each_identifier, self->checks);
 	
 	res = parse_file_blocks (fd, update_from_any_block, login, self);
-	
-	/* 
-	 * Always need to fire off added signals for all identifiers 
-	 * whether (partially) successful or not.
-	 */
-
-	g_hash_table_foreach (self->added, emit_each_added_identifier, self);
-
 	if (res == GCK_DATA_SUCCESS) {
 		
 		/* Our last read was a success, can write */
@@ -1188,8 +1179,7 @@
 	}
 	
 	g_hash_table_destroy (self->checks);
-	g_hash_table_destroy (self->added);
-	self->checks = self->added = NULL;
+	self->checks = NULL;
 	
 	return res;
 }

Modified: trunk/pkcs11/user-store/gck-user-storage.c
==============================================================================
--- trunk/pkcs11/user-store/gck-user-storage.c	(original)
+++ trunk/pkcs11/user-store/gck-user-storage.c	Sat Feb 28 03:05:56 2009
@@ -507,7 +507,6 @@
 {
 	GError *error = NULL;
 	GckObject *object;
-	GckDataResult res;
 	gboolean ret;
 	guchar *data;
 	gsize n_data;
@@ -553,26 +552,12 @@
 	g_return_if_fail (GCK_SERIALIZABLE_GET_INTERFACE (object)->extension);
 
 	/* And load the data into it */
-	res = gck_serializable_load (GCK_SERIALIZABLE (object), self->login, data, n_data);
-	g_free (data);
-	
-	switch (res) {
-	case GCK_DATA_FAILURE:
-		g_message ("failed to load file in user store: %s", identifier);
-		return;
-	case GCK_DATA_UNRECOGNIZED:
-		g_message ("invalid or unparsable file in user store: %s", identifier);
-		return;
-	case GCK_DATA_LOCKED:
-		g_message ("file is locked with unknown password: %s", identifier);
-		return;
-	case GCK_DATA_SUCCESS:
+	if (gck_serializable_load (GCK_SERIALIZABLE (object), self->login, data, n_data)) 
 		take_object_ownership (self, identifier, object);
-		break;
-	default:
-		g_assert_not_reached ();
-	}
+	else 
+		g_message ("failed to load file in user store: %s", identifier);
 	
+	g_free (data);
 	g_object_unref (object);
 }
 
@@ -613,11 +598,9 @@
 {
 	GError *error = NULL;
 	GckObject *object;
-	GckDataResult res;
 	guchar *data;
 	gsize n_data;
 	GType type;
-	CK_RV rv;
 	
 	g_assert (GCK_IS_USER_STORAGE (self));
 	g_assert (GCK_IS_TRANSACTION (transaction));
@@ -660,59 +643,28 @@
 	}
 	
 	/* Load it into our temporary object */
-	res = gck_serializable_load (GCK_SERIALIZABLE (object), old_login, data, n_data);
-	g_free (data);
-	
-	switch (res) {
-	case GCK_DATA_FAILURE:
-	case GCK_DATA_UNRECOGNIZED:
+	if (!gck_serializable_load (GCK_SERIALIZABLE (object), old_login, data, n_data)) {
 		g_message ("unrecognized or invalid user store file: %s", identifier);
-		rv = CKR_FUNCTION_FAILED;
-		break;
-	case GCK_DATA_LOCKED:
-		g_message ("old login is invalid for user store file: %s", identifier);
-		rv = CKR_PIN_INCORRECT;
-		break;
-	case GCK_DATA_SUCCESS:
-		rv = CKR_OK;
-		break;
-	default:
-		g_assert_not_reached ();
-	}
-	
-	if (rv != CKR_OK) {
-		gck_transaction_fail (transaction, rv);
+		gck_transaction_fail (transaction, CKR_FUNCTION_FAILED);
+		g_free (data);
 		g_object_unref (object);
 		return;
-	}
+	} 
+	
+	g_free (data);
+	data = NULL;
 		
 	/* Read it out of our temporary object */
-	res = gck_serializable_save (GCK_SERIALIZABLE (object), new_login, &data, &n_data);
-	g_object_unref (object);
-	
-	switch (res) {
-	case GCK_DATA_FAILURE:
-	case GCK_DATA_UNRECOGNIZED:
+	if (!gck_serializable_save (GCK_SERIALIZABLE (object), new_login, &data, &n_data)) {
 		g_warning ("unable to serialize data with new login: %s", identifier);
-		rv = CKR_GENERAL_ERROR;
-		break;
-	case GCK_DATA_LOCKED:
-		g_message ("new login is invalid for user store file: %s", identifier);
-		rv = CKR_PIN_INVALID;
-		break;
-	case GCK_DATA_SUCCESS:
-		rv = CKR_OK;
-		break;
-	default:
-		g_assert_not_reached ();
-	}
-
-	if (rv != CKR_OK) {
-		gck_transaction_fail (transaction, rv);
+		gck_transaction_fail (transaction, CKR_GENERAL_ERROR);
+		g_object_unref (object);
 		g_free (data);
 		return;
 	}
 	
+	g_object_unref (object);
+	
 	/* And write it back out to the file */
 	gck_transaction_write_file (transaction, path, data, n_data);
 	
@@ -1157,8 +1109,7 @@
 	}
 	
 	/* Serialize the object in question */
-	res = gck_serializable_save (GCK_SERIALIZABLE (object), is_private ? self->login : NULL, &data, &n_data);
-	if (res != GCK_DATA_SUCCESS) {
+	if (!gck_serializable_save (GCK_SERIALIZABLE (object), is_private ? self->login : NULL, &data, &n_data)) {
 		gck_transaction_fail (transaction, CKR_FUNCTION_FAILED);
 		g_return_if_reached ();
 	}