Merge branch 'bc/gnome-keyring'

Cleanups and tweaks for credential handling to work with ancient versions
of the gnome-keyring library that are still in use.

* bc/gnome-keyring:
  contrib/git-credential-gnome-keyring.c: support really ancient gnome-keyring
  contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring
  contrib/git-credential-gnome-keyring.c: report failure to store password
  contrib/git-credential-gnome-keyring.c: use glib messaging functions
  contrib/git-credential-gnome-keyring.c: use glib memory allocation functions
  contrib/git-credential-gnome-keyring.c: use secure memory for reading passwords
  contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds
  contrib/git-credential-gnome-keyring.c: use gnome helpers in keyring_object()
  contrib/git-credential-gnome-keyring.c: set Gnome application name
  contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing
  contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not ssize_t
  contrib/git-credential-gnome-keyring.c: exit non-zero when called incorrectly
  contrib/git-credential-gnome-keyring.c: add static where applicable
  contrib/git-credential-gnome-keyring.c: *style* use "if ()" not "if()" etc.
  contrib/git-credential-gnome-keyring.c: remove unused die() function
  contrib/git-credential-gnome-keyring.c: remove unnecessary pre-declarations

contrib/credential/gnome-keyring/Makefile

@@ -8,8 +8,8 @@ CFLAGS = -g -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-INCS:=$(shell pkg-config --cflags gnome-keyring-1)
-LIBS:=$(shell pkg-config --libs gnome-keyring-1)
+INCS:=$(shell pkg-config --cflags gnome-keyring-1 glib-2.0)
+LIBS:=$(shell pkg-config --libs gnome-keyring-1 glib-2.0)
 
 SRCS:=$(MAIN).c
 OBJS:=$(SRCS:.c=.o)

contrib/credential/gnome-keyring/git-credential-gnome-keyring.c

@@ -25,133 +25,150 @@
 
 #include <stdio.h>
 #include <string.h>
-#include <stdarg.h>
 #include <stdlib.h>
-#include <errno.h>
+#include <glib.h>
 #include <gnome-keyring.h>
 
-/*
- * This credential struct and API is simplified from git's credential.{h,c}
- */
-struct credential
-{
-	char          *protocol;
-	char          *host;
-	unsigned short port;
-	char          *path;
-	char          *username;
-	char          *password;
-};
+#ifdef GNOME_KEYRING_DEFAULT
 
-#define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+   /* Modern gnome-keyring */
 
-void credential_init(struct credential *c);
-void credential_clear(struct credential *c);
-int  credential_read(struct credential *c);
-void credential_write(const struct credential *c);
+#include <gnome-keyring-memory.h>
 
-typedef int (*credential_op_cb)(struct credential*);
+#else
 
-struct credential_operation
-{
-	char             *name;
-	credential_op_cb op;
-};
+   /*
+    * Support ancient gnome-keyring, circ. RHEL 5.X.
+    * GNOME_KEYRING_DEFAULT seems to have been introduced with Gnome 2.22,
+    * and the other features roughly around Gnome 2.20, 6 months before.
+    * Ubuntu 8.04 used Gnome 2.22 (I think).  Not sure any distro used 2.20.
+    * So the existence/non-existence of GNOME_KEYRING_DEFAULT seems like
+    * a decent thing to use as an indicator.
+    */
 
-#define CREDENTIAL_OP_END \
-  { NULL,NULL }
+#define GNOME_KEYRING_DEFAULT NULL
 
 /*
- * Table with operation callbacks is defined in concrete
- * credential helper implementation and contains entries
- * like { "get", function_to_get_credential } terminated
- * by CREDENTIAL_OP_END.
+ * ancient gnome-keyring returns DENIED when an entry is not found.
+ * Setting NO_MATCH to DENIED will prevent us from reporting DENIED
+ * errors during get and erase operations, but we will still report
+ * DENIED errors during a store.
  */
-struct credential_operation const credential_helper_ops[];
+#define GNOME_KEYRING_RESULT_NO_MATCH GNOME_KEYRING_RESULT_DENIED
 
-/* ---------------- common helper functions ----------------- */
+#define gnome_keyring_memory_alloc g_malloc
+#define gnome_keyring_memory_free gnome_keyring_free_password
+#define gnome_keyring_memory_strdup g_strdup
 
-static inline void free_password(char *password)
+static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)
 {
-	char *c = password;
-	if (!password)
-		return;
-
-	while (*c) *c++ = '\0';
-	free(password);
+	switch (result) {
+	case GNOME_KEYRING_RESULT_OK:
+		return "OK";
+	case GNOME_KEYRING_RESULT_DENIED:
+		return "Denied";
+	case GNOME_KEYRING_RESULT_NO_KEYRING_DAEMON:
+		return "No Keyring Daemon";
+	case GNOME_KEYRING_RESULT_ALREADY_UNLOCKED:
+		return "Already UnLocked";
+	case GNOME_KEYRING_RESULT_NO_SUCH_KEYRING:
+		return "No Such Keyring";
+	case GNOME_KEYRING_RESULT_BAD_ARGUMENTS:
+		return "Bad Arguments";
+	case GNOME_KEYRING_RESULT_IO_ERROR:
+		return "IO Error";
+	case GNOME_KEYRING_RESULT_CANCELLED:
+		return "Cancelled";
+	case GNOME_KEYRING_RESULT_ALREADY_EXISTS:
+		return "Already Exists";
+	default:
+		return "Unknown Error";
+	}
 }
 
-static inline void warning(const char *fmt, ...)
+/*
+ * Support really ancient gnome-keyring, circ. RHEL 4.X.
+ * Just a guess for the Glib version.  Glib 2.8 was roughly Gnome 2.12 ?
+ * Which was released with gnome-keyring 0.4.3 ??
+ */
+#if GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 8
+
+static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data)
 {
-	va_list ap;
+	gpointer *data = (gpointer*) user_data;
+	int *done = (int*) data[0];
+	GnomeKeyringResult *r = (GnomeKeyringResult*) data[1];
 
-	va_start(ap, fmt);
-	fprintf(stderr, "warning: ");
-	vfprintf(stderr, fmt, ap);
-	fprintf(stderr, "\n" );
-	va_end(ap);
+	*r = result;
+	*done = 1;
 }
 
-static inline void error(const char *fmt, ...)
+static void wait_for_request_completion(int *done)
 {
-	va_list ap;
-
-	va_start(ap, fmt);
-	fprintf(stderr, "error: ");
-	vfprintf(stderr, fmt, ap);
-	fprintf(stderr, "\n" );
-	va_end(ap);
+	GMainContext *mc = g_main_context_default();
+	while (!*done)
+		g_main_context_iteration(mc, TRUE);
 }
 
-static inline void die(const char *fmt, ...)
+static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, guint32 id)
 {
-	va_list ap;
+	int done = 0;
+	GnomeKeyringResult result;
+	gpointer data[] = { &done, &result };
+
+	gnome_keyring_item_delete(keyring, id, gnome_keyring_done_cb, data,
+		NULL);
+
+	wait_for_request_completion(&done);
 
-	va_start(ap,fmt);
-	error(fmt, ap);
-	va_end(ap);
-	exit(EXIT_FAILURE);
+	return result;
 }
 
-static inline void die_errno(int err)
+#endif
+#endif
+
+/*
+ * This credential struct and API is simplified from git's credential.{h,c}
+ */
+struct credential
 {
-	error("%s", strerror(err));
-	exit(EXIT_FAILURE);
-}
+	char          *protocol;
+	char          *host;
+	unsigned short port;
+	char          *path;
+	char          *username;
+	char          *password;
+};
 
-static inline char *xstrdup(const char *str)
+#define CREDENTIAL_INIT \
+  { NULL,NULL,0,NULL,NULL,NULL }
+
+typedef int (*credential_op_cb)(struct credential*);
+
+struct credential_operation
 {
-	char *ret = strdup(str);
-	if (!ret)
-		die_errno(errno);
+	char             *name;
+	credential_op_cb op;
+};
 
-	return ret;
-}
+#define CREDENTIAL_OP_END \
+  { NULL,NULL }
 
 /* ----------------- GNOME Keyring functions ----------------- */
 
 /* create a special keyring option string, if path is given */
 static char* keyring_object(struct credential *c)
 {
-	char* object = NULL;
-
 	if (!c->path)
-		return object;
-
-	object = (char*) malloc(strlen(c->host)+strlen(c->path)+8);
-	if(!object)
-		die_errno(errno);
+		return NULL;
 
-	if(c->port)
-		sprintf(object,"%s:%hd/%s",c->host,c->port,c->path);
-	else
-		sprintf(object,"%s/%s",c->host,c->path);
+	if (c->port)
+		return g_strdup_printf("%s:%hd/%s", c->host, c->port, c->path);
 
-	return object;
+	return g_strdup_printf("%s/%s", c->host, c->path);
 }
 
-int keyring_get(struct credential *c)
+static int keyring_get(struct credential *c)
 {
 	char* object = NULL;
 	GList *entries;
@@ -173,7 +190,7 @@ int keyring_get(struct credential *c)
 				c->port,
 				&entries);
 
-	free(object);
+	g_free(object);
 
 	if (result == GNOME_KEYRING_RESULT_NO_MATCH)
 		return EXIT_SUCCESS;
@@ -182,18 +199,18 @@ int keyring_get(struct credential *c)
 		return EXIT_SUCCESS;
 
 	if (result != GNOME_KEYRING_RESULT_OK) {
-		error("%s",gnome_keyring_result_to_message(result));
+		g_critical("%s", gnome_keyring_result_to_message(result));
 		return EXIT_FAILURE;
 	}
 
 	/* pick the first one from the list */
 	password_data = (GnomeKeyringNetworkPasswordData *) entries->data;
 
-	free_password(c->password);
-	c->password = xstrdup(password_data->password);
+	gnome_keyring_memory_free(c->password);
+	c->password = gnome_keyring_memory_strdup(password_data->password);
 
 	if (!c->username)
-		c->username = xstrdup(password_data->user);
+		c->username = g_strdup(password_data->user);
 
 	gnome_keyring_network_password_list_free(entries);
 
@@ -201,10 +218,11 @@ int keyring_get(struct credential *c)
 }
 
 
-int keyring_store(struct credential *c)
+static int keyring_store(struct credential *c)
 {
 	guint32 item_id;
 	char  *object = NULL;
+	GnomeKeyringResult result;
 
 	/*
 	 * Sanity check that what we are storing is actually sensible.
@@ -219,7 +237,7 @@ int keyring_store(struct credential *c)
 
 	object = keyring_object(c);
 
-	gnome_keyring_set_network_password_sync(
+	result = gnome_keyring_set_network_password_sync(
 				GNOME_KEYRING_DEFAULT,
 				c->username,
 				NULL /* domain */,
@@ -231,11 +249,18 @@ int keyring_store(struct credential *c)
 				c->password,
 				&item_id);
 
-	free(object);
+	g_free(object);
+
+	if (result != GNOME_KEYRING_RESULT_OK &&
+	    result != GNOME_KEYRING_RESULT_CANCELLED) {
+		g_critical("%s", gnome_keyring_result_to_message(result));
+		return EXIT_FAILURE;
+	}
+
 	return EXIT_SUCCESS;
 }
 
-int keyring_erase(struct credential *c)
+static int keyring_erase(struct credential *c)
 {
 	char  *object = NULL;
 	GList *entries;
@@ -265,7 +290,7 @@ int keyring_erase(struct credential *c)
 				c->port,
 				&entries);
 
-	free(object);
+	g_free(object);
 
 	if (result == GNOME_KEYRING_RESULT_NO_MATCH)
 		return EXIT_SUCCESS;
@@ -275,7 +300,7 @@ int keyring_erase(struct credential *c)
 
 	if (result != GNOME_KEYRING_RESULT_OK)
 	{
-		error("%s",gnome_keyring_result_to_message(result));
+		g_critical("%s", gnome_keyring_result_to_message(result));
 		return EXIT_FAILURE;
 	}
 
@@ -289,7 +314,7 @@ int keyring_erase(struct credential *c)
 
 	if (result != GNOME_KEYRING_RESULT_OK)
 	{
-		error("%s",gnome_keyring_result_to_message(result));
+		g_critical("%s", gnome_keyring_result_to_message(result));
 		return EXIT_FAILURE;
 	}
 
@@ -300,7 +325,7 @@ int keyring_erase(struct credential *c)
  * Table with helper operation callbacks, used by generic
  * credential helper main function.
  */
-struct credential_operation const credential_helper_ops[] =
+static struct credential_operation const credential_helper_ops[] =
 {
 	{ "get",   keyring_get   },
 	{ "store", keyring_store },
@@ -310,66 +335,69 @@ struct credential_operation const credential_helper_ops[] =
 
 /* ------------------ credential functions ------------------ */
 
-void credential_init(struct credential *c)
+static void credential_init(struct credential *c)
 {
 	memset(c, 0, sizeof(*c));
 }
 
-void credential_clear(struct credential *c)
+static void credential_clear(struct credential *c)
 {
-	free(c->protocol);
-	free(c->host);
-	free(c->path);
-	free(c->username);
-	free_password(c->password);
+	g_free(c->protocol);
+	g_free(c->host);
+	g_free(c->path);
+	g_free(c->username);
+	gnome_keyring_memory_free(c->password);
 
 	credential_init(c);
 }
 
-int credential_read(struct credential *c)
+static int credential_read(struct credential *c)
 {
-	char    buf[1024];
-	ssize_t line_len = 0;
-	char   *key      = buf;
+	char    *buf;
+	size_t line_len;
+	char   *key;
 	char   *value;
 
-	while (fgets(buf, sizeof(buf), stdin))
+	key = buf = gnome_keyring_memory_alloc(1024);
+
+	while (fgets(buf, 1024, stdin))
 	{
 		line_len = strlen(buf);
 
-		if(buf[line_len-1]=='\n')
+		if (line_len && buf[line_len-1] == '\n')
 			buf[--line_len]='\0';
 
-		if(!line_len)
+		if (!line_len)
 			break;
 
 		value = strchr(buf,'=');
-		if(!value) {
-			warning("invalid credential line: %s", key);
+		if (!value) {
+			g_warning("invalid credential line: %s", key);
+			gnome_keyring_memory_free(buf);
 			return -1;
 		}
 		*value++ = '\0';
 
 		if (!strcmp(key, "protocol")) {
-			free(c->protocol);
-			c->protocol = xstrdup(value);
+			g_free(c->protocol);
+			c->protocol = g_strdup(value);
 		} else if (!strcmp(key, "host")) {
-			free(c->host);
-			c->host = xstrdup(value);
+			g_free(c->host);
+			c->host = g_strdup(value);
 			value = strrchr(c->host,':');
 			if (value) {
 				*value++ = '\0';
 				c->port = atoi(value);
 			}
 		} else if (!strcmp(key, "path")) {
-			free(c->path);
-			c->path = xstrdup(value);
+			g_free(c->path);
+			c->path = g_strdup(value);
 		} else if (!strcmp(key, "username")) {
-			free(c->username);
-			c->username = xstrdup(value);
+			g_free(c->username);
+			c->username = g_strdup(value);
 		} else if (!strcmp(key, "password")) {
-			free_password(c->password);
-			c->password = xstrdup(value);
+			gnome_keyring_memory_free(c->password);
+			c->password = gnome_keyring_memory_strdup(value);
 			while (*value) *value++ = '\0';
 		}
 		/*
@@ -378,17 +406,20 @@ int credential_read(struct credential *c)
 		 * learn new lines, and the helpers are updated to match.
 		 */
 	}
+
+	gnome_keyring_memory_free(buf);
+
 	return 0;
 }
 
-void credential_write_item(FILE *fp, const char *key, const char *value)
+static void credential_write_item(FILE *fp, const char *key, const char *value)
 {
 	if (!value)
 		return;
 	fprintf(fp, "%s=%s\n", key, value);
 }
 
-void credential_write(const struct credential *c)
+static void credential_write(const struct credential *c)
 {
 	/* only write username/password, if set */
 	credential_write_item(stdout, "username", c->username);
@@ -402,9 +433,9 @@ static void usage(const char *name)
 
 	basename = (basename) ? basename + 1 : name;
 	fprintf(stderr, "usage: %s <", basename);
-	while(try_op->name) {
+	while (try_op->name) {
 		fprintf(stderr,"%s",(try_op++)->name);
-		if(try_op->name)
+		if (try_op->name)
 			fprintf(stderr,"%s","|");
 	}
 	fprintf(stderr,"%s",">\n");
@@ -419,19 +450,21 @@ int main(int argc, char *argv[])
 
 	if (!argv[1]) {
 		usage(argv[0]);
-		goto out;
+		exit(EXIT_FAILURE);
 	}
 
+	g_set_application_name("Git Credential Helper");
+
 	/* lookup operation callback */
-	while(try_op->name && strcmp(argv[1], try_op->name))
+	while (try_op->name && strcmp(argv[1], try_op->name))
 		try_op++;
 
 	/* unsupported operation given -- ignore silently */
-	if(!try_op->name || !try_op->op)
+	if (!try_op->name || !try_op->op)
 		goto out;
 
 	ret = credential_read(&cred);
-	if(ret)
+	if (ret)
 		goto out;
 
 	/* perform credential operation */