[tracker/miner-web-review] libtracker-miner: Fix lock/unlock API for TrackerPasswordProvider

[Martyn James Russell <mr src gnome org>]

commit 256a8d83fa2d905a586916477947979be17024b6
Author: Martyn Russell <martyn lanedo com>
Date:   Thu Mar 18 13:36:23 2010 +0000

    libtracker-miner: Fix lock/unlock API for TrackerPasswordProvider

 .../libtracker-miner/libtracker-miner-sections.txt |    6 +-
 .../tracker-password-provider-gnome.c              |    2 +-
 src/libtracker-miner/tracker-password-provider.c   |   67 ++++++++++++++++++--
 src/libtracker-miner/tracker-password-provider.h   |   37 +++++------
 .../tracker-password-provider-test.c               |    8 ++-
 5 files changed, 88 insertions(+), 32 deletions(-)
---
diff --git a/docs/reference/libtracker-miner/libtracker-miner-sections.txt b/docs/reference/libtracker-miner/libtracker-miner-sections.txt
index a35cc96..3ad05d5 100644
--- a/docs/reference/libtracker-miner/libtracker-miner-sections.txt
+++ b/docs/reference/libtracker-miner/libtracker-miner-sections.txt
@@ -150,16 +150,14 @@ TRACKER_PASSWORD_PROVIDER_GET_INTERFACE
 TrackerPasswordProvider
 TrackerPasswordProviderError
 TrackerPasswordProviderIface
-forget_password
-get_password
-store_password
 tracker_password_provider_error_quark
 tracker_password_provider_get
 tracker_password_provider_get_name
 tracker_password_provider_store_password
 tracker_password_provider_get_password
 tracker_password_provider_forget_password
-tracker_password_provider_strdup_mlock
+tracker_password_provider_lock_password
+tracker_password_provider_unlock_password
 <SUBSECTION Standard>
 TRACKER_PASSWORD_PROVIDER
 TRACKER_IS_PASSWORD_PROVIDER
diff --git a/src/libtracker-miner/tracker-password-provider-gnome.c b/src/libtracker-miner/tracker-password-provider-gnome.c
index c1894cb..7903d49 100644
--- a/src/libtracker-miner/tracker-password-provider-gnome.c
+++ b/src/libtracker-miner/tracker-password-provider-gnome.c
@@ -257,7 +257,7 @@ password_provider_gnome_get (TrackerPasswordProvider  *provider,
 		}
 	}
 
-	password = tracker_password_provider_strdup_mlock (found->secret);
+	password = tracker_password_provider_lock_password (found->secret);
 
 	gnome_keyring_found_list_free (found_items);
 
diff --git a/src/libtracker-miner/tracker-password-provider.c b/src/libtracker-miner/tracker-password-provider.c
index 3d73f8e..8c80b1f 100644
--- a/src/libtracker-miner/tracker-password-provider.c
+++ b/src/libtracker-miner/tracker-password-provider.c
@@ -206,17 +206,72 @@ tracker_password_provider_forget_password (TrackerPasswordProvider  *provider,
 	iface->forget_password (provider, service, error);
 }
 
+/**
+ * tracker_password_provider_lock_password:
+ * @password: a string pointer
+ *
+ * This function calls mlock() to secure a memory region newly
+ * allocated and @password is copied using memcpy() into the new
+ * address.
+ * 
+ * Password can not be %NULL or an empty string ("").
+ *
+ * Returns: a newly allocated string which <emphasis>MUST</emphasis>
+ * be freed with tracker_password_provider_unlock_password(). On
+ * failure %NULL is returned.
+ **/
 gchar *
-tracker_password_provider_strdup_mlock (const gchar *source)
+tracker_password_provider_lock_password (const gchar *password)
 {
 	gchar *dest;
+	int retval;
 
-	g_return_val_if_fail (source != NULL, NULL);
+	g_return_val_if_fail (password != NULL, NULL);
+	g_return_val_if_fail (password[0] != '\0', NULL);
 
-	dest = malloc (1 + strlen (source));
-	dest = memset (dest, 0, 1 + strlen (source));
-	mlock (dest, sizeof (dest));
-	memcpy (dest, source, strlen (source));
+	dest = g_malloc0 (strlen (password) + 1);
+	retval = mlock (dest, sizeof (dest));
+
+	if (retval != 0) {
+		g_free (dest);
+		return NULL;
+	}
+
+	memcpy (dest, password, strlen (password));
 
 	return dest;
 }
