[gnome-keyring/dbus-api] [dbus] Fix problems with CreateItem and secret transfer.

[Stefan Walter <stefw src gnome org>]

commit 1d544c5692fd375eba88f3870a8c73e62420242f
Author: Stef Walter <stef memberwebs com>
Date:   Fri Dec 11 01:35:52 2009 +0000

    [dbus] Fix problems with CreateItem and secret transfer.
    
    Rework how CreateItem works and fix some problems with
    transferring of secrets.

 daemon/data/introspect-session.xml |    6 +-
 daemon/dbus/gkd-secret-objects.c   |  105 +++++++++++++++++++++++++-----------
 daemon/dbus/gkd-secret-secret.c    |    8 ++-
 3 files changed, 81 insertions(+), 38 deletions(-)
---
diff --git a/daemon/data/introspect-session.xml b/daemon/data/introspect-session.xml
index 1d777b5..5235947 100644
--- a/daemon/data/introspect-session.xml
+++ b/daemon/data/introspect-session.xml
@@ -19,15 +19,15 @@
 		</method>
 		<method name="GetSecret">
 			<arg name="item" type="o" direction="in"/>
-			<arg name="secret" type="(sayay)" direction="out"/>
+			<arg name="secret" type="(oayay)" direction="out"/>
 		</method>
 		<method name="SetSecret">
 			<arg name="item" type="o" direction="in"/>
-			<arg name="secret" type="(sayay)" direction="in"/>
+			<arg name="secret" type="(oayay)" direction="in"/>
 		</method>
 		<method name="GetSecrets">
 			<arg name="items" type="ao" direction="in"/>
-			<arg name="secrets" type="a{o(sayay)}" direction="out"/>
+			<arg name="secrets" type="a{o(oayay)}" direction="out"/>
 		</method>
 	</interface>
 
