GNOME.org
 

[gnome-keyring] [login] Fix various issues storing and using auto unlock passwords.

commit d864698a290c55d1ccda5cc20946894ade5e827d
Author: Stef Walter <stef memberwebs com>
Date:   Sun Mar 21 15:55:51 2010 +0000

    [login] Fix various issues storing and using auto unlock passwords.
    
     * Unwrap secrets directly into login keyring  for auto unlock.
     * Fix various corner cases using auto unlock stuff in login keyring.

 daemon/dbus/gkd-secret-session.c |    2 +
 daemon/dbus/gkd-secret-unlock.c  |   51 +++++++--------
 daemon/login/gkd-login.c         |  129 ++++++++++++++++++++++++--------------
 daemon/login/gkd-login.h         |    7 +-
 4 files changed, 110 insertions(+), 79 deletions(-)
---
diff --git a/daemon/dbus/gkd-secret-session.c b/daemon/dbus/gkd-secret-session.c
index 80cd054..73551df 100644
--- a/daemon/dbus/gkd-secret-session.c
+++ b/daemon/dbus/gkd-secret-session.c
@@ -730,6 +730,8 @@ gkd_secret_session_create_credential (GkdSecretSession *self, GP11Session *sessi
 		}
 		g_clear_error (&error);
 		return NULL;
+	} else {
+		gp11_object_set_session (object, session);
 	}
 
 	return object;
diff --git a/daemon/dbus/gkd-secret-unlock.c b/daemon/dbus/gkd-secret-unlock.c
index 8a70ddc..f5df63b 100644
--- a/daemon/dbus/gkd-secret-unlock.c
+++ b/daemon/dbus/gkd-secret-unlock.c
@@ -247,43 +247,35 @@ check_locked_collection (GP11Object *collection, gboolean *locked)
 }
 
 static void
-attach_credential_to_login (GP11Object *collection, GP11Object *cred)
+attach_unlock_to_login (GP11Object *collection, GkdSecretSecret *master)
 {
-	GError *error = NULL;
+	DBusError derr = DBUS_ERROR_INIT;
 	GP11Attributes *attrs;
-	gpointer value;
-	gsize n_value;
+	GP11Object *cred;
 	gchar *location;
 	gchar *label;
 
 	g_assert (GP11_IS_OBJECT (collection));
-	g_assert (GP11_IS_OBJECT (cred));
 
+	/* Relevant information for the unlock item */
 	attrs = attributes_for_collection (collection);
 	g_return_if_fail (attrs);
-
 	location = location_string_for_attributes (attrs);
 	label = label_string_for_attributes (attrs);
 	gp11_attributes_unref (attrs);
 
-	value = gp11_object_get_data_full (cred, CKA_VALUE, egg_secure_realloc, NULL, &n_value, &error);
-	if (value) {
-		if (g_utf8_validate (value, n_value, NULL))
-			gkd_login_attach_secret (label, value, "keyring", location, NULL);
-		else
-			g_warning ("couldn't save non utf-8 unlock credentials in login keyring");
-		egg_secure_clear (value, n_value);
-		egg_secure_free (value);
-
-	} else {
-		if (!g_error_matches (error, GP11_ERROR, CKR_USER_NOT_LOGGED_IN))
-			g_warning ("couldn't read unlock credentials to save in login keyring: %s",
-			           egg_error_message (error));
-		g_clear_error (&error);
-	}
-
+	attrs = gkd_login_attach_make_attributes (label, "keyring", location, NULL);
 	g_free (location);
 	g_free (label);
+
+	cred = gkd_secret_session_create_credential (master->session, NULL, attrs, master, &derr);
+	gp11_attributes_unref (attrs);
+	g_object_unref (cred);
+
+	if (!cred) {
+		g_warning ("couldn't save unlock password in login collection: %s", derr.message);
+		dbus_error_free (&derr);
+	}
 }
 
 static void