+
+/**
+ * tracker_password_provider_unlock_password:
+ * @password: a string pointer
+ *
+ * This function calls munlock() on @password which should be a
+ * secured memory region. The @password is zeroed first with bzero()
+ * and once unlocked it is freed with g_free(). 
+ *
+ * The @password can not be %NULL or an empty string (""). In
+ * addition, @password <emphasis>MUST</emphasis> be a string created
+ * with tracker_password_provider_lock_password().
+ *
+ * Returns: %TRUE if munlock() succeeded, otherwise %FALSE is returned.
+ **/
+gboolean
+tracker_password_provider_unlock_password (gchar *password)
+{
+	int retval;
+
+	g_return_val_if_fail (password != NULL, FALSE);
+	g_return_val_if_fail (password[0] != '\0', FALSE);
+	
+	bzero (password, strlen (password));
+	retval = munlock (password, sizeof (password));
+	g_free (password);
+
+	if (retval != 0) {
+		/* FIXME: Handle errors? */
+		return FALSE;
+	}
+
+	return TRUE;
+}
diff --git a/src/libtracker-miner/tracker-password-provider.h b/src/libtracker-miner/tracker-password-provider.h
index cd745d2..92530b0 100644
--- a/src/libtracker-miner/tracker-password-provider.h
+++ b/src/libtracker-miner/tracker-password-provider.h
@@ -81,27 +81,26 @@ struct TrackerPasswordProviderIface {
 	                              GError                  **error);
 };
 
-GType    tracker_password_provider_get_type        (void) G_GNUC_CONST;
-GQuark   tracker_password_provider_error_quark     (void);
+GType    tracker_password_provider_get_type         (void) G_GNUC_CONST;
+GQuark   tracker_password_provider_error_quark      (void);
 
-gchar*   tracker_password_provider_get_name        (TrackerPasswordProvider  *provider);
+gchar *  tracker_password_provider_get_name         (TrackerPasswordProvider  *provider);
+gboolean tracker_password_provider_store_password   (TrackerPasswordProvider  *provider,
+                                                     const gchar              *service,
+                                                     const gchar              *description,
+                                                     const gchar              *username,
+                                                     const gchar              *password,
+                                                     GError                  **error);
+gchar *  tracker_password_provider_get_password     (TrackerPasswordProvider  *provider,
+                                                     const gchar              *service,
+                                                     gchar                   **username,
+                                                     GError                  **error);
+void     tracker_password_provider_forget_password  (TrackerPasswordProvider  *provider,
+                                                     const gchar              *service,
+                                                     GError                  **error);
+gchar *  tracker_password_provider_lock_password    (const gchar              *password);
+gboolean tracker_password_provider_unlock_password  (gchar                    *password);
 
-gboolean tracker_password_provider_store_password  (TrackerPasswordProvider  *provider,
-                                                    const gchar              *service,
-                                                    const gchar              *description,
-                                                    const gchar              *username,
-                                                    const gchar              *password,
-                                                    GError                  **error);
-gchar*   tracker_password_provider_get_password    (TrackerPasswordProvider  *provider,
-                                                    const gchar              *service,
-                                                    gchar                   **username,
-                                                    GError                  **error);
-void     tracker_password_provider_forget_password (TrackerPasswordProvider  *provider,
-                                                    const gchar              *service,
-                                                    GError                  **error);
-gchar*   tracker_password_provider_strdup_mlock    (const gchar              *source);
-
-/* Must be defined by the selected implementation */
 /**
  * tracker_password_provider_get:
  *
diff --git a/tests/libtracker-miner/tracker-password-provider-test.c b/tests/libtracker-miner/tracker-password-provider-test.c
index 76e84b0..27ff44e 100644
--- a/tests/libtracker-miner/tracker-password-provider-test.c
+++ b/tests/libtracker-miner/tracker-password-provider-test.c
@@ -54,6 +54,7 @@ test_password_provider_getting (void)
 {
 	gchar *username = NULL;
 	gchar *password = NULL;
+	gboolean success;
 	GError *error = NULL;
 
 	password = tracker_password_provider_get_password (provider,
@@ -69,7 +70,9 @@ test_password_provider_getting (void)
 	         username);
 
 	g_free (username);
-	g_free (password);
+
+	success = tracker_password_provider_unlock_password (password);
+	g_assert_cmpint (success, ==, TRUE);
 
 	/* Also test without getting the username */
 	password = tracker_password_provider_get_password (provider,
@@ -81,7 +84,8 @@ test_password_provider_getting (void)
 
 	g_print ("Found password is '%s' for NULL username\n", password);
 
-	g_free (password);
+	success = tracker_password_provider_unlock_password (password);
+	g_assert_cmpint (success, ==, TRUE);
 }
 
 int