diff --git a/daemon/dbus/gkd-secret-objects.c b/daemon/dbus/gkd-secret-objects.c
index 815b0af..c815bce 100644
--- a/daemon/dbus/gkd-secret-objects.c
+++ b/daemon/dbus/gkd-secret-objects.c
@@ -556,33 +556,53 @@ collection_find_matching_item (GkdSecretObjects *self, GP11Object *coll, GP11Att
 static DBusMessage*
 collection_method_create_item (GkdSecretObjects *self, GP11Object *object, DBusMessage *message)
 {
+	GP11Session *pkcs11_session = NULL;
+	DBusError derr = DBUS_ERROR_INIT;
+	GkdSecretSecret *secret = NULL;
+	GkdSecretSession *session;
 	dbus_bool_t replace = FALSE;
-	GP11Attributes *attrs;
+	GP11Attributes *attrs = NULL;
 	GP11Attribute *fields;
 	DBusMessageIter iter, array;
 	GP11Object *item = NULL;
 	const gchar *prompt;
 	GError *error = NULL;
-	GP11Session *session;
-	DBusMessage *reply;
+	DBusMessage *reply = NULL;
 	gchar *path = NULL;
 	gchar *identifier;
+	gboolean created = FALSE;
 
 	/* Parse the message */
-	if (!dbus_message_has_signature (message, "a{sv}b"))
+	if (!dbus_message_has_signature (message, "a{sv}(oayay)b"))
 		return NULL;
 	if (!dbus_message_iter_init (message, &iter))
 		g_return_val_if_reached (NULL);
 	attrs = gp11_attributes_new ();
 	dbus_message_iter_recurse (&iter, &array);
 	if (!gkd_secret_property_parse_all (&array, attrs)) {
-		gp11_attributes_unref (attrs);
-		return dbus_message_new_error_printf (message, DBUS_ERROR_INVALID_ARGS,
-		                                      "Invalid properties");
+		reply = dbus_message_new_error (message, DBUS_ERROR_INVALID_ARGS,
+		                                "Invalid properties argument");
+		goto cleanup;
+	}
+	dbus_message_iter_next (&iter);
+	secret = gkd_secret_secret_parse (&iter);
+	if (secret == NULL) {
+		reply = dbus_message_new_error (message, DBUS_ERROR_INVALID_ARGS,
+		                                "Invalid secret argument");
+		goto cleanup;
 	}
 	dbus_message_iter_next (&iter);
 	dbus_message_iter_get_basic (&iter, &replace);
 
+	/* Figure out which session we're dealing with */
+	session = gkd_secret_service_lookup_session (self->service, secret->path,
+	                                             dbus_message_get_sender (message));
+	if (session == NULL) {
+		reply = dbus_message_new_error (message, SECRET_ERROR_NO_SESSION,
+		                                "No such secret session exists");
+		goto cleanup;
+	}
+
 	if (replace) {
 		fields = gp11_attributes_find (attrs, CKA_G_FIELDS);
 		if (fields)
@@ -591,47 +611,68 @@ collection_method_create_item (GkdSecretObjects *self, GP11Object *object, DBusM
 
 	/* Replace the item */
 	if (item) {
-		if (!gp11_object_set_full (item, attrs, NULL, &error)) {
-			g_object_unref (item);
-			item = NULL;
-		}
+		if (!gp11_object_set_full (item, attrs, NULL, &error))
+			goto cleanup;
 
 	/* Create a new item */
 	} else {
-		session = gp11_object_get_session (object);
-		g_return_val_if_fail (session, NULL);
+		pkcs11_session = gp11_object_get_session (object);
+		g_return_val_if_fail (pkcs11_session, NULL);
 		identifier = gkd_secret_util_identifier_for_collection (object);
 		g_return_val_if_fail (identifier, NULL);
-		gp11_attributes_add_ulong (attrs, CKA_CLASS, CKO_SECRET_KEY);
 		gp11_attributes_add_string (attrs, CKA_G_COLLECTION, identifier);
-		item = gp11_session_create_object_full (session, attrs, NULL, &error);
 		g_free (identifier);
+		gp11_attributes_add_ulong (attrs, CKA_CLASS, CKO_SECRET_KEY);
+		item = gp11_session_create_object_full (pkcs11_session, attrs, NULL, &error);
+		if (item == NULL)
+			goto cleanup;
+		gp11_object_set_session (item, pkcs11_session);
+		created = TRUE;
 	}
 
-	/* Build up the item identifier */
-	if (error == NULL)
-		path = gkd_secret_util_path_for_item (item);
-
-	if (path != NULL) {
-		prompt = "/";
-		reply = dbus_message_new_method_return (message);
-		dbus_message_append_args (reply, DBUS_TYPE_OBJECT_PATH, &path,
-		                          DBUS_TYPE_OBJECT_PATH, &prompt,
-		                          DBUS_TYPE_INVALID);
+	/* Set the secret */
+	if (!gkd_secret_session_set_item_secret (session, item, secret, &derr)) {
+		if (created) /* If we created, then try to destroy on failure */
+			gp11_object_destroy (item, NULL);
+		goto cleanup;
+	}
 
-	} else {
-		if (error->code == CKR_USER_NOT_LOGGED_IN)
-			reply = dbus_message_new_error_printf (message, SECRET_ERROR_IS_LOCKED,
-			                                       "Cannot create an item in a locked collection");
-		else
-			reply = dbus_message_new_error_printf (message, DBUS_ERROR_FAILED,
-			                                       "Couldn't create item: %s", error->message);
+	/* Build up the item identifier */
+	path = gkd_secret_util_path_for_item (item);
+	g_return_val_if_fail (path, NULL);
+	prompt = "/";
+	reply = dbus_message_new_method_return (message);
+	dbus_message_append_args (reply, DBUS_TYPE_OBJECT_PATH, &path,
+	                          DBUS_TYPE_OBJECT_PATH, &prompt,
+	                          DBUS_TYPE_INVALID);
+
+cleanup:
+	if (error) {
+		if (!reply) {
+			if (error->code == CKR_USER_NOT_LOGGED_IN)
+				reply = dbus_message_new_error_printf (message, SECRET_ERROR_IS_LOCKED,
+				                                       "Cannot create an item in a locked collection");
+			else
+				reply = dbus_message_new_error_printf (message, DBUS_ERROR_FAILED,
+				                                       "Couldn't create item: %s", error->message);
+		}
 		g_clear_error (&error);
 	}
 
+	if (dbus_error_is_set (&derr)) {
+		if (!reply)
+			reply = dbus_message_new_error (message, derr.name, derr.message);
+		dbus_error_free (&derr);
+	}
+
+	gkd_secret_secret_free (secret);
+	gp11_attributes_unref (attrs);
 	if (item)
 		g_object_unref (item);
+	if (pkcs11_session)
+		g_object_unref (pkcs11_session);
 	g_free (path);
+
 	return reply;
 }
 
diff --git a/daemon/dbus/gkd-secret-secret.c b/daemon/dbus/gkd-secret-secret.c
index 7ac8385..d1202d9 100644
--- a/daemon/dbus/gkd-secret-secret.c
+++ b/daemon/dbus/gkd-secret-secret.c
@@ -46,7 +46,7 @@ gkd_secret_secret_create_and_take_memory (const gchar *path, gpointer parameter,
 GkdSecretSecret*
 gkd_secret_secret_parse (DBusMessageIter *iter)
 {
-	DBusMessageIter struc;
+	DBusMessageIter struc, array;
 	const void *parameter, *value;
 	int n_value, n_parameter;
 	const char *path;
@@ -64,14 +64,16 @@ gkd_secret_secret_parse (DBusMessageIter *iter)
 	    dbus_message_iter_get_arg_type (&struc) != DBUS_TYPE_ARRAY ||
 	    dbus_message_iter_get_element_type(&struc) != DBUS_TYPE_BYTE)
 		return NULL;
-	dbus_message_iter_get_fixed_array (&struc, &parameter, &n_parameter);
+	dbus_message_iter_recurse (&struc, &array);
+	dbus_message_iter_get_fixed_array (&array, &parameter, &n_parameter);
 
 	/* Get the value */
 	if (!dbus_message_iter_next (&struc) ||
 	    dbus_message_iter_get_arg_type (&struc) != DBUS_TYPE_ARRAY ||
 	    dbus_message_iter_get_element_type(&struc) != DBUS_TYPE_BYTE)
 		return NULL;
-	dbus_message_iter_get_fixed_array (&struc, &value, &n_value);
+	dbus_message_iter_recurse (&struc, &array);
+	dbus_message_iter_get_fixed_array (&array, &value, &n_value);
 
 	return gkd_secret_secret_create_and_take_memory (path,
 	                                                 n_parameter ? g_memdup (parameter, n_parameter) : NULL,