@@ -304,6 +296,7 @@ authenticate_collection (GkdSecretUnlock *self, GP11Object *collection, gboolean
 	GP11Attribute *attr;
 	GP11Object *cred;
 	gboolean transient;
+	gboolean result;
 
 	g_assert (GKD_SECRET_IS_UNLOCK (self));
 	g_assert (GP11_IS_OBJECT (collection));
@@ -336,35 +329,37 @@ authenticate_collection (GkdSecretUnlock *self, GP11Object *collection, gboolean
 	}
 
 	cred = gkd_secret_session_create_credential (master->session, NULL, template, master, &derr);
-	gkd_secret_secret_free (master);
+	g_object_unref (cred);
 
 	if (cred) {
 		/* Save it to the login keyring */
 		if (!transient)
-			attach_credential_to_login (collection, cred);
-		g_object_unref (cred);
+			attach_unlock_to_login (collection, master);
 
 		/* Save away the unlock options for next time */
 		gp11_object_set_template (collection, CKA_G_CREDENTIAL_TEMPLATE, template, NULL);
 		gp11_attributes_unref (template);
 
 		*locked = FALSE;
-		return TRUE; /* Operation succeeded, and unlocked */
+		result = TRUE; /* Operation succeeded, and unlocked */
 
 	} else {
 		gp11_attributes_unref (template);
 		if (dbus_error_has_name (&derr, INTERNAL_ERROR_DENIED)) {
 			dbus_error_free (&derr);
 			*locked = TRUE;
-			return TRUE; /* Operation succeded, although not unlocked*/
+			result = TRUE; /* Operation succeded, although not unlocked*/
 
 		} else {
 			g_warning ("couldn't create credential for collection: %s",
 			           derr.message);
 			dbus_error_free (&derr);
-			return FALSE; /* Operation failed */
+			result = FALSE; /* Operation failed */
 		}
 	}
+
+	gkd_secret_secret_free (master);
+	return result;
 }
 
 /* -----------------------------------------------------------------------------
diff --git a/daemon/login/gkd-login.c b/daemon/login/gkd-login.c
index bdef57d..373561c 100644
--- a/daemon/login/gkd-login.c
+++ b/daemon/login/gkd-login.c
@@ -77,7 +77,9 @@ open_and_login_session (GP11Slot *slot, CK_USER_TYPE user_type, GError **error)
 	session = gp11_slot_open_session (slot, CKF_RW_SESSION, error);
 	if (session != NULL) {
 		if (!gp11_session_login (session, user_type, NULL, 0, error)) {
-			if ((*error)->code != CKR_USER_ALREADY_LOGGED_IN) {
+			if (g_error_matches (*error, GP11_ERROR, CKR_USER_ALREADY_LOGGED_IN)) {
+				g_clear_error (error);
+			} else {
 				g_object_unref (session);
 				session = NULL;
 			}
@@ -574,20 +576,88 @@ find_login_keyring_item (GP11Session *session, GP11Attribute *fields)
 	return item;
 }
 
+static GP11Attributes*
+attach_make_attributes_va (GP11Session *session, const gchar *label,
+                           const gchar *first, va_list va)
+{
+	GP11Attributes *attrs;
+	GP11Attribute fields;
+	gchar *display_name;
+	GP11Object* item;
+	GError *error = NULL;
+	gpointer value;
+	gsize n_value;
+
+	attrs = gp11_attributes_new ();
+
+	gp11_attribute_init_empty (&fields, CKA_G_FIELDS);
+	string_attribute_list_va (va, first, &fields);
+
+	/*
+	 * If there already is such an item, then include its identifier.
+	 * What this does is overwrite that item, rather than creating new.
+	 */
+	item = find_login_keyring_item (session, &fields);
+	if (item) {
+		value = gp11_object_get_data (item, CKA_ID, &n_value, &error);
+		if (value != NULL) {
+			gp11_attributes_add_data (attrs, CKA_ID, value, n_value);
+			g_free (value);
+		} else {
+			g_warning ("couldn't retrieve id for previous login item: %s",
+			           egg_error_message (error));
+			g_clear_error (&error);
+		}
+		g_object_unref (item);
+	}
+
+	if (label == NULL)
+		label = _("Unnamed");
+
+	display_name = g_strdup_printf (_("Unlock password for: %s"), label);
+	gp11_attributes_add_string (attrs, CKA_LABEL, display_name);
+
+	gp11_attributes_add_boolean (attrs, CKA_TOKEN, TRUE);
+	gp11_attributes_add_ulong (attrs, CKA_CLASS, CKO_SECRET_KEY);
+	gp11_attributes_add_data (attrs, CKA_G_COLLECTION, "login", (gsize)5);
+	gp11_attributes_add (attrs, &fields);
+
+	gp11_attribute_clear (&fields);
+	return attrs;
+}
+
+GP11Attributes*
+gkd_login_attach_make_attributes (const gchar *label, const gchar *first, ...)
+{
+	GP11Attributes *attrs;
+	GP11Session *session;
+	GP11Module *module;
+	va_list va;
+
+	module = module_instance ();
+	session = lookup_login_session (module);
+
+	va_start (va, first);
+	attrs = attach_make_attributes_va (session, label, first, va);
+	va_end (va);
+
+	g_object_unref (session);
+	g_object_unref (module);
+
+	return attrs;
+}
+
 void
 gkd_login_attach_secret (const gchar *label, const gchar *secret,
                          const gchar *first, ...)
 {
 	GError *error = NULL;
-	GP11Attribute fields;
 	GP11Session *session;
 	GP11Module *module;
-	gchar *display_name;
-	GP11Object* item;
+	GP11Attributes *attrs;
+	GP11Object *item;
 	va_list va;
 
-	if (label == NULL)
-		label = _("Unnamed");
 	if (secret == NULL)
 		secret = "";
 
@@ -595,29 +665,11 @@ gkd_login_attach_secret (const gchar *label, const gchar *secret,
 	session = lookup_login_session (module);
 
 	va_start(va, first);
-	gp11_attribute_init_empty (&fields, CKA_G_FIELDS);
-	string_attribute_list_va (va, first, &fields);
+	attrs = attach_make_attributes_va (session, label, first, va);
 	va_end(va);
 
-	display_name = g_strdup_printf (_("Unlock password for: %s"), label);
-
-	item = find_login_keyring_item (session, &fields);
-	if (item) {
-		gp11_object_set (item, &error,
-		                 CKA_LABEL, strlen (display_name), display_name,
-		                 CKA_VALUE, strlen (secret), secret,
-		                 GP11_INVALID);
-	} else {
-		item = gp11_session_create_object (session, &error,
-		                                   CKA_TOKEN, GP11_BOOLEAN, TRUE,
-		                                   CKA_CLASS, GP11_ULONG, CKO_SECRET_KEY,
-		                                   CKA_LABEL, strlen (display_name), display_name,
-		                                   CKA_VALUE, strlen (secret), secret,
-		                                   CKA_G_COLLECTION, (gsize)5, "login",
-		                                   CKA_G_FIELDS, fields.length, fields.value,
-		                                   GP11_INVALID);
-	}
-
+	gp11_attributes_add_string (attrs, CKA_VALUE, secret);
+	item = gp11_session_create_object_full (session, attrs, NULL, &error);
 	if (error != NULL) {
 		g_warning ("couldn't store secret in login keyring: %s", egg_error_message (error));
 		g_clear_error (&error);
@@ -625,8 +677,8 @@ gkd_login_attach_secret (const gchar *label, const gchar *secret,
 
 	if (item)
 		g_object_unref (item);
-	g_free (display_name);
-	gp11_attribute_clear (&fields);
+
+	gp11_attributes_unref (attrs);
 	g_object_unref (session);
 	g_object_unref (module);
 }
@@ -701,22 +753,3 @@ gkd_login_remove_secret (const gchar *first, ...)
 	g_object_unref (session);
 	g_object_unref (module);
 }
-
-GP11Attributes*
-gkd_login_attributes_for_secret (const gchar *first, ...)
-{
-	GP11Attributes *attrs;
-	GP11Attribute *fields;
-	va_list va;
-
-	attrs = gp11_attributes_newv (CKA_CLASS, GP11_ULONG, CKO_SECRET_KEY,
-	                              CKA_G_COLLECTION, (gsize)5, "login",
-	                              GP11_INVALID);
-
-	va_start(va, first);
-	fields = gp11_attributes_add_empty (attrs, CKA_G_FIELDS);
-	string_attribute_list_va (va, first, fields);
-	va_end(va);
-
-	return attrs;
-}
diff --git a/daemon/login/gkd-login.h b/daemon/login/gkd-login.h
index 89157b1..849b9f4 100644
--- a/daemon/login/gkd-login.h
+++ b/daemon/login/gkd-login.h
@@ -40,13 +40,14 @@ void              gkd_login_attach_secret            (const gchar *label,
                                                       const gchar *first,
                                                       ...);
 
-gchar*            gkd_login_lookup_secret            (const gchar *first,
+GP11Attributes*   gkd_login_attach_make_attributes   (const gchar *label,
+                                                      const gchar *first,
                                                       ...);
 
-void              gkd_login_remove_secret            (const gchar *first,
+gchar*            gkd_login_lookup_secret            (const gchar *first,
                                                       ...);
 
-GP11Attributes*   gkd_login_attributes_for_secret    (const gchar *first,
+void              gkd_login_remove_secret            (const gchar *first,
                                                       ...);
 
 #endif /* __GKD_LOGIN_H__ */
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]