* [PATCH] contrib: GnomeKeyring support + generic helper implementation
@ 2012-08-23 16:57 Philipp A. Hartmann
2012-08-23 16:57 ` [PATCH 1/4] contrib: add credential helper for GnomeKeyring Philipp A. Hartmann
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Philipp A. Hartmann @ 2012-08-23 16:57 UTC (permalink / raw)
To: git; +Cc: Jeff King, John Szakmeister
All,
the following patch series proposes enhancements to the credential helper
implementations in the contrib section. The detailed development history
can be found at GitHub [1].
The first patch adds a GnomeKeyring credential backend. The GnomeKeyring
specific parts are based on the work by John Szakmeister, who wrote the
helper originally for the initial, unpublished version of the credential
helper protocol.
The second patch adds a generic implementation of a credential helper
based on a simplified credential API and common helper functions. Helpers
based on this implementation do not need to worry about the credential
protocol and only need to implement callback functions for the different
credential operations.
The third and fourth patches port the existing helpers to this generic
implementation.
Looking forward to your thoughts.
Greetings from Oldenburg,
Philipp
[1] https://github.com/pah/git-credential-helper
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/4] contrib: add credential helper for GnomeKeyring 2012-08-23 16:57 [PATCH] contrib: GnomeKeyring support + generic helper implementation Philipp A. Hartmann @ 2012-08-23 16:57 ` Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 2/4] contrib: add generic credential helper Philipp A. Hartmann ` (3 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Philipp A. Hartmann @ 2012-08-23 16:57 UTC (permalink / raw) To: git; +Cc: Jeff King, John Szakmeister, Philipp A. Hartmann From: "Philipp A. Hartmann" <pah@qo.cx> With this installed in your $PATH, you can store git-over-http passwords in your keyring by doing: git config credential.helper gnome-keyring The code is based in large part on the work of John Szakmeister who wrote the helper originally for the initial, unpublished version of the credential helper protocol. This version will pass t0303 if you do: GIT_TEST_CREDENTIAL_HELPER=gnome-keyring \ ./t0303-credential-external.sh Signed-off-by: Philipp A. Hartmann <pah@qo.cx> --- contrib/credential/gnome-keyring/.gitignore | 1 + contrib/credential/gnome-keyring/Makefile | 24 ++ .../gnome-keyring/git-credential-gnome-keyring.c | 445 ++++++++++++++++++++ 3 files changed, 470 insertions(+) create mode 100644 contrib/credential/gnome-keyring/.gitignore create mode 100644 contrib/credential/gnome-keyring/Makefile create mode 100644 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c diff --git a/contrib/credential/gnome-keyring/.gitignore b/contrib/credential/gnome-keyring/.gitignore new file mode 100644 index 0000000..88d8fcd --- /dev/null +++ b/contrib/credential/gnome-keyring/.gitignore @@ -0,0 +1 @@ +git-credential-gnome-keyring diff --git a/contrib/credential/gnome-keyring/Makefile b/contrib/credential/gnome-keyring/Makefile new file mode 100644 index 0000000..e6561d8 --- /dev/null +++ b/contrib/credential/gnome-keyring/Makefile @@ -0,0 +1,24 @@ +MAIN:=git-credential-gnome-keyring +all:: $(MAIN) + +CC = gcc +RM = rm -f +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) + +SRCS:=$(MAIN).c +OBJS:=$(SRCS:.c=.o) + +%.o: %.c + $(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $< + +$(MAIN): $(OBJS) + $(CC) -o $@ $(LDFLAGS) $^ $(LIBS) + +clean: + @$(RM) $(MAIN) $(OBJS) diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c new file mode 100644 index 0000000..41f61c5 --- /dev/null +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c @@ -0,0 +1,445 @@ +/* + * Copyright (C) 2011 John Szakmeister <john@szakmeister.net> + * 2012 Philipp A. Hartmann <pah@qo.cx> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +/* + * Credits: + * - GNOME Keyring API handling originally written by John Szakmeister + * - ported to credential helper API by Philipp A. Hartmann + */ + +#include <stdio.h> +#include <string.h> +#include <stdarg.h> +#include <stdlib.h> +#include <errno.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; +}; + +#define CREDENTIAL_INIT \ + { NULL,NULL,0,NULL,NULL,NULL } + +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); + +typedef int (*credential_op_cb)(struct credential*); + +struct credential_operation +{ + char *name; + credential_op_cb op; +}; + +#define CREDENTIAL_OP_END \ + { NULL,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. + */ +struct credential_operation const credential_helper_ops[]; + +/* ---------------- common helper functions ----------------- */ + +static inline void free_password(char *password) +{ + char *c = password; + if (!password) + return; + + while (*c) *c++ = '\0'; + free(password); +} + +static inline void warning(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + fprintf(stderr, "warning: "); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n" ); + va_end(ap); +} + +static inline void error(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + fprintf(stderr, "error: "); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n" ); + va_end(ap); +} + +static inline void die(const char *fmt, ...) +{ + va_list ap; + + va_start(ap,fmt); + error(fmt, ap); + va_end(ap); + exit(EXIT_FAILURE); +} + +static inline void die_errno(int err) +{ + error("%s", strerror(err)); + exit(EXIT_FAILURE); +} + +static inline char *xstrdup(const char *str) +{ + char *ret = strdup(str); + if (!ret) + die_errno(errno); + + return ret; +} + +/* ----------------- 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); + + if(c->port) + sprintf(object,"%s:%hd/%s",c->host,c->port,c->path); + else + sprintf(object,"%s/%s",c->host,c->path); + + return object; +} + +int keyring_get(struct credential *c) +{ + char* object = NULL; + GList *entries; + GnomeKeyringNetworkPasswordData *password_data; + GnomeKeyringResult result; + + if (!c->protocol || !(c->host || c->path)) + return EXIT_FAILURE; + + object = keyring_object(c); + + result = gnome_keyring_find_network_password_sync( + c->username, + NULL /* domain */, + c->host, + object, + c->protocol, + NULL /* authtype */, + c->port, + &entries); + + free(object); + + if (result == GNOME_KEYRING_RESULT_NO_MATCH) + return EXIT_SUCCESS; + + if (result == GNOME_KEYRING_RESULT_CANCELLED) + return EXIT_SUCCESS; + + if (result != GNOME_KEYRING_RESULT_OK) { + error("%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); + + if (!c->username) + c->username = xstrdup(password_data->user); + + gnome_keyring_network_password_list_free(entries); + + return EXIT_SUCCESS; +} + + +int keyring_store(struct credential *c) +{ + guint32 item_id; + char *object = NULL; + + /* + * Sanity check that what we are storing is actually sensible. + * In particular, we can't make a URL without a protocol field. + * Without either a host or pathname (depending on the scheme), + * we have no primary key. And without a username and password, + * we are not actually storing a credential. + */ + if (!c->protocol || !(c->host || c->path) || + !c->username || !c->password) + return EXIT_FAILURE; + + object = keyring_object(c); + + gnome_keyring_set_network_password_sync( + GNOME_KEYRING_DEFAULT, + c->username, + NULL /* domain */, + c->host, + object, + c->protocol, + NULL /* authtype */, + c->port, + c->password, + &item_id); + + free(object); + return EXIT_SUCCESS; +} + +int keyring_erase(struct credential *c) +{ + char *object = NULL; + GList *entries; + GnomeKeyringNetworkPasswordData *password_data; + GnomeKeyringResult result; + + /* + * Sanity check that we actually have something to match + * against. The input we get is a restrictive pattern, + * so technically a blank credential means "erase everything". + * But it is too easy to accidentally send this, since it is equivalent + * to empty input. So explicitly disallow it, and require that the + * pattern have some actual content to match. + */ + if (!c->protocol && !c->host && !c->path && !c->username) + return EXIT_FAILURE; + + object = keyring_object(c); + + result = gnome_keyring_find_network_password_sync( + c->username, + NULL /* domain */, + c->host, + object, + c->protocol, + NULL /* authtype */, + c->port, + &entries); + + free(object); + + if (result == GNOME_KEYRING_RESULT_NO_MATCH) + return EXIT_SUCCESS; + + if (result == GNOME_KEYRING_RESULT_CANCELLED) + return EXIT_SUCCESS; + + if (result != GNOME_KEYRING_RESULT_OK) + { + error("%s",gnome_keyring_result_to_message(result)); + return EXIT_FAILURE; + } + + /* pick the first one from the list (delete all matches?) */ + password_data = (GnomeKeyringNetworkPasswordData *) entries->data; + + result = gnome_keyring_item_delete_sync( + password_data->keyring, password_data->item_id); + + gnome_keyring_network_password_list_free(entries); + + if (result != GNOME_KEYRING_RESULT_OK) + { + error("%s",gnome_keyring_result_to_message(result)); + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; +} + +/* + * Table with helper operation callbacks, used by generic + * credential helper main function. + */ +struct credential_operation const credential_helper_ops[] = +{ + { "get", keyring_get }, + { "store", keyring_store }, + { "erase", keyring_erase }, + CREDENTIAL_OP_END +}; + +/* ------------------ credential functions ------------------ */ + +void credential_init(struct credential *c) +{ + memset(c, 0, sizeof(*c)); +} + +void credential_clear(struct credential *c) +{ + free(c->protocol); + free(c->host); + free(c->path); + free(c->username); + free_password(c->password); + + credential_init(c); +} + +int credential_read(struct credential *c) +{ + char buf[1024]; + ssize_t line_len = 0; + char *key = buf; + char *value; + + while (fgets(buf, sizeof(buf), stdin)) + { + line_len = strlen(buf); + + if(buf[line_len-1]=='\n') + buf[--line_len]='\0'; + + if(!line_len) + break; + + value = strchr(buf,'='); + if(!value) { + warning("invalid credential line: %s", key); + return -1; + } + *value++ = '\0'; + + if (!strcmp(key, "protocol")) { + free(c->protocol); + c->protocol = xstrdup(value); + } else if (!strcmp(key, "host")) { + free(c->host); + c->host = xstrdup(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); + } else if (!strcmp(key, "username")) { + free(c->username); + c->username = xstrdup(value); + } else if (!strcmp(key, "password")) { + free_password(c->password); + c->password = xstrdup(value); + while (*value) *value++ = '\0'; + } + /* + * Ignore other lines; we don't know what they mean, but + * this future-proofs us when later versions of git do + * learn new lines, and the helpers are updated to match. + */ + } + return 0; +} + +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) +{ + /* only write username/password, if set */ + credential_write_item(stdout, "username", c->username); + credential_write_item(stdout, "password", c->password); +} + +static void usage(const char *name) +{ + struct credential_operation const *try_op = credential_helper_ops; + const char *basename = strrchr(name,'/'); + + basename = (basename) ? basename + 1 : name; + fprintf(stderr, "Usage: %s <", basename); + while(try_op->name) { + fprintf(stderr,"%s",(try_op++)->name); + if(try_op->name) + fprintf(stderr,"%s","|"); + } + fprintf(stderr,"%s",">\n"); +} + +int main(int argc, char *argv[]) +{ + int ret = EXIT_SUCCESS; + + struct credential_operation const *try_op = credential_helper_ops; + struct credential cred = CREDENTIAL_INIT; + + if (!argv[1]) { + usage(argv[0]); + goto out; + } + + /* lookup operation callback */ + while(try_op->name && strcmp(argv[1], try_op->name)) + try_op++; + + /* unsupported operation given -- ignore silently */ + if(!try_op->name || !try_op->op) + goto out; + + ret = credential_read(&cred); + if(ret) + goto out; + + /* perform credential operation */ + ret = (*try_op->op)(&cred); + + credential_write(&cred); + +out: + credential_clear(&cred); + return ret; +} -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/4] contrib: add generic credential helper 2012-08-23 16:57 [PATCH] contrib: GnomeKeyring support + generic helper implementation Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 1/4] contrib: add credential helper for GnomeKeyring Philipp A. Hartmann @ 2012-08-23 16:57 ` Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 3/4] gnome-keyring: port to generic helper implementation Philipp A. Hartmann ` (2 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Philipp A. Hartmann @ 2012-08-23 16:57 UTC (permalink / raw) To: git; +Cc: Jeff King, John Szakmeister, Philipp A. Hartmann From: "Philipp A. Hartmann" <pah@qo.cx> This adds a generic implementation for credential helpers. It provides a header file credential_helper.h containing a simplified credential API and common helper functions. The implementation in credential_helper.c already provides a main() function and chooses the credential operation from a function table: struct credential_operation const credential_helper_ops[] = { { "get", my_backend_get }, { "store", my_backend_store }, { "erase", my_backend_erase }, CREDENTIAL_OP_END }; With this, a specific credential helper backend usually only needs to implement these operation callbacks. Signed-off-by: Philipp A. Hartmann <pah@qo.cx> --- contrib/credential/helper/credential_helper.c | 149 +++++++++++++++++++++++++ contrib/credential/helper/credential_helper.h | 116 +++++++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 contrib/credential/helper/credential_helper.c create mode 100644 contrib/credential/helper/credential_helper.h diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c new file mode 100644 index 0000000..e99c2ec --- /dev/null +++ b/contrib/credential/helper/credential_helper.c @@ -0,0 +1,149 @@ +/* + * Copyright (C) 2012 Philipp A. Hartmann <pah@qo.cx> + * + * This file is licensed under the GPL v2, or a later version + * at the discretion of Linus. + * + * This credential struct and API is simplified from git's + * credential.{h,c} to be used within credential helper + * implementations. + */ + +#include <credential_helper.h> + +void credential_init(struct credential *c) +{ + memset(c, 0, sizeof(*c)); +} + +void credential_clear(struct credential *c) +{ + free(c->protocol); + free(c->host); + free(c->path); + free(c->username); + free_password(c->password); + + credential_init(c); +} + +int credential_read(struct credential *c) +{ + char buf[1024]; + ssize_t line_len = 0; + char *key = buf; + char *value; + + while (fgets(buf, sizeof(buf), stdin)) + { + line_len = strlen(buf); + + if(buf[line_len-1]=='\n') + buf[--line_len]='\0'; + + if(!line_len) + break; + + value = strchr(buf,'='); + if(!value) { + warning("invalid credential line: %s", key); + return -1; + } + *value++ = '\0'; + + if (!strcmp(key, "protocol")) { + free(c->protocol); + c->protocol = xstrdup(value); + } else if (!strcmp(key, "host")) { + free(c->host); + c->host = xstrdup(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); + } else if (!strcmp(key, "username")) { + free(c->username); + c->username = xstrdup(value); + } else if (!strcmp(key, "password")) { + free_password(c->password); + c->password = xstrdup(value); + while (*value) *value++ = '\0'; + } + /* + * Ignore other lines; we don't know what they mean, but + * this future-proofs us when later versions of git do + * learn new lines, and the helpers are updated to match. + */ + } + return 0; +} + +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) +{ + /* only write username/password, if set */ + credential_write_item(stdout, "username", c->username); + credential_write_item(stdout, "password", c->password); +} + +static void usage(const char *name) +{ + struct credential_operation const *try_op = credential_helper_ops; + const char *basename = strrchr(name,'/'); + + basename = (basename) ? basename + 1 : name; + fprintf(stderr, "Usage: %s <", basename); + while(try_op->name) { + fprintf(stderr,"%s",(try_op++)->name); + if(try_op->name) + fprintf(stderr,"%s","|"); + } + fprintf(stderr,"%s",">\n"); +} + +/* + * generic main function for credential helpers + */ +int main(int argc, char *argv[]) +{ + int ret = EXIT_SUCCESS; + + struct credential_operation const *try_op = credential_helper_ops; + struct credential cred = CREDENTIAL_INIT; + + if (!argv[1]) { + usage(argv[0]); + goto out; + } + + /* lookup operation callback */ + while(try_op->name && strcmp(argv[1], try_op->name)) + try_op++; + + /* unsupported operation given -- ignore silently */ + if(!try_op->name || !try_op->op) + goto out; + + ret = credential_read(&cred); + if(ret) + goto out; + + /* perform credential operation */ + ret = (*try_op->op)(&cred); + + credential_write(&cred); + +out: + credential_clear(&cred); + return ret; +} diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h new file mode 100644 index 0000000..8266078 --- /dev/null +++ b/contrib/credential/helper/credential_helper.h @@ -0,0 +1,116 @@ +/* + * Copyright (C) 2012 Philipp A. Hartmann <pah@qo.cx> + * + * This file is licensed under the GPL v2, or a later version + * at the discretion of Linus. + * + * This credential struct and API is simplified from git's + * credential.{h,c} to be used within credential helper + * implementations. + */ +#ifndef CREDENTIAL_HELPER_H_INCLUDED_ +#define CREDENTIAL_HELPER_H_INCLUDED_ + +#include <stdio.h> +#include <string.h> +#include <stdarg.h> +#include <stdlib.h> +#include <errno.h> + +struct credential +{ + char *protocol; + char *host; + unsigned short port; + char *path; + char *username; + char *password; +}; + +#define CREDENTIAL_INIT \ + { NULL,NULL,0,NULL,NULL,NULL } + +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); + +typedef int (*credential_op_cb)(struct credential*); + +struct credential_operation +{ + char *name; + credential_op_cb op; +}; + +#define CREDENTIAL_OP_END \ + { NULL,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. + */ +extern struct credential_operation const credential_helper_ops[]; + +/* ---------------- common helper functions ---------------- */ + +static inline void free_password(char *password) +{ + char *c = password; + if (!password) + return; + + while (*c) *c++ = '\0'; + free(password); +} + +static inline void warning(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + fprintf(stderr, "warning: "); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n" ); + va_end(ap); +} + +static inline void error(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + fprintf(stderr, "error: "); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n" ); + va_end(ap); +} + +static inline void die(const char *fmt, ...) +{ + va_list ap; + + va_start(ap,fmt); + error(fmt, ap); + va_end(ap); + exit(EXIT_FAILURE); +} + +static inline void die_errno(int err) +{ + error("%s", strerror(err)); + exit(EXIT_FAILURE); +} + +static inline char *xstrdup(const char *str) +{ + char *ret = strdup(str); + if (!ret) + die_errno(errno); + + return ret; +} + +#endif /* CREDENTIAL_HELPER_H_INCLUDED_ */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/4] gnome-keyring: port to generic helper implementation 2012-08-23 16:57 [PATCH] contrib: GnomeKeyring support + generic helper implementation Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 1/4] contrib: add credential helper for GnomeKeyring Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 2/4] contrib: add generic credential helper Philipp A. Hartmann @ 2012-08-23 16:57 ` Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 4/4] osxkeychain: port to generic credential " Philipp A. Hartmann 2012-08-24 18:15 ` [PATCH] contrib: GnomeKeyring support + generic " Junio C Hamano 4 siblings, 0 replies; 16+ messages in thread From: Philipp A. Hartmann @ 2012-08-23 16:57 UTC (permalink / raw) To: git; +Cc: Jeff King, John Szakmeister, Philipp A. Hartmann From: "Philipp A. Hartmann" <pah@qo.cx> Use generic credential helper implementation in the GnomeKeyring credential helper. The GnomeKeyring helper has been using the generic implementation internally already and therefore only drops the duplicate code. Signed-off-by: Philipp A. Hartmann <pah@qo.cx> --- contrib/credential/gnome-keyring/Makefile | 6 +- .../gnome-keyring/git-credential-gnome-keyring.c | 243 +------------------- 2 files changed, 6 insertions(+), 243 deletions(-) diff --git a/contrib/credential/gnome-keyring/Makefile b/contrib/credential/gnome-keyring/Makefile index e6561d8..7f3ec11 100644 --- a/contrib/credential/gnome-keyring/Makefile +++ b/contrib/credential/gnome-keyring/Makefile @@ -11,11 +11,15 @@ CFLAGS = -g -O2 -Wall INCS:=$(shell pkg-config --cflags gnome-keyring-1) LIBS:=$(shell pkg-config --libs gnome-keyring-1) +HELPER:=../helper +VPATH +=$(HELPER) + SRCS:=$(MAIN).c +SRCS+=credential_helper.c OBJS:=$(SRCS:.c=.o) %.o: %.c - $(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $< + $(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) $(INCS) -o $@ -c $< $(MAIN): $(OBJS) $(CC) -o $@ $(LDFLAGS) $^ $(LIBS) diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c index 41f61c5..00244aa 100644 --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c @@ -23,114 +23,9 @@ * - ported to credential helper API by Philipp A. Hartmann */ -#include <stdio.h> -#include <string.h> -#include <stdarg.h> -#include <stdlib.h> -#include <errno.h> +#include <credential_helper.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; -}; - -#define CREDENTIAL_INIT \ - { NULL,NULL,0,NULL,NULL,NULL } - -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); - -typedef int (*credential_op_cb)(struct credential*); - -struct credential_operation -{ - char *name; - credential_op_cb op; -}; - -#define CREDENTIAL_OP_END \ - { NULL,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. - */ -struct credential_operation const credential_helper_ops[]; - -/* ---------------- common helper functions ----------------- */ - -static inline void free_password(char *password) -{ - char *c = password; - if (!password) - return; - - while (*c) *c++ = '\0'; - free(password); -} - -static inline void warning(const char *fmt, ...) -{ - va_list ap; - - va_start(ap, fmt); - fprintf(stderr, "warning: "); - vfprintf(stderr, fmt, ap); - fprintf(stderr, "\n" ); - va_end(ap); -} - -static inline void error(const char *fmt, ...) -{ - va_list ap; - - va_start(ap, fmt); - fprintf(stderr, "error: "); - vfprintf(stderr, fmt, ap); - fprintf(stderr, "\n" ); - va_end(ap); -} - -static inline void die(const char *fmt, ...) -{ - va_list ap; - - va_start(ap,fmt); - error(fmt, ap); - va_end(ap); - exit(EXIT_FAILURE); -} - -static inline void die_errno(int err) -{ - error("%s", strerror(err)); - exit(EXIT_FAILURE); -} - -static inline char *xstrdup(const char *str) -{ - char *ret = strdup(str); - if (!ret) - die_errno(errno); - - return ret; -} - -/* ----------------- GNOME Keyring functions ----------------- */ - /* create a special keyring option string, if path is given */ static char* keyring_object(struct credential *c) { @@ -307,139 +202,3 @@ struct credential_operation const credential_helper_ops[] = { "erase", keyring_erase }, CREDENTIAL_OP_END }; - -/* ------------------ credential functions ------------------ */ - -void credential_init(struct credential *c) -{ - memset(c, 0, sizeof(*c)); -} - -void credential_clear(struct credential *c) -{ - free(c->protocol); - free(c->host); - free(c->path); - free(c->username); - free_password(c->password); - - credential_init(c); -} - -int credential_read(struct credential *c) -{ - char buf[1024]; - ssize_t line_len = 0; - char *key = buf; - char *value; - - while (fgets(buf, sizeof(buf), stdin)) - { - line_len = strlen(buf); - - if(buf[line_len-1]=='\n') - buf[--line_len]='\0'; - - if(!line_len) - break; - - value = strchr(buf,'='); - if(!value) { - warning("invalid credential line: %s", key); - return -1; - } - *value++ = '\0'; - - if (!strcmp(key, "protocol")) { - free(c->protocol); - c->protocol = xstrdup(value); - } else if (!strcmp(key, "host")) { - free(c->host); - c->host = xstrdup(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); - } else if (!strcmp(key, "username")) { - free(c->username); - c->username = xstrdup(value); - } else if (!strcmp(key, "password")) { - free_password(c->password); - c->password = xstrdup(value); - while (*value) *value++ = '\0'; - } - /* - * Ignore other lines; we don't know what they mean, but - * this future-proofs us when later versions of git do - * learn new lines, and the helpers are updated to match. - */ - } - return 0; -} - -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) -{ - /* only write username/password, if set */ - credential_write_item(stdout, "username", c->username); - credential_write_item(stdout, "password", c->password); -} - -static void usage(const char *name) -{ - struct credential_operation const *try_op = credential_helper_ops; - const char *basename = strrchr(name,'/'); - - basename = (basename) ? basename + 1 : name; - fprintf(stderr, "Usage: %s <", basename); - while(try_op->name) { - fprintf(stderr,"%s",(try_op++)->name); - if(try_op->name) - fprintf(stderr,"%s","|"); - } - fprintf(stderr,"%s",">\n"); -} - -int main(int argc, char *argv[]) -{ - int ret = EXIT_SUCCESS; - - struct credential_operation const *try_op = credential_helper_ops; - struct credential cred = CREDENTIAL_INIT; - - if (!argv[1]) { - usage(argv[0]); - goto out; - } - - /* lookup operation callback */ - while(try_op->name && strcmp(argv[1], try_op->name)) - try_op++; - - /* unsupported operation given -- ignore silently */ - if(!try_op->name || !try_op->op) - goto out; - - ret = credential_read(&cred); - if(ret) - goto out; - - /* perform credential operation */ - ret = (*try_op->op)(&cred); - - credential_write(&cred); - -out: - credential_clear(&cred); - return ret; -} -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/4] osxkeychain: port to generic credential helper implementation 2012-08-23 16:57 [PATCH] contrib: GnomeKeyring support + generic helper implementation Philipp A. Hartmann ` (2 preceding siblings ...) 2012-08-23 16:57 ` [PATCH 3/4] gnome-keyring: port to generic helper implementation Philipp A. Hartmann @ 2012-08-23 16:57 ` Philipp A. Hartmann 2012-08-24 18:15 ` [PATCH] contrib: GnomeKeyring support + generic " Junio C Hamano 4 siblings, 0 replies; 16+ messages in thread From: Philipp A. Hartmann @ 2012-08-23 16:57 UTC (permalink / raw) To: git; +Cc: Jeff King, John Szakmeister, Philipp A. Hartmann From: "Philipp A. Hartmann" <pah@qo.cx> This reduces code duplication in the osxkeychain helper by basing the implementation on the generic helper implementation. Alongside, the return codes of the helper are tightened to be more consistent in corner cases and the memory containing cleartext passwords is explicitly cleared when possible. Signed-off-by: Philipp A. Hartmann <pah@qo.cx> --- contrib/credential/helper/credential_helper.h | 9 + contrib/credential/osxkeychain/Makefile | 22 ++- .../osxkeychain/git-credential-osxkeychain.c | 183 ++++++++------------ 3 files changed, 96 insertions(+), 118 deletions(-) diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h index 8266078..76b6e50 100644 --- a/contrib/credential/helper/credential_helper.h +++ b/contrib/credential/helper/credential_helper.h @@ -113,4 +113,13 @@ static inline char *xstrdup(const char *str) return ret; } +static inline char *xstrndup(const char *str, size_t len) +{ + char *ret = strndup(str,len); + if (!ret) + die_errno(errno); + + return ret; +} + #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */ diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile index 4b3a08a..64ee7c5 100644 --- a/contrib/credential/osxkeychain/Makefile +++ b/contrib/credential/osxkeychain/Makefile @@ -1,4 +1,5 @@ -all:: git-credential-osxkeychain +MAIN:=git-credential-osxkeychain +all:: $(MAIN) CC = gcc RM = rm -f @@ -7,11 +8,20 @@ CFLAGS = -g -O2 -Wall -include ../../../config.mak.autogen -include ../../../config.mak -git-credential-osxkeychain: git-credential-osxkeychain.o - $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) -Wl,-framework -Wl,Security +INCS:= +LIBS:=-Wl,-framework -Wl,Security +HELPER:=../helper +VPATH +=$(HELPER) -git-credential-osxkeychain.o: git-credential-osxkeychain.c - $(CC) -c $(CFLAGS) $< +SRCS:=$(MAIN).c +SRCS+=credential_helper.c +OBJS:=$(SRCS:.c=.o) + +%.o: %.c + $(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) $(INCS) -o $@ -c $< + +$(MAIN): $(OBJS) + $(CC) -o $@ $(LDFLAGS) $^ $(LIBS) clean: - $(RM) git-credential-osxkeychain git-credential-osxkeychain.o + @$(RM) $(MAIN) $(OBJS) diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c index 6beed12..60bd973 100644 --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c @@ -1,53 +1,40 @@ + +#include <credential_helper.h> + #include <stdio.h> #include <string.h> #include <stdlib.h> #include <Security/Security.h> static SecProtocolType protocol; -static char *host; -static char *path; -static char *username; -static char *password; -static UInt16 port; - -static void die(const char *err, ...) -{ - char msg[4096]; - va_list params; - va_start(params, err); - vsnprintf(msg, sizeof(msg), err, params); - fprintf(stderr, "%s\n", msg); - va_end(params); - exit(1); -} - -static void *xstrdup(const char *s1) -{ - void *ret = strdup(s1); - if (!ret) - die("Out of memory"); - return ret; -} #define KEYCHAIN_ITEM(x) (x ? strlen(x) : 0), x -#define KEYCHAIN_ARGS \ +#define KEYCHAIN_ARGS(c) \ NULL, /* default keychain */ \ - KEYCHAIN_ITEM(host), \ + KEYCHAIN_ITEM(c->host), \ 0, NULL, /* account domain */ \ - KEYCHAIN_ITEM(username), \ - KEYCHAIN_ITEM(path), \ - port, \ + KEYCHAIN_ITEM(c->username), \ + KEYCHAIN_ITEM(c->path), \ + (UInt16) c->port, \ protocol, \ kSecAuthenticationTypeDefault -static void write_item(const char *what, const char *buf, int len) +static int prepare_internet_password(struct credential *c) { - printf("%s=", what); - fwrite(buf, 1, len, stdout); - putchar('\n'); + if (!c->protocol) + return -1; + else if (!strcmp(c->protocol, "https")) + protocol = kSecProtocolTypeHTTPS; + else if (!strcmp(c->protocol, "http")) + protocol = kSecProtocolTypeHTTP; + else /* we don't yet handle other protocols */ + return -1; + + return 0; } -static void find_username_in_item(SecKeychainItemRef item) +static void +find_username_in_item(SecKeychainItemRef item, struct credential *c) { SecKeychainAttributeList list; SecKeychainAttribute attr; @@ -59,27 +46,37 @@ static void find_username_in_item(SecKeychainItemRef item) if (SecKeychainItemCopyContent(item, NULL, &list, NULL, NULL)) return; - write_item("username", attr.data, attr.length); + free(c->username); + c->username = xstrndup(attr.data, attr.length); + SecKeychainItemFreeContent(&list, NULL); } -static void find_internet_password(void) +static int find_internet_password(struct credential *c) { void *buf; UInt32 len; SecKeychainItemRef item; - if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS, &len, &buf, &item)) - return; + /* Silently ignore unsupported protocols */ + if (prepare_internet_password(c)) + return EXIT_SUCCESS; - write_item("password", buf, len); - if (!username) - find_username_in_item(item); + if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS(c), &len, &buf, &item)) + return EXIT_SUCCESS; + + free_password(c->password); + c->password = xstrndup(buf, len); + memset(buf,len,'\0'); + + if (!c->username) + find_username_in_item(item, c); SecKeychainItemFreeContent(NULL, buf); + return EXIT_SUCCESS; } -static void delete_internet_password(void) +static int delete_internet_password(struct credential *c) { SecKeychainItemRef item; @@ -88,86 +85,48 @@ static void delete_internet_password(void) * will give us; if you want to do something more fancy, use the * Keychain manager. */ - if (!protocol || !host) - return; + if (!c->protocol || !c->host) + return EXIT_FAILURE; - if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS, 0, NULL, &item)) - return; + /* Silently ignore unsupported protocols */ + if (prepare_internet_password(c)) + return EXIT_SUCCESS; + + if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS(c), 0, NULL, &item)) + return EXIT_SUCCESS; - SecKeychainItemDelete(item); + if (!SecKeychainItemDelete(item)) + return EXIT_SUCCESS; + + return EXIT_FAILURE; } -static void add_internet_password(void) +static int add_internet_password(struct credential *c) { /* Only store complete credentials */ - if (!protocol || !host || !username || !password) - return; + if (!c->protocol || !c->host || !c->username || !c->password) + return EXIT_FAILURE; + + if (prepare_internet_password(c)) + return EXIT_FAILURE; if (SecKeychainAddInternetPassword( - KEYCHAIN_ARGS, - KEYCHAIN_ITEM(password), + KEYCHAIN_ARGS(c), + KEYCHAIN_ITEM(c->password), NULL)) - return; -} + return EXIT_FAILURE; -static void read_credential(void) -{ - char buf[1024]; - - while (fgets(buf, sizeof(buf), stdin)) { - char *v; - - if (!strcmp(buf, "\n")) - break; - buf[strlen(buf)-1] = '\0'; - - v = strchr(buf, '='); - if (!v) - die("bad input: %s", buf); - *v++ = '\0'; - - if (!strcmp(buf, "protocol")) { - if (!strcmp(v, "https")) - protocol = kSecProtocolTypeHTTPS; - else if (!strcmp(v, "http")) - protocol = kSecProtocolTypeHTTP; - else /* we don't yet handle other protocols */ - exit(0); - } - else if (!strcmp(buf, "host")) { - char *colon = strchr(v, ':'); - if (colon) { - *colon++ = '\0'; - port = atoi(colon); - } - host = xstrdup(v); - } - else if (!strcmp(buf, "path")) - path = xstrdup(v); - else if (!strcmp(buf, "username")) - username = xstrdup(v); - else if (!strcmp(buf, "password")) - password = xstrdup(v); - } + return EXIT_SUCCESS; } -int main(int argc, const char **argv) +/* + * Table with helper operation callbacks, used by generic + * credential helper main function. + */ +struct credential_operation const credential_helper_ops[] = { - const char *usage = - "Usage: git credential-osxkeychain <get|store|erase>"; - - if (!argv[1]) - die(usage); - - read_credential(); - - if (!strcmp(argv[1], "get")) - find_internet_password(); - else if (!strcmp(argv[1], "store")) - add_internet_password(); - else if (!strcmp(argv[1], "erase")) - delete_internet_password(); - /* otherwise, ignore unknown action */ - - return 0; -} + { "get", find_internet_password }, + { "store", add_internet_password }, + { "erase", delete_internet_password }, + CREDENTIAL_OP_END +}; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation 2012-08-23 16:57 [PATCH] contrib: GnomeKeyring support + generic helper implementation Philipp A. Hartmann ` (3 preceding siblings ...) 2012-08-23 16:57 ` [PATCH 4/4] osxkeychain: port to generic credential " Philipp A. Hartmann @ 2012-08-24 18:15 ` Junio C Hamano 2012-08-24 21:33 ` Jeff King 4 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-08-24 18:15 UTC (permalink / raw) To: Philipp A. Hartmann; +Cc: git, Jeff King, John Szakmeister "Philipp A. Hartmann" <pah@qo.cx> writes: > All, > > the following patch series proposes enhancements to the credential helper > implementations in the contrib section. The detailed development history > can be found at GitHub [1]. > > The first patch adds a GnomeKeyring credential backend. The GnomeKeyring > specific parts are based on the work by John Szakmeister, who wrote the > helper originally for the initial, unpublished version of the credential > helper protocol. > > The second patch adds a generic implementation of a credential helper > based on a simplified credential API and common helper functions. Helpers > based on this implementation do not need to worry about the credential > protocol and only need to implement callback functions for the different > credential operations. > > The third and fourth patches port the existing helpers to this generic > implementation. > > Looking forward to your thoughts. It struck me somewhat odd to see a new one added as the first step in the series, and then "the generic", the third patch only to lose code from the first one, and then use the same code reduction of existing one with the last one in the series. A natural progression would have been the introduction of generic infrastructure (including the tiny bit this series had to add as part of 4/4) as the first patch, update existing OSX one to it as the second, and then build a new Gnome one on the infrastructure as the third and final patch; that way we have to review less code, too ;-). I gave it a cursory look; other than getting distracted by inconsistent coding styles here and there, the patches seemed reasonably clean and well thought out. Thanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation 2012-08-24 18:15 ` [PATCH] contrib: GnomeKeyring support + generic " Junio C Hamano @ 2012-08-24 21:33 ` Jeff King 2012-08-24 21:46 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Jeff King @ 2012-08-24 21:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Philipp A. Hartmann, git, John Szakmeister On Fri, Aug 24, 2012 at 11:15:36AM -0700, Junio C Hamano wrote: > > The third and fourth patches port the existing helpers to this generic > > implementation. > > > It struck me somewhat odd to see a new one added as the first step > in the series, and then "the generic", the third patch only to lose > code from the first one, and then use the same code reduction of > existing one with the last one in the series. A natural progression > would have been the introduction of generic infrastructure > (including the tiny bit this series had to add as part of 4/4) as > the first patch, update existing OSX one to it as the second, and > then build a new Gnome one on the infrastructure as the third and > final patch; that way we have to review less code, too ;-). I think this is partially because I talked with Phillipp off-list and was somewhat unsure how much we wanted to factor out of the helpers. My initial thought was that the protocol would be sufficiently simple that helpers would just be stand-alone and not need to share code with each other. Then the generic bits would not have to worry about being cross-platform compatible. However, the shared bits are simple enough that maybe that is not a concern. An interesting test would be to add a 5/4 porting Erik's win32 credential helper, since that is the platform least like our other ones. So I am OK with this series, but I am also OK with leaving it at patch 1, and just keeping the implementations separate. -Peff ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation 2012-08-24 21:33 ` Jeff King @ 2012-08-24 21:46 ` Junio C Hamano 2012-08-26 17:46 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-08-24 21:46 UTC (permalink / raw) To: Jeff King; +Cc: Erik Faye-Lund, Philipp A. Hartmann, git, John Szakmeister Jeff King <peff@peff.net> writes: > However, the shared bits are simple enough that maybe that is not a > concern. An interesting test would be to add a 5/4 porting Erik's win32 > credential helper, since that is the platform least like our other ones. Very true. > So I am OK with this series, but I am also OK with leaving it at patch > 1, and just keeping the implementations separate. Amen. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation 2012-08-24 21:46 ` Junio C Hamano @ 2012-08-26 17:46 ` Junio C Hamano 2012-08-26 18:16 ` Philipp A. Hartmann 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-08-26 17:46 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Jeff King, Philipp A. Hartmann, git, John Szakmeister Junio C Hamano <gitster@pobox.com> writes: > Jeff King <peff@peff.net> writes: > >> However, the shared bits are simple enough that maybe that is not a >> concern. An interesting test would be to add a 5/4 porting Erik's win32 >> credential helper, since that is the platform least like our other ones. > > Very true. > >> So I am OK with this series, but I am also OK with leaving it at patch >> 1, and just keeping the implementations separate. > > Amen. Just to make sure we do not leave loose ends, could somebody try to see if the new "generic helper" infrastructure is useful to shrink Erik's win32 credential helper implementation? If we see much code reduction and improved clarity, this refactoring may worth keeping. Otherwise it may be sufficient to drop the later ones in the series. Without knowing which, it is hard to decide. Thanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation 2012-08-26 17:46 ` Junio C Hamano @ 2012-08-26 18:16 ` Philipp A. Hartmann 2012-08-26 22:04 ` [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) Philipp A. Hartmann 0 siblings, 1 reply; 16+ messages in thread From: Philipp A. Hartmann @ 2012-08-26 18:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Jeff King, git, John Szakmeister n 26/08/12 19:46, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> Jeff King <peff@peff.net> writes: >> >>> However, the shared bits are simple enough that maybe that is not a >>> concern. An interesting test would be to add a 5/4 porting Erik's win32 >>> credential helper, since that is the platform least like our other ones. >> >> Very true. In principle, I agree that with the current set of helper implementations, the generic infrastructure may not yet pay off. >>> So I am OK with this series, but I am also OK with leaving it at patch >>> 1, and just keeping the implementations separate. >> >> Amen. > > Just to make sure we do not leave loose ends, could somebody try to > see if the new "generic helper" infrastructure is useful to shrink > Erik's win32 credential helper implementation? I'll try to give it a shot now, although I won't be able to test it. I'll send the results as a single 5/4 addition to the previous series. > If we see much code reduction and improved clarity, this refactoring > may worth keeping. Otherwise it may be sufficient to drop the later > ones in the series. Without knowing which, it is hard to decide. If we decide that the generic implementation is useful, I'll then resend the reordered series adding the generic helper first, then refactoring the existing ones and adding the new one for GnomeKeyring last. Thanks, Philipp ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) 2012-08-26 18:16 ` Philipp A. Hartmann @ 2012-08-26 22:04 ` Philipp A. Hartmann 2012-08-26 22:45 ` [PATCH 5/4 v2] " Philipp A. Hartmann 2012-08-30 18:27 ` [PATCH 5/4] " Erik Faye-Lund 0 siblings, 2 replies; 16+ messages in thread From: Philipp A. Hartmann @ 2012-08-26 22:04 UTC (permalink / raw) To: Junio C Hamano, Erik Faye-Lund; +Cc: git, Jeff King, Philipp A. Hartmann From: "Philipp A. Hartmann" <pah@qo.cx> This patch is an experiment to port the wincred helper to the generic implementation. As of know, it is completely untested. In addition to porting the helper to the generic API, this patch clears up all passwords from memory, which reduces the total amount to saved lines. Signed-off-by: Philipp A. Hartmann <pah@qo.cx> --- The porting was fairly easy, but due to the lack of a testing platform, it is completely untested. Erik: Can you try to have look? The patch is against the current 'next' with my series applied. A github repository with all the helpers can also be found at git://github.com/pah/git-credential-helper.git In order to keep things simple, I decided to add the setup of the binary pipes in the generic helper's main() already. This was the only platform-specific change needed in the helper. Other changes to the generic helper were - the addition of an URI field in the credential struct (can be reused in the GNOME implementation) - the early stop in case of incomplete credentials (simplifies handling this case in all three helpers) The overall code reduction could be increased (also in the osxkeychain helper), if we add init/cleanup functions to the generic helper. I skipped this for now. Some additions to the wincred code were needed (or at least motivated) to clean up the memory correctly. Especially for the password strings, this seems to be a sane thing to do. Since the wincred helper duplicates parts of these buffers to comply with the wchar_t-based Windows API, this required more manual free's than in the other helpers. In summary, although the platform and API looked fairly different from the other platforms, the generic code could be used without too much pain. Thanks, Philipp contrib/credential/helper/credential_helper.c | 41 ++- contrib/credential/helper/credential_helper.h | 3 +- contrib/credential/wincred/.gitignore | 1 + contrib/credential/wincred/Makefile | 19 +- .../credential/wincred/git-credential-wincred.c | 310 +++++++++----------- 5 files changed, 199 insertions(+), 175 deletions(-) create mode 100644 contrib/credential/wincred/.gitignore diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c index e99c2ec..5ddf5a5 100644 --- a/contrib/credential/helper/credential_helper.c +++ b/contrib/credential/helper/credential_helper.c @@ -11,6 +11,11 @@ #include <credential_helper.h> +#ifdef WIN32 +#include <fcntl.h> +#include <io.h> +#endif + void credential_init(struct credential *c) { memset(c, 0, sizeof(*c)); @@ -23,6 +28,7 @@ void credential_clear(struct credential *c) free(c->path); free(c->username); free_password(c->password); + free(c->uri); credential_init(c); } @@ -79,6 +85,29 @@ int credential_read(struct credential *c) * learn new lines, and the helpers are updated to match. */ } + + /* Rebuild URI from parts */ + *buf = '\0'; + if (c->protocol) { + strncat(buf, c->protocol, sizeof(buf)); + strncat(buf, "://", sizeof(buf)); + } + if (c->username) { + strncat(buf, c->username, sizeof(buf)); + strncat(buf, "@", sizeof(buf)); + } + if (c->host) + strncat(buf, c->host, sizeof(buf)); + if (c->port) { + value = buf + strlen(buf); + snprintf(value, sizeof(buf)-(value-buf), ":%hd", c->port); + } + if (c->path) { + strncat(buf, "/", sizeof(buf)); + strncat(buf, c->path, sizeof(buf)); + } + c->uri = xstrdup(buf); + return 0; } @@ -126,6 +155,12 @@ int main(int argc, char *argv[]) goto out; } +#ifdef WIN32 + /* git on Windows uses binary pipes to avoid CRLF-issues */ + _setmode(_fileno(stdin), _O_BINARY); + _setmode(_fileno(stdout), _O_BINARY); +#endif + /* lookup operation callback */ while(try_op->name && strcmp(argv[1], try_op->name)) try_op++; @@ -138,11 +173,15 @@ int main(int argc, char *argv[]) if(ret) goto out; + if (!cred.protocol || !(cred.host || cred.path)) { + ret = EXIT_FAILURE; + goto out; + } + /* perform credential operation */ ret = (*try_op->op)(&cred); credential_write(&cred); - out: credential_clear(&cred); return ret; diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h index 76b6e50..822b471 100644 --- a/contrib/credential/helper/credential_helper.h +++ b/contrib/credential/helper/credential_helper.h @@ -25,10 +25,11 @@ struct credential char *path; char *username; char *password; + char *uri; /* <protocol>://[username@][host[:port]][/path] */ }; #define CREDENTIAL_INIT \ - { NULL,NULL,0,NULL,NULL,NULL } + { NULL,NULL,0,NULL,NULL,NULL,NULL } void credential_init(struct credential *c); void credential_clear(struct credential *c); diff --git a/contrib/credential/wincred/.gitignore b/contrib/credential/wincred/.gitignore new file mode 100644 index 0000000..4780e4e --- /dev/null +++ b/contrib/credential/wincred/.gitignore @@ -0,0 +1 @@ +git-credential-wincred.exe diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile index bad45ca..ee7a8ef 100644 --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,4 +1,5 @@ -all: git-credential-wincred.exe +MAIN:=git-credential-wincred +all:: $(MAIN) CC = gcc RM = rm -f @@ -7,8 +8,18 @@ CFLAGS = -O2 -Wall -include ../../../config.mak.autogen -include ../../../config.mak -git-credential-wincred.exe : git-credential-wincred.c - $(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@ +HELPER:=../helper +VPATH +=$(HELPER) + +SRCS:=$(MAIN).c +SRCS+=credential_helper.c +OBJS:=$(SRCS:.c=.o) + +%.o: %.c + $(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) -o $@ -c $< + +$(MAIN).exe: $(OBJS) + $(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -o $@ clean: - $(RM) git-credential-wincred.exe + $(RM) $(MAIN).exe $(OBJS) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index cbaec5f..721e59f 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -7,37 +7,6 @@ #include <io.h> #include <fcntl.h> -/* common helpers */ - -static void die(const char *err, ...) -{ - char msg[4096]; - va_list params; - va_start(params, err); - vsnprintf(msg, sizeof(msg), err, params); - fprintf(stderr, "%s\n", msg); - va_end(params); - exit(1); -} - -static void *xmalloc(size_t size) -{ - void *ret = malloc(size); - if (!ret && !size) - ret = malloc(1); - if (!ret) - die("Out of memory"); - return ret; -} - -static char *xstrdup(const char *str) -{ - char *ret = strdup(str); - if (!ret) - die("Out of memory"); - return ret; -} - /* MinGW doesn't have wincred.h, so we need to define stuff */ typedef struct _CREDENTIAL_ATTRIBUTEW { @@ -84,13 +53,16 @@ static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW; static CredFreeT CredFree; static CredDeleteWT CredDeleteW; -static void load_cred_funcs(void) + +static int load_cred_funcs(void) { /* load DLLs */ advapi = LoadLibrary("advapi32.dll"); credui = LoadLibrary("credui.dll"); - if (!advapi || !credui) - die("failed to load DLLs"); + if (!advapi || !credui) { + error("failed to load DLLs"); + return EXIT_FAILURE; + } /* get function pointers */ CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW"); @@ -104,28 +76,58 @@ static void load_cred_funcs(void) CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW"); if (!CredWriteW || !CredUnPackAuthenticationBufferW || !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree || - !CredDeleteW) - die("failed to load functions"); + !CredDeleteW) { + error("failed to load functions"); + return EXIT_FAILURE; + } + return EXIT_SUCCESS; } static char target_buf[1024]; -static char *protocol, *host, *path, *username; -static WCHAR *wusername, *password, *target; +static char port_buf[8]; +static WCHAR *wusername, *wpassword; -static void write_item(const char *what, WCHAR *wbuf) +static WCHAR *utf8_to_utf16_dup(const char *str) { - char *buf; - int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL, - FALSE); - buf = xmalloc(len); - - if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE)) - die("WideCharToMultiByte failed!"); - - printf("%s=", what); - fwrite(buf, 1, len - 1, stdout); - putchar('\n'); - free(buf); + int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); + WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen); + if (!MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen)) + die("MultiByteToWideChar failed"); + return wstr; +} + +static char *utf16_to_utf8_dup(const WCHAR *wstr) +{ + int len = WideCharToMultiByte(CP_UTF8, 0, wstr, -1, NULL, 0, NULL, + FALSE); + char *str = xmalloc(len); + if (!WideCharToMultiByte(CP_UTF8, 0, wstr, -1, str, len, NULL, FALSE)) + die("WideCharToMultiByte failed"); + return str; +} + +static void free_wpassword(WCHAR *pass) +{ + WCHAR *w = pass; + if(!pass) + return; + while(*w) *w++ = L'\0'; + free(pass); +} + +static int prepare_credential(struct credential *c) +{ + if (load_cred_funcs() ) + return EXIT_FAILURE; + + if (c->username) + wusername = utf8_to_utf16_dup(c->username); + if (c->password) + wpassword = utf8_to_utf16_dup(c->password); + if (c->port) { + snprintf(port_buf,"%hd",c->port); + } + return EXIT_SUCCESS; } static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword, @@ -143,24 +145,28 @@ static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword, return 0; /* not found */ } -static int match_cred(const CREDENTIALW *cred) +static int match_cred(const CREDENTIALW *cred, const struct credential *c) { return (!wusername || !wcscmp(wusername, cred->UserName)) && - match_attr(cred, L"git_protocol", protocol) && - match_attr(cred, L"git_host", host) && - match_attr(cred, L"git_path", path); + match_attr(cred, L"git_protocol", c->protocol) && + match_attr(cred, L"git_host", c->host) && + (c->port && match_attr(cred, L"git_port", port_buf)) && + match_attr(cred, L"git_path", c->path); } -static void get_credential(void) +static int get_credential(struct credential *c) { - WCHAR *user_buf, *pass_buf; + WCHAR *user_buf = NULL, *pass_buf = NULL; DWORD user_buf_size = 0, pass_buf_size = 0; CREDENTIALW **creds, *cred = NULL; DWORD num_creds; - int i; + int i, ret = EXIT_SUCCESS; + + if (prepare_credential(c)) + return EXIT_FAILURE; if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds)) - return; + return EXIT_FAILURE; /* search for the first credential that matches username */ for (i = 0; i < num_creds; ++i) @@ -169,7 +175,7 @@ static void get_credential(void) break; } if (!cred) - return; + goto out; CredUnPackAuthenticationBufferW(0, cred->CredentialBlob, cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL, @@ -180,20 +186,29 @@ static void get_credential(void) if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob, cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL, - pass_buf, &pass_buf_size)) - die("CredUnPackAuthenticationBuffer failed"); - - CredFree(creds); + pass_buf, &pass_buf_size)) { + error("CredUnPackAuthenticationBuffer failed"); + ret = EXIT_FAILURE; + goto out; + } /* zero-terminate (sizes include zero-termination) */ user_buf[user_buf_size - 1] = L'\0'; pass_buf[pass_buf_size - 1] = L'\0'; - write_item("username", user_buf); - write_item("password", pass_buf); + if (!c->username) + c->username = utf16_to_utf8_dup(user_buf); + free_password(c->password); + c->password = utf16_to_utf8_dup(pass_buf); + +out: + CredFree(creds); free(user_buf); - free(pass_buf); + free(wusername); + free_wpassword(pass_buf); + + return ret; } static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword, @@ -205,29 +220,42 @@ static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword, attr->Value = (LPBYTE)value; } -static void store_credential(void) +static int store_credential(struct credential *c) { CREDENTIALW cred; BYTE *auth_buf; DWORD auth_buf_size = 0; CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES]; + WCHAR *wtarget; + int ret = EXIT_SUCCESS; - if (!wusername || !password) - return; + if (prepare_credential(c)) + return EXIT_FAILURE; + + /* prepare 'target', the unique key for the credential */ + strncat(target_buf,"git:",sizeof(target_buf)); + strncat(target_buf,c->uri,sizeof(target_buf)); + wtarget = utf8_to_utf16_dup(target_buf); + + if (!wusername || !wpassword) + return EXIT_FAILURE; /* query buffer size */ - CredPackAuthenticationBufferW(0, wusername, password, + CredPackAuthenticationBufferW(0, wusername, wpassword, NULL, &auth_buf_size); auth_buf = xmalloc(auth_buf_size); - if (!CredPackAuthenticationBufferW(0, wusername, password, - auth_buf, &auth_buf_size)) - die("CredPackAuthenticationBuffer failed"); + if (!CredPackAuthenticationBufferW(0, wusername, wpassword, + auth_buf, &auth_buf_size)) { + error("CredPackAuthenticationBuffer failed"); + ret = EXIT_FAILURE; + goto out; + } cred.Flags = 0; cred.Type = CRED_TYPE_GENERIC; - cred.TargetName = target; + cred.TargetName = wtarget; cred.Comment = L"saved by git-credential-wincred"; cred.CredentialBlobSize = auth_buf_size; cred.CredentialBlob = auth_buf; @@ -237,121 +265,65 @@ static void store_credential(void) cred.TargetAlias = NULL; cred.UserName = wusername; - write_attr(attrs, L"git_protocol", protocol); + write_attr(attrs, L"git_protocol", c->protocol); - if (host) { - write_attr(attrs + cred.AttributeCount, L"git_host", host); + if (c->host) { + write_attr(attrs + cred.AttributeCount, L"git_host", c->host); cred.AttributeCount++; } - if (path) { - write_attr(attrs + cred.AttributeCount, L"git_path", path); + if (c->port) { + write_attr(attrs + cred.AttributeCount, L"git_port", port_buf); cred.AttributeCount++; } - if (!CredWriteW(&cred, 0)) - die("CredWrite failed"); + if (c->path) { + write_attr(attrs + cred.AttributeCount, L"git_path", c->path); + cred.AttributeCount++; + } + + if (!CredWriteW(&cred, 0)) { + error("CredWrite failed"); + ret = EXIT_FAILURE; + } + +out: + free(auth_buf); + free(wusername); + free_wpassword(wpassword); + free(wtarget); + return ret; } -static void erase_credential(void) +static int erase_credential(struct credential *c) { CREDENTIALW **creds; DWORD num_creds; int i; + if (prepare_credential(c)) + return EXIT_FAILURE; + if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds)) - return; + return EXIT_FAILURE; for (i = 0; i < num_creds; ++i) { - if (match_cred(creds[i])) + if (match_cred(creds[i],c)) CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0); } CredFree(creds); + return EXIT_SUCCESS; } -static WCHAR *utf8_to_utf16_dup(const char *str) -{ - int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); - WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen); - MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen); - return wstr; -} - -static void read_credential(void) -{ - char buf[1024]; - - while (fgets(buf, sizeof(buf), stdin)) { - char *v; - - if (!strcmp(buf, "\n")) - break; - buf[strlen(buf)-1] = '\0'; - - v = strchr(buf, '='); - if (!v) - die("bad input: %s", buf); - *v++ = '\0'; - - if (!strcmp(buf, "protocol")) - protocol = xstrdup(v); - else if (!strcmp(buf, "host")) - host = xstrdup(v); - else if (!strcmp(buf, "path")) - path = xstrdup(v); - else if (!strcmp(buf, "username")) { - username = xstrdup(v); - wusername = utf8_to_utf16_dup(v); - } else if (!strcmp(buf, "password")) - password = utf8_to_utf16_dup(v); - else - die("unrecognized input"); - } -} - -int main(int argc, char *argv[]) +/* + * Table with helper operation callbacks, used by generic + * credential helper main function. + */ +struct credential_operation const credential_helper_ops[] = { - const char *usage = - "Usage: git credential-wincred <get|store|erase>\n"; - - if (!argv[1]) - die(usage); - - /* git use binary pipes to avoid CRLF-issues */ - _setmode(_fileno(stdin), _O_BINARY); - _setmode(_fileno(stdout), _O_BINARY); - - read_credential(); - - load_cred_funcs(); - - if (!protocol || !(host || path)) - return 0; - - /* prepare 'target', the unique key for the credential */ - strncat(target_buf, "git:", sizeof(target_buf)); - strncat(target_buf, protocol, sizeof(target_buf)); - strncat(target_buf, "://", sizeof(target_buf)); - if (username) { - strncat(target_buf, username, sizeof(target_buf)); - strncat(target_buf, "@", sizeof(target_buf)); - } - if (host) - strncat(target_buf, host, sizeof(target_buf)); - if (path) { - strncat(target_buf, "/", sizeof(target_buf)); - strncat(target_buf, path, sizeof(target_buf)); - } - - target = utf8_to_utf16_dup(target_buf); - - if (!strcmp(argv[1], "get")) - get_credential(); - else if (!strcmp(argv[1], "store")) - store_credential(); - else if (!strcmp(argv[1], "erase")) - erase_credential(); - /* otherwise, ignore unknown action */ - return 0; -} + { "get", get_credential }, + { "store", store_credential }, + { "erase", erase_credential }, + CREDENTIAL_OP_END +}; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/4 v2] wincred: port to generic credential helper (UNTESTED) 2012-08-26 22:04 ` [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) Philipp A. Hartmann @ 2012-08-26 22:45 ` Philipp A. Hartmann 2012-08-30 18:27 ` [PATCH 5/4] " Erik Faye-Lund 1 sibling, 0 replies; 16+ messages in thread From: Philipp A. Hartmann @ 2012-08-26 22:45 UTC (permalink / raw) To: Junio C Hamano, Erik Faye-Lund; +Cc: git, Jeff King, Philipp A. Hartmann From: "Philipp A. Hartmann" <pah@qo.cx> This patch is an experiment to port the wincred helper to the generic implementation. As of know, it is completely untested. In addition to porting the helper to the generic API, this patch clears up all passwords from memory, which reduces the total amount to saved lines. This version adds the missing xmalloc() implementation. Signed-off-by: Philipp A. Hartmann <pah@qo.cx> --- The previous version of the patch somehow lacked the xmalloc implementation moved to credential_helper.h. Sorry for the inconvenience. Apart from being still untested, this version at least matches my tree at git://github.com/pah/git-credential-helper.git. contrib/credential/helper/credential_helper.c | 41 ++- contrib/credential/helper/credential_helper.h | 14 +- contrib/credential/wincred/.gitignore | 1 + contrib/credential/wincred/Makefile | 19 +- .../credential/wincred/git-credential-wincred.c | 310 +++++++++----------- 5 files changed, 210 insertions(+), 175 deletions(-) create mode 100644 contrib/credential/wincred/.gitignore diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c index e99c2ec..5ddf5a5 100644 --- a/contrib/credential/helper/credential_helper.c +++ b/contrib/credential/helper/credential_helper.c @@ -11,6 +11,11 @@ #include <credential_helper.h> +#ifdef WIN32 +#include <fcntl.h> +#include <io.h> +#endif + void credential_init(struct credential *c) { memset(c, 0, sizeof(*c)); @@ -23,6 +28,7 @@ void credential_clear(struct credential *c) free(c->path); free(c->username); free_password(c->password); + free(c->uri); credential_init(c); } @@ -79,6 +85,29 @@ int credential_read(struct credential *c) * learn new lines, and the helpers are updated to match. */ } + + /* Rebuild URI from parts */ + *buf = '\0'; + if (c->protocol) { + strncat(buf, c->protocol, sizeof(buf)); + strncat(buf, "://", sizeof(buf)); + } + if (c->username) { + strncat(buf, c->username, sizeof(buf)); + strncat(buf, "@", sizeof(buf)); + } + if (c->host) + strncat(buf, c->host, sizeof(buf)); + if (c->port) { + value = buf + strlen(buf); + snprintf(value, sizeof(buf)-(value-buf), ":%hd", c->port); + } + if (c->path) { + strncat(buf, "/", sizeof(buf)); + strncat(buf, c->path, sizeof(buf)); + } + c->uri = xstrdup(buf); + return 0; } @@ -126,6 +155,12 @@ int main(int argc, char *argv[]) goto out; } +#ifdef WIN32 + /* git on Windows uses binary pipes to avoid CRLF-issues */ + _setmode(_fileno(stdin), _O_BINARY); + _setmode(_fileno(stdout), _O_BINARY); +#endif + /* lookup operation callback */ while(try_op->name && strcmp(argv[1], try_op->name)) try_op++; @@ -138,11 +173,15 @@ int main(int argc, char *argv[]) if(ret) goto out; + if (!cred.protocol || !(cred.host || cred.path)) { + ret = EXIT_FAILURE; + goto out; + } + /* perform credential operation */ ret = (*try_op->op)(&cred); credential_write(&cred); - out: credential_clear(&cred); return ret; diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h index 76b6e50..7e73fc6 100644 --- a/contrib/credential/helper/credential_helper.h +++ b/contrib/credential/helper/credential_helper.h @@ -25,10 +25,11 @@ struct credential char *path; char *username; char *password; + char *uri; /* <protocol>://[username@][host[:port]][/path] */ }; #define CREDENTIAL_INIT \ - { NULL,NULL,0,NULL,NULL,NULL } + { NULL,NULL,0,NULL,NULL,NULL,NULL } void credential_init(struct credential *c); void credential_clear(struct credential *c); @@ -104,6 +105,17 @@ static inline void die_errno(int err) exit(EXIT_FAILURE); } +static inline void *xmalloc(size_t size) +{ + void *ret = malloc(size); + if (!ret && !size) + ret = malloc(1); + if (!ret) + die_errno(errno); + + return ret; +} + static inline char *xstrdup(const char *str) { char *ret = strdup(str); diff --git a/contrib/credential/wincred/.gitignore b/contrib/credential/wincred/.gitignore new file mode 100644 index 0000000..4780e4e --- /dev/null +++ b/contrib/credential/wincred/.gitignore @@ -0,0 +1 @@ +git-credential-wincred.exe diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile index bad45ca..ee7a8ef 100644 --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,4 +1,5 @@ -all: git-credential-wincred.exe +MAIN:=git-credential-wincred +all:: $(MAIN) CC = gcc RM = rm -f @@ -7,8 +8,18 @@ CFLAGS = -O2 -Wall -include ../../../config.mak.autogen -include ../../../config.mak -git-credential-wincred.exe : git-credential-wincred.c - $(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@ +HELPER:=../helper +VPATH +=$(HELPER) + +SRCS:=$(MAIN).c +SRCS+=credential_helper.c +OBJS:=$(SRCS:.c=.o) + +%.o: %.c + $(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) -o $@ -c $< + +$(MAIN).exe: $(OBJS) + $(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -o $@ clean: - $(RM) git-credential-wincred.exe + $(RM) $(MAIN).exe $(OBJS) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index cbaec5f..721e59f 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -7,37 +7,6 @@ #include <io.h> #include <fcntl.h> -/* common helpers */ - -static void die(const char *err, ...) -{ - char msg[4096]; - va_list params; - va_start(params, err); - vsnprintf(msg, sizeof(msg), err, params); - fprintf(stderr, "%s\n", msg); - va_end(params); - exit(1); -} - -static void *xmalloc(size_t size) -{ - void *ret = malloc(size); - if (!ret && !size) - ret = malloc(1); - if (!ret) - die("Out of memory"); - return ret; -} - -static char *xstrdup(const char *str) -{ - char *ret = strdup(str); - if (!ret) - die("Out of memory"); - return ret; -} - /* MinGW doesn't have wincred.h, so we need to define stuff */ typedef struct _CREDENTIAL_ATTRIBUTEW { @@ -84,13 +53,16 @@ static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW; static CredFreeT CredFree; static CredDeleteWT CredDeleteW; -static void load_cred_funcs(void) + +static int load_cred_funcs(void) { /* load DLLs */ advapi = LoadLibrary("advapi32.dll"); credui = LoadLibrary("credui.dll"); - if (!advapi || !credui) - die("failed to load DLLs"); + if (!advapi || !credui) { + error("failed to load DLLs"); + return EXIT_FAILURE; + } /* get function pointers */ CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW"); @@ -104,28 +76,58 @@ static void load_cred_funcs(void) CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW"); if (!CredWriteW || !CredUnPackAuthenticationBufferW || !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree || - !CredDeleteW) - die("failed to load functions"); + !CredDeleteW) { + error("failed to load functions"); + return EXIT_FAILURE; + } + return EXIT_SUCCESS; } static char target_buf[1024]; -static char *protocol, *host, *path, *username; -static WCHAR *wusername, *password, *target; +static char port_buf[8]; +static WCHAR *wusername, *wpassword; -static void write_item(const char *what, WCHAR *wbuf) +static WCHAR *utf8_to_utf16_dup(const char *str) { - char *buf; - int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL, - FALSE); - buf = xmalloc(len); - - if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE)) - die("WideCharToMultiByte failed!"); - - printf("%s=", what); - fwrite(buf, 1, len - 1, stdout); - putchar('\n'); - free(buf); + int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); + WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen); + if (!MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen)) + die("MultiByteToWideChar failed"); + return wstr; +} + +static char *utf16_to_utf8_dup(const WCHAR *wstr) +{ + int len = WideCharToMultiByte(CP_UTF8, 0, wstr, -1, NULL, 0, NULL, + FALSE); + char *str = xmalloc(len); + if (!WideCharToMultiByte(CP_UTF8, 0, wstr, -1, str, len, NULL, FALSE)) + die("WideCharToMultiByte failed"); + return str; +} + +static void free_wpassword(WCHAR *pass) +{ + WCHAR *w = pass; + if(!pass) + return; + while(*w) *w++ = L'\0'; + free(pass); +} + +static int prepare_credential(struct credential *c) +{ + if (load_cred_funcs() ) + return EXIT_FAILURE; + + if (c->username) + wusername = utf8_to_utf16_dup(c->username); + if (c->password) + wpassword = utf8_to_utf16_dup(c->password); + if (c->port) { + snprintf(port_buf,"%hd",c->port); + } + return EXIT_SUCCESS; } static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword, @@ -143,24 +145,28 @@ static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword, return 0; /* not found */ } -static int match_cred(const CREDENTIALW *cred) +static int match_cred(const CREDENTIALW *cred, const struct credential *c) { return (!wusername || !wcscmp(wusername, cred->UserName)) && - match_attr(cred, L"git_protocol", protocol) && - match_attr(cred, L"git_host", host) && - match_attr(cred, L"git_path", path); + match_attr(cred, L"git_protocol", c->protocol) && + match_attr(cred, L"git_host", c->host) && + (c->port && match_attr(cred, L"git_port", port_buf)) && + match_attr(cred, L"git_path", c->path); } -static void get_credential(void) +static int get_credential(struct credential *c) { - WCHAR *user_buf, *pass_buf; + WCHAR *user_buf = NULL, *pass_buf = NULL; DWORD user_buf_size = 0, pass_buf_size = 0; CREDENTIALW **creds, *cred = NULL; DWORD num_creds; - int i; + int i, ret = EXIT_SUCCESS; + + if (prepare_credential(c)) + return EXIT_FAILURE; if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds)) - return; + return EXIT_FAILURE; /* search for the first credential that matches username */ for (i = 0; i < num_creds; ++i) @@ -169,7 +175,7 @@ static void get_credential(void) break; } if (!cred) - return; + goto out; CredUnPackAuthenticationBufferW(0, cred->CredentialBlob, cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL, @@ -180,20 +186,29 @@ static void get_credential(void) if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob, cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL, - pass_buf, &pass_buf_size)) - die("CredUnPackAuthenticationBuffer failed"); - - CredFree(creds); + pass_buf, &pass_buf_size)) { + error("CredUnPackAuthenticationBuffer failed"); + ret = EXIT_FAILURE; + goto out; + } /* zero-terminate (sizes include zero-termination) */ user_buf[user_buf_size - 1] = L'\0'; pass_buf[pass_buf_size - 1] = L'\0'; - write_item("username", user_buf); - write_item("password", pass_buf); + if (!c->username) + c->username = utf16_to_utf8_dup(user_buf); + free_password(c->password); + c->password = utf16_to_utf8_dup(pass_buf); + +out: + CredFree(creds); free(user_buf); - free(pass_buf); + free(wusername); + free_wpassword(pass_buf); + + return ret; } static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword, @@ -205,29 +220,42 @@ static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword, attr->Value = (LPBYTE)value; } -static void store_credential(void) +static int store_credential(struct credential *c) { CREDENTIALW cred; BYTE *auth_buf; DWORD auth_buf_size = 0; CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES]; + WCHAR *wtarget; + int ret = EXIT_SUCCESS; - if (!wusername || !password) - return; + if (prepare_credential(c)) + return EXIT_FAILURE; + + /* prepare 'target', the unique key for the credential */ + strncat(target_buf,"git:",sizeof(target_buf)); + strncat(target_buf,c->uri,sizeof(target_buf)); + wtarget = utf8_to_utf16_dup(target_buf); + + if (!wusername || !wpassword) + return EXIT_FAILURE; /* query buffer size */ - CredPackAuthenticationBufferW(0, wusername, password, + CredPackAuthenticationBufferW(0, wusername, wpassword, NULL, &auth_buf_size); auth_buf = xmalloc(auth_buf_size); - if (!CredPackAuthenticationBufferW(0, wusername, password, - auth_buf, &auth_buf_size)) - die("CredPackAuthenticationBuffer failed"); + if (!CredPackAuthenticationBufferW(0, wusername, wpassword, + auth_buf, &auth_buf_size)) { + error("CredPackAuthenticationBuffer failed"); + ret = EXIT_FAILURE; + goto out; + } cred.Flags = 0; cred.Type = CRED_TYPE_GENERIC; - cred.TargetName = target; + cred.TargetName = wtarget; cred.Comment = L"saved by git-credential-wincred"; cred.CredentialBlobSize = auth_buf_size; cred.CredentialBlob = auth_buf; @@ -237,121 +265,65 @@ static void store_credential(void) cred.TargetAlias = NULL; cred.UserName = wusername; - write_attr(attrs, L"git_protocol", protocol); + write_attr(attrs, L"git_protocol", c->protocol); - if (host) { - write_attr(attrs + cred.AttributeCount, L"git_host", host); + if (c->host) { + write_attr(attrs + cred.AttributeCount, L"git_host", c->host); cred.AttributeCount++; } - if (path) { - write_attr(attrs + cred.AttributeCount, L"git_path", path); + if (c->port) { + write_attr(attrs + cred.AttributeCount, L"git_port", port_buf); cred.AttributeCount++; } - if (!CredWriteW(&cred, 0)) - die("CredWrite failed"); + if (c->path) { + write_attr(attrs + cred.AttributeCount, L"git_path", c->path); + cred.AttributeCount++; + } + + if (!CredWriteW(&cred, 0)) { + error("CredWrite failed"); + ret = EXIT_FAILURE; + } + +out: + free(auth_buf); + free(wusername); + free_wpassword(wpassword); + free(wtarget); + return ret; } -static void erase_credential(void) +static int erase_credential(struct credential *c) { CREDENTIALW **creds; DWORD num_creds; int i; + if (prepare_credential(c)) + return EXIT_FAILURE; + if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds)) - return; + return EXIT_FAILURE; for (i = 0; i < num_creds; ++i) { - if (match_cred(creds[i])) + if (match_cred(creds[i],c)) CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0); } CredFree(creds); + return EXIT_SUCCESS; } -static WCHAR *utf8_to_utf16_dup(const char *str) -{ - int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); - WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen); - MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen); - return wstr; -} - -static void read_credential(void) -{ - char buf[1024]; - - while (fgets(buf, sizeof(buf), stdin)) { - char *v; - - if (!strcmp(buf, "\n")) - break; - buf[strlen(buf)-1] = '\0'; - - v = strchr(buf, '='); - if (!v) - die("bad input: %s", buf); - *v++ = '\0'; - - if (!strcmp(buf, "protocol")) - protocol = xstrdup(v); - else if (!strcmp(buf, "host")) - host = xstrdup(v); - else if (!strcmp(buf, "path")) - path = xstrdup(v); - else if (!strcmp(buf, "username")) { - username = xstrdup(v); - wusername = utf8_to_utf16_dup(v); - } else if (!strcmp(buf, "password")) - password = utf8_to_utf16_dup(v); - else - die("unrecognized input"); - } -} - -int main(int argc, char *argv[]) +/* + * Table with helper operation callbacks, used by generic + * credential helper main function. + */ +struct credential_operation const credential_helper_ops[] = { - const char *usage = - "Usage: git credential-wincred <get|store|erase>\n"; - - if (!argv[1]) - die(usage); - - /* git use binary pipes to avoid CRLF-issues */ - _setmode(_fileno(stdin), _O_BINARY); - _setmode(_fileno(stdout), _O_BINARY); - - read_credential(); - - load_cred_funcs(); - - if (!protocol || !(host || path)) - return 0; - - /* prepare 'target', the unique key for the credential */ - strncat(target_buf, "git:", sizeof(target_buf)); - strncat(target_buf, protocol, sizeof(target_buf)); - strncat(target_buf, "://", sizeof(target_buf)); - if (username) { - strncat(target_buf, username, sizeof(target_buf)); - strncat(target_buf, "@", sizeof(target_buf)); - } - if (host) - strncat(target_buf, host, sizeof(target_buf)); - if (path) { - strncat(target_buf, "/", sizeof(target_buf)); - strncat(target_buf, path, sizeof(target_buf)); - } - - target = utf8_to_utf16_dup(target_buf); - - if (!strcmp(argv[1], "get")) - get_credential(); - else if (!strcmp(argv[1], "store")) - store_credential(); - else if (!strcmp(argv[1], "erase")) - erase_credential(); - /* otherwise, ignore unknown action */ - return 0; -} + { "get", get_credential }, + { "store", store_credential }, + { "erase", erase_credential }, + CREDENTIAL_OP_END +}; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) 2012-08-26 22:04 ` [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) Philipp A. Hartmann 2012-08-26 22:45 ` [PATCH 5/4 v2] " Philipp A. Hartmann @ 2012-08-30 18:27 ` Erik Faye-Lund 2012-08-30 20:11 ` Junio C Hamano 2012-08-31 15:44 ` Erik Faye-Lund 1 sibling, 2 replies; 16+ messages in thread From: Erik Faye-Lund @ 2012-08-30 18:27 UTC (permalink / raw) To: Philipp A. Hartmann; +Cc: Junio C Hamano, git, Jeff King On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@qo.cx> wrote: > From: "Philipp A. Hartmann" <pah@qo.cx> > > This patch is an experiment to port the wincred helper > to the generic implementation. As of know, it is > completely untested. > > In addition to porting the helper to the generic API, > this patch clears up all passwords from memory, which > reduces the total amount to saved lines. > > Signed-off-by: Philipp A. Hartmann <pah@qo.cx> > --- > > The porting was fairly easy, but due to the lack of a testing > platform, it is completely untested. > > Erik: Can you try to have look? Sorry for the late reply, I'm currently in bed with pneumonia. But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors. But with this patch on top, it seems to at least compile OK: ---8<--- diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h index 7e73fc6..13b611e 100644 --- a/contrib/credential/helper/credential_helper.h +++ b/contrib/credential/helper/credential_helper.h @@ -125,6 +125,7 @@ static inline char *xstrdup(const char *str) return ret; } +#ifndef NO_STRNDUP static inline char *xstrndup(const char *str, size_t len) { char *ret = strndup(str,len); @@ -133,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len) return ret; } +#endif #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */ diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile index ee7a8ef..3900322 100644 --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,9 +1,10 @@ MAIN:=git-credential-wincred -all:: $(MAIN) +all:: $(MAIN).exe CC = gcc RM = rm -f CFLAGS = -O2 -Wall +CPPFLAGS = -DNO_STRNDUP -include ../../../config.mak.autogen -include ../../../config.mak diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index 721e59f..e26ba47 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -6,6 +6,7 @@ #include <stdio.h> #include <io.h> #include <fcntl.h> +#include <credential_helper.h> /* MinGW doesn't have wincred.h, so we need to define stuff */ @@ -124,9 +125,8 @@ static int prepare_credential(struct credential *c) wusername = utf8_to_utf16_dup(c->username); if (c->password) wpassword = utf8_to_utf16_dup(c->password); - if (c->port) { - snprintf(port_buf,"%hd",c->port); - } + if (c->port) + snprintf(port_buf, sizeof(port_buf), "%hd", c->port); return EXIT_SUCCESS; } @@ -170,7 +170,7 @@ static int get_credential(struct credential *c) /* search for the first credential that matches username */ for (i = 0; i < num_creds; ++i) - if (match_cred(creds[i])) { + if (match_cred(creds[i], c)) { cred = creds[i]; break; } ---8<--- However, I haven't been able to successfully run the tests on the result. When I did, I got this error: ---8<--- rm: cannot remove `t/trash directory.t0303-credential-external/stderr': Permissi on denied rm: cannot remove `t/trash directory.t0303-credential-external/stdout': Permissi on denied rm: cannot remove directory `t/trash directory.t0303-credential-external': Direc tory not empty ---8<--- And I'm not currently feeling up to debugging stuck processes or whatever it is. ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) 2012-08-30 18:27 ` [PATCH 5/4] " Erik Faye-Lund @ 2012-08-30 20:11 ` Junio C Hamano 2012-08-31 15:44 ` Erik Faye-Lund 1 sibling, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2012-08-30 20:11 UTC (permalink / raw) To: kusmabite; +Cc: Philipp A. Hartmann, git, Jeff King Erik Faye-Lund <kusmabite@gmail.com> writes: > On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@qo.cx> wrote: >> From: "Philipp A. Hartmann" <pah@qo.cx> >> >> This patch is an experiment to port the wincred helper >> to the generic implementation. As of know, it is >> completely untested. >> >> In addition to porting the helper to the generic API, >> this patch clears up all passwords from memory, which >> reduces the total amount to saved lines. >> >> Signed-off-by: Philipp A. Hartmann <pah@qo.cx> >> --- >> >> The porting was fairly easy, but due to the lack of a testing >> platform, it is completely untested. >> >> Erik: Can you try to have look? > > Sorry for the late reply, I'm currently in bed with pneumonia. > > But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors. Thanks, and get well soon. For now, let's not worry about merging the "let's share code across helpers" bit that comes later in Philipp's series. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) 2012-08-30 18:27 ` [PATCH 5/4] " Erik Faye-Lund 2012-08-30 20:11 ` Junio C Hamano @ 2012-08-31 15:44 ` Erik Faye-Lund 2012-09-01 12:42 ` [PATCH 5/4 v3] wincred: port to generic credential helper Philipp A. Hartmann 1 sibling, 1 reply; 16+ messages in thread From: Erik Faye-Lund @ 2012-08-31 15:44 UTC (permalink / raw) To: Philipp A. Hartmann; +Cc: Junio C Hamano, git, Jeff King On Thu, Aug 30, 2012 at 8:27 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote: > On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@qo.cx> wrote: >> From: "Philipp A. Hartmann" <pah@qo.cx> >> >> This patch is an experiment to port the wincred helper >> to the generic implementation. As of know, it is >> completely untested. >> >> In addition to porting the helper to the generic API, >> this patch clears up all passwords from memory, which >> reduces the total amount to saved lines. >> >> Signed-off-by: Philipp A. Hartmann <pah@qo.cx> >> --- >> >> The porting was fairly easy, but due to the lack of a testing >> platform, it is completely untested. >> >> Erik: Can you try to have look? > > Sorry for the late reply, I'm currently in bed with pneumonia. > > But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors. > > But with this patch on top, it seems to at least compile OK: > > ---8<--- > diff --git a/contrib/credential/helper/credential_helper.h > b/contrib/credential/helper/credential_helper.h > index 7e73fc6..13b611e 100644 > --- a/contrib/credential/helper/credential_helper.h > +++ b/contrib/credential/helper/credential_helper.h > @@ -125,6 +125,7 @@ static inline char *xstrdup(const char *str) > return ret; > } > > +#ifndef NO_STRNDUP > static inline char *xstrndup(const char *str, size_t len) > { > char *ret = strndup(str,len); > @@ -133,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len) > > return ret; > } > +#endif > > #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */ > diff --git a/contrib/credential/wincred/Makefile > b/contrib/credential/wincred/Makefile > index ee7a8ef..3900322 100644 > --- a/contrib/credential/wincred/Makefile > +++ b/contrib/credential/wincred/Makefile > @@ -1,9 +1,10 @@ > MAIN:=git-credential-wincred > -all:: $(MAIN) > +all:: $(MAIN).exe > > CC = gcc > RM = rm -f > CFLAGS = -O2 -Wall > +CPPFLAGS = -DNO_STRNDUP > > -include ../../../config.mak.autogen > -include ../../../config.mak > diff --git a/contrib/credential/wincred/git-credential-wincred.c > b/contrib/credential/wincred/git-credential-wincred.c > index 721e59f..e26ba47 100644 > --- a/contrib/credential/wincred/git-credential-wincred.c > +++ b/contrib/credential/wincred/git-credential-wincred.c > @@ -6,6 +6,7 @@ > #include <stdio.h> > #include <io.h> > #include <fcntl.h> > +#include <credential_helper.h> > > /* MinGW doesn't have wincred.h, so we need to define stuff */ > > @@ -124,9 +125,8 @@ static int prepare_credential(struct credential *c) > wusername = utf8_to_utf16_dup(c->username); > if (c->password) > wpassword = utf8_to_utf16_dup(c->password); > - if (c->port) { > - snprintf(port_buf,"%hd",c->port); > - } > + if (c->port) > + snprintf(port_buf, sizeof(port_buf), "%hd", c->port); > return EXIT_SUCCESS; > } > > @@ -170,7 +170,7 @@ static int get_credential(struct credential *c) > > /* search for the first credential that matches username */ > for (i = 0; i < num_creds; ++i) > - if (match_cred(creds[i])) { > + if (match_cred(creds[i], c)) { > cred = creds[i]; > break; > } > ---8<--- > > However, I haven't been able to successfully run the tests on the > result. When I did, I got this error: > > ---8<--- > rm: cannot remove `t/trash directory.t0303-credential-external/stderr': Permissi > on denied > rm: cannot remove `t/trash directory.t0303-credential-external/stdout': Permissi > on denied > rm: cannot remove directory `t/trash directory.t0303-credential-external': Direc > tory not empty > ---8<--- > > And I'm not currently feeling up to debugging stuck processes or whatever it is. OK, that was due to a stuck process, and after killing it I got to test properly. However, three tests fail now: $ (cd t && GIT_TEST_CREDENTIAL_HELPER=wincred ./t0303-credential-external.sh) ok 1 - helper (wincred) has no existing data ok 2 - helper (wincred) stores password not ok - 3 helper (wincred) can retrieve password # # check fill $HELPER <<-\EOF # protocol=https # host=example.com # -- # protocol=https # host=example.com # username=store-user # password=store-pass # -- # EOF # ok 4 - helper (wincred) requires matching protocol ok 5 - helper (wincred) requires matching host ok 6 - helper (wincred) requires matching username ok 7 - helper (wincred) requires matching path ok 8 - helper (wincred) can forget host not ok - 9 helper (wincred) can store multiple users # # check approve $HELPER <<-\EOF && # protocol=https # host=example.com # username=user1 # password=pass1 # EOF # check approve $HELPER <<-\EOF && # protocol=https # host=example.com # username=user2 # password=pass2 # EOF # check fill $HELPER <<-\EOF && # protocol=https # host=example.com # username=user1 # -- # protocol=https # host=example.com # username=user1 # password=pass1 # EOF # check fill $HELPER <<-\EOF # protocol=https # host=example.com # username=user2 # -- # protocol=https # host=example.com # username=user2 # password=pass2 # EOF # ok 10 - helper (wincred) can forget user not ok - 11 helper (wincred) remembers other user # # check fill $HELPER <<-\EOF # protocol=https # host=example.com # username=user2 # -- # protocol=https # host=example.com # username=user2 # password=pass2 # EOF # # skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set) # failed 3 among 11 test(s) 1..11 So, something else is up as well. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/4 v3] wincred: port to generic credential helper 2012-08-31 15:44 ` Erik Faye-Lund @ 2012-09-01 12:42 ` Philipp A. Hartmann 0 siblings, 0 replies; 16+ messages in thread From: Philipp A. Hartmann @ 2012-09-01 12:42 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Jeff King, git, Philipp A. Hartmann From: "Philipp A. Hartmann" <pah@qo.cx> In addition to porting the helper to the generic API, this patch clears up all passwords from memory, which reduces the total amount to saved lines. This version will now pass t0303 if you do GIT_TEST_CREDENTIAL_HELPER=wincred \ ./t0303-credential-external.sh Signed-off-by: Philipp A. Hartmann <pah@qo.cx> --- Erik, thanks for testing! I finally managed to test the wincred implementation against t0303 and found another stupid bug in the credential matching of the port: - (c->port && match_attr(cred, L"git_port", port_buf)) && + (!c->port || match_attr(cred, L"git_port", port_buf)) && Together with the fixes from Erik, the tests (and additional manual tests with explicit ports, etc) now pass. If this makes the generic implementation useful enough, I'd re-roll at least the patches #2-#5 (#1 is already merged to next) to a new series by adding the "final" generic helper in one step and adding the ports afterwards. [Apologies for reposting, I somehow broke the "From:" header in my original mail.] contrib/credential/helper/credential_helper.c | 41 ++- contrib/credential/helper/credential_helper.h | 16 +- contrib/credential/wincred/.gitignore | 1 + contrib/credential/wincred/Makefile | 20 +- .../credential/wincred/git-credential-wincred.c | 312 +++++++++----------- 5 files changed, 214 insertions(+), 176 deletions(-) create mode 100644 contrib/credential/wincred/.gitignore diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c index e99c2ec..15536a6 100644 --- a/contrib/credential/helper/credential_helper.c +++ b/contrib/credential/helper/credential_helper.c @@ -11,6 +11,11 @@ #include <credential_helper.h> +#ifdef WIN32 +#include <fcntl.h> +#include <io.h> +#endif + void credential_init(struct credential *c) { memset(c, 0, sizeof(*c)); @@ -23,6 +28,7 @@ void credential_clear(struct credential *c) free(c->path); free(c->username); free_password(c->password); + free(c->url); credential_init(c); } @@ -79,6 +85,29 @@ int credential_read(struct credential *c) * learn new lines, and the helpers are updated to match. */ } + + /* Rebuild URI from parts */ + *buf = '\0'; + if (c->protocol) { + strncat(buf, c->protocol, sizeof(buf)); + strncat(buf, "://", sizeof(buf)); + } + if (c->username) { + strncat(buf, c->username, sizeof(buf)); + strncat(buf, "@", sizeof(buf)); + } + if (c->host) + strncat(buf, c->host, sizeof(buf)); + if (c->port) { + value = buf + strlen(buf); + snprintf(value, sizeof(buf)-(value-buf), ":%hd", c->port); + } + if (c->path) { + strncat(buf, "/", sizeof(buf)); + strncat(buf, c->path, sizeof(buf)); + } + c->url = xstrdup(buf); + return 0; } @@ -126,6 +155,12 @@ int main(int argc, char *argv[]) goto out; } +#ifdef WIN32 + /* git on Windows uses binary pipes to avoid CRLF-issues */ + _setmode(_fileno(stdin), _O_BINARY); + _setmode(_fileno(stdout), _O_BINARY); +#endif + /* lookup operation callback */ while(try_op->name && strcmp(argv[1], try_op->name)) try_op++; @@ -138,11 +173,15 @@ int main(int argc, char *argv[]) if(ret) goto out; + if (!cred.protocol || !(cred.host || cred.path)) { + ret = EXIT_FAILURE; + goto out; + } + /* perform credential operation */ ret = (*try_op->op)(&cred); credential_write(&cred); - out: credential_clear(&cred); return ret; diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h index 76b6e50..b5ffa5b 100644 --- a/contrib/credential/helper/credential_helper.h +++ b/contrib/credential/helper/credential_helper.h @@ -25,10 +25,11 @@ struct credential char *path; char *username; char *password; + char *url; /* <protocol>://[username@][host[:port]][/path] */ }; #define CREDENTIAL_INIT \ - { NULL,NULL,0,NULL,NULL,NULL } + { NULL,NULL,0,NULL,NULL,NULL,NULL } void credential_init(struct credential *c); void credential_clear(struct credential *c); @@ -104,6 +105,17 @@ static inline void die_errno(int err) exit(EXIT_FAILURE); } +static inline void *xmalloc(size_t size) +{ + void *ret = malloc(size); + if (!ret && !size) + ret = malloc(1); + if (!ret) + die_errno(errno); + + return ret; +} + static inline char *xstrdup(const char *str) { char *ret = strdup(str); @@ -113,6 +125,7 @@ static inline char *xstrdup(const char *str) return ret; } +#ifndef NO_STRNDUP static inline char *xstrndup(const char *str, size_t len) { char *ret = strndup(str,len); @@ -121,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len) return ret; } +#endif #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */ diff --git a/contrib/credential/wincred/.gitignore b/contrib/credential/wincred/.gitignore new file mode 100644 index 0000000..4780e4e --- /dev/null +++ b/contrib/credential/wincred/.gitignore @@ -0,0 +1 @@ +git-credential-wincred.exe diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile index bad45ca..3900322 100644 --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,14 +1,26 @@ -all: git-credential-wincred.exe +MAIN:=git-credential-wincred +all:: $(MAIN).exe CC = gcc RM = rm -f CFLAGS = -O2 -Wall +CPPFLAGS = -DNO_STRNDUP -include ../../../config.mak.autogen -include ../../../config.mak -git-credential-wincred.exe : git-credential-wincred.c - $(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@ +HELPER:=../helper +VPATH +=$(HELPER) + +SRCS:=$(MAIN).c +SRCS+=credential_helper.c +OBJS:=$(SRCS:.c=.o) + +%.o: %.c + $(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) -o $@ -c $< + +$(MAIN).exe: $(OBJS) + $(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -o $@ clean: - $(RM) git-credential-wincred.exe + $(RM) $(MAIN).exe $(OBJS) diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c index cbaec5f..6dabeb7 100644 --- a/contrib/credential/wincred/git-credential-wincred.c +++ b/contrib/credential/wincred/git-credential-wincred.c @@ -6,37 +6,7 @@ #include <stdio.h> #include <io.h> #include <fcntl.h> - -/* common helpers */ - -static void die(const char *err, ...) -{ - char msg[4096]; - va_list params; - va_start(params, err); - vsnprintf(msg, sizeof(msg), err, params); - fprintf(stderr, "%s\n", msg); - va_end(params); - exit(1); -} - -static void *xmalloc(size_t size) -{ - void *ret = malloc(size); - if (!ret && !size) - ret = malloc(1); - if (!ret) - die("Out of memory"); - return ret; -} - -static char *xstrdup(const char *str) -{ - char *ret = strdup(str); - if (!ret) - die("Out of memory"); - return ret; -} +#include <credential_helper.h> /* MinGW doesn't have wincred.h, so we need to define stuff */ @@ -84,13 +54,16 @@ static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW; static CredFreeT CredFree; static CredDeleteWT CredDeleteW; -static void load_cred_funcs(void) + +static int load_cred_funcs(void) { /* load DLLs */ advapi = LoadLibrary("advapi32.dll"); credui = LoadLibrary("credui.dll"); - if (!advapi || !credui) - die("failed to load DLLs"); + if (!advapi || !credui) { + error("failed to load DLLs"); + return EXIT_FAILURE; + } /* get function pointers */ CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW"); @@ -104,28 +77,57 @@ static void load_cred_funcs(void) CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW"); if (!CredWriteW || !CredUnPackAuthenticationBufferW || !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree || - !CredDeleteW) - die("failed to load functions"); + !CredDeleteW) { + error("failed to load functions"); + return EXIT_FAILURE; + } + return EXIT_SUCCESS; } static char target_buf[1024]; -static char *protocol, *host, *path, *username; -static WCHAR *wusername, *password, *target; +static char port_buf[8]; +static WCHAR *wusername, *wpassword; -static void write_item(const char *what, WCHAR *wbuf) +static WCHAR *utf8_to_utf16_dup(const char *str) { - char *buf; - int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL, - FALSE); - buf = xmalloc(len); - - if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE)) - die("WideCharToMultiByte failed!"); - - printf("%s=", what); - fwrite(buf, 1, len - 1, stdout); - putchar('\n'); - free(buf); + int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); + WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen); + if (!MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen)) + die("MultiByteToWideChar failed"); + return wstr; +} + +static char *utf16_to_utf8_dup(const WCHAR *wstr) +{ + int len = WideCharToMultiByte(CP_UTF8, 0, wstr, -1, NULL, 0, NULL, + FALSE); + char *str = xmalloc(len); + if (!WideCharToMultiByte(CP_UTF8, 0, wstr, -1, str, len, NULL, FALSE)) + die("WideCharToMultiByte failed"); + return str; +} + +static void free_wpassword(WCHAR *pass) +{ + WCHAR *w = pass; + if(!pass) + return; + while(*w) *w++ = L'\0'; + free(pass); +} + +static int prepare_credential(struct credential *c) +{ + if (load_cred_funcs() ) + return EXIT_FAILURE; + + if (c->username) + wusername = utf8_to_utf16_dup(c->username); + if (c->password) + wpassword = utf8_to_utf16_dup(c->password); + if (c->port) + snprintf(port_buf, sizeof(port_buf), "%hd", c->port); + return EXIT_SUCCESS; } static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword, @@ -143,33 +145,37 @@ static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword, return 0; /* not found */ } -static int match_cred(const CREDENTIALW *cred) +static int match_cred(const CREDENTIALW *cred, const struct credential *c) { return (!wusername || !wcscmp(wusername, cred->UserName)) && - match_attr(cred, L"git_protocol", protocol) && - match_attr(cred, L"git_host", host) && - match_attr(cred, L"git_path", path); + match_attr(cred, L"git_protocol", c->protocol) && + match_attr(cred, L"git_host", c->host) && + (!c->port || match_attr(cred, L"git_port", port_buf)) && + match_attr(cred, L"git_path", c->path); } -static void get_credential(void) +static int get_credential(struct credential *c) { - WCHAR *user_buf, *pass_buf; + WCHAR *user_buf = NULL, *pass_buf = NULL; DWORD user_buf_size = 0, pass_buf_size = 0; CREDENTIALW **creds, *cred = NULL; DWORD num_creds; - int i; + int i, ret = EXIT_SUCCESS; + + if (prepare_credential(c)) + return EXIT_FAILURE; if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds)) - return; + return EXIT_FAILURE; /* search for the first credential that matches username */ for (i = 0; i < num_creds; ++i) - if (match_cred(creds[i])) { + if (match_cred(creds[i], c)) { cred = creds[i]; break; } if (!cred) - return; + goto out; CredUnPackAuthenticationBufferW(0, cred->CredentialBlob, cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL, @@ -180,20 +186,29 @@ static void get_credential(void) if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob, cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL, - pass_buf, &pass_buf_size)) - die("CredUnPackAuthenticationBuffer failed"); - - CredFree(creds); + pass_buf, &pass_buf_size)) { + error("CredUnPackAuthenticationBuffer failed"); + ret = EXIT_FAILURE; + goto out; + } /* zero-terminate (sizes include zero-termination) */ user_buf[user_buf_size - 1] = L'\0'; pass_buf[pass_buf_size - 1] = L'\0'; - write_item("username", user_buf); - write_item("password", pass_buf); + if (!c->username) + c->username = utf16_to_utf8_dup(user_buf); + free_password(c->password); + c->password = utf16_to_utf8_dup(pass_buf); + +out: + CredFree(creds); free(user_buf); - free(pass_buf); + free(wusername); + free_wpassword(pass_buf); + + return ret; } static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword, @@ -205,29 +220,42 @@ static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword, attr->Value = (LPBYTE)value; } -static void store_credential(void) +static int store_credential(struct credential *c) { CREDENTIALW cred; BYTE *auth_buf; DWORD auth_buf_size = 0; CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES]; + WCHAR *wtarget; + int ret = EXIT_SUCCESS; - if (!wusername || !password) - return; + if (prepare_credential(c)) + return EXIT_FAILURE; + + if (!wusername || !wpassword) + return EXIT_FAILURE; + + /* prepare 'target', the unique key for the credential */ + strncat(target_buf,"git:",sizeof(target_buf)); + strncat(target_buf,c->url,sizeof(target_buf)); + wtarget = utf8_to_utf16_dup(target_buf); /* query buffer size */ - CredPackAuthenticationBufferW(0, wusername, password, + CredPackAuthenticationBufferW(0, wusername, wpassword, NULL, &auth_buf_size); auth_buf = xmalloc(auth_buf_size); - if (!CredPackAuthenticationBufferW(0, wusername, password, - auth_buf, &auth_buf_size)) - die("CredPackAuthenticationBuffer failed"); + if (!CredPackAuthenticationBufferW(0, wusername, wpassword, + auth_buf, &auth_buf_size)) { + error("CredPackAuthenticationBuffer failed"); + ret = EXIT_FAILURE; + goto out; + } cred.Flags = 0; cred.Type = CRED_TYPE_GENERIC; - cred.TargetName = target; + cred.TargetName = wtarget; cred.Comment = L"saved by git-credential-wincred"; cred.CredentialBlobSize = auth_buf_size; cred.CredentialBlob = auth_buf; @@ -237,121 +265,65 @@ static void store_credential(void) cred.TargetAlias = NULL; cred.UserName = wusername; - write_attr(attrs, L"git_protocol", protocol); + write_attr(attrs, L"git_protocol", c->protocol); + + if (c->host) { + write_attr(attrs + cred.AttributeCount, L"git_host", c->host); + cred.AttributeCount++; + } - if (host) { - write_attr(attrs + cred.AttributeCount, L"git_host", host); + if (c->port) { + write_attr(attrs + cred.AttributeCount, L"git_port", port_buf); cred.AttributeCount++; } - if (path) { - write_attr(attrs + cred.AttributeCount, L"git_path", path); + if (c->path) { + write_attr(attrs + cred.AttributeCount, L"git_path", c->path); cred.AttributeCount++; } - if (!CredWriteW(&cred, 0)) - die("CredWrite failed"); + if (!CredWriteW(&cred, 0)) { + error("CredWrite failed"); + ret = EXIT_FAILURE; + } + +out: + free(auth_buf); + free(wusername); + free_wpassword(wpassword); + free(wtarget); + return ret; } -static void erase_credential(void) +static int erase_credential(struct credential *c) { CREDENTIALW **creds; DWORD num_creds; int i; + if (prepare_credential(c)) + return EXIT_FAILURE; + if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds)) - return; + return EXIT_FAILURE; for (i = 0; i < num_creds; ++i) { - if (match_cred(creds[i])) + if (match_cred(creds[i],c)) CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0); } CredFree(creds); + return EXIT_SUCCESS; } -static WCHAR *utf8_to_utf16_dup(const char *str) -{ - int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0); - WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen); - MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen); - return wstr; -} - -static void read_credential(void) -{ - char buf[1024]; - - while (fgets(buf, sizeof(buf), stdin)) { - char *v; - - if (!strcmp(buf, "\n")) - break; - buf[strlen(buf)-1] = '\0'; - - v = strchr(buf, '='); - if (!v) - die("bad input: %s", buf); - *v++ = '\0'; - - if (!strcmp(buf, "protocol")) - protocol = xstrdup(v); - else if (!strcmp(buf, "host")) - host = xstrdup(v); - else if (!strcmp(buf, "path")) - path = xstrdup(v); - else if (!strcmp(buf, "username")) { - username = xstrdup(v); - wusername = utf8_to_utf16_dup(v); - } else if (!strcmp(buf, "password")) - password = utf8_to_utf16_dup(v); - else - die("unrecognized input"); - } -} - -int main(int argc, char *argv[]) +/* + * Table with helper operation callbacks, used by generic + * credential helper main function. + */ +struct credential_operation const credential_helper_ops[] = { - const char *usage = - "Usage: git credential-wincred <get|store|erase>\n"; - - if (!argv[1]) - die(usage); - - /* git use binary pipes to avoid CRLF-issues */ - _setmode(_fileno(stdin), _O_BINARY); - _setmode(_fileno(stdout), _O_BINARY); - - read_credential(); - - load_cred_funcs(); - - if (!protocol || !(host || path)) - return 0; - - /* prepare 'target', the unique key for the credential */ - strncat(target_buf, "git:", sizeof(target_buf)); - strncat(target_buf, protocol, sizeof(target_buf)); - strncat(target_buf, "://", sizeof(target_buf)); - if (username) { - strncat(target_buf, username, sizeof(target_buf)); - strncat(target_buf, "@", sizeof(target_buf)); - } - if (host) - strncat(target_buf, host, sizeof(target_buf)); - if (path) { - strncat(target_buf, "/", sizeof(target_buf)); - strncat(target_buf, path, sizeof(target_buf)); - } - - target = utf8_to_utf16_dup(target_buf); - - if (!strcmp(argv[1], "get")) - get_credential(); - else if (!strcmp(argv[1], "store")) - store_credential(); - else if (!strcmp(argv[1], "erase")) - erase_credential(); - /* otherwise, ignore unknown action */ - return 0; -} + { "get", get_credential }, + { "store", store_credential }, + { "erase", erase_credential }, + CREDENTIAL_OP_END +}; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-09-01 12:43 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-08-23 16:57 [PATCH] contrib: GnomeKeyring support + generic helper implementation Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 1/4] contrib: add credential helper for GnomeKeyring Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 2/4] contrib: add generic credential helper Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 3/4] gnome-keyring: port to generic helper implementation Philipp A. Hartmann 2012-08-23 16:57 ` [PATCH 4/4] osxkeychain: port to generic credential " Philipp A. Hartmann 2012-08-24 18:15 ` [PATCH] contrib: GnomeKeyring support + generic " Junio C Hamano 2012-08-24 21:33 ` Jeff King 2012-08-24 21:46 ` Junio C Hamano 2012-08-26 17:46 ` Junio C Hamano 2012-08-26 18:16 ` Philipp A. Hartmann 2012-08-26 22:04 ` [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) Philipp A. Hartmann 2012-08-26 22:45 ` [PATCH 5/4 v2] " Philipp A. Hartmann 2012-08-30 18:27 ` [PATCH 5/4] " Erik Faye-Lund 2012-08-30 20:11 ` Junio C Hamano 2012-08-31 15:44 ` Erik Faye-Lund 2012-09-01 12:42 ` [PATCH 5/4 v3] wincred: port to generic credential helper Philipp A. Hartmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.