{"thread":{"id":"28387","subject":"[PATCH] contrib: add a credential helper for Mac OS X's keychain","startedAt":"2011-09-14T17:58:37Z","lastAt":"2011-09-15T00:16:05Z","messageCount":7,"participants":["Jay Soffian","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"175483","messageId":"1316023117-84755-1-git-send-email-jaysoffian@gmail.com","threadId":"28387","inReplyTo":null,"subject":"[PATCH] contrib: add a credential helper for Mac OS X's keychain","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-09-14T17:58:37Z","receivedAt":"2011-09-14T17:58:37Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"This credential helper adds, searches, and removes entries from\nthe Mac OS X keychain via OS X's Security Framework.\n\nTested with 10.6.8.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nAnd here's a C version. I cargo-culted the Makefile from\ncontrib/svn-fe/Makefile. Sadly, linking against git bloats the\nbinary quite a bit which is dissappointing since this helper\nwon't be installed as a hard-link. Hmm, maybe it can be if I\ndlopen the security framework instead of linking against it.\n\n contrib/credential-osxkeychain/Makefile            |   35 ++++\n .../git-credential-osxkeychain.c                   |  205 ++++++++++++++++++++\n 2 files changed, 240 insertions(+), 0 deletions(-)\n create mode 100644 contrib/credential-osxkeychain/Makefile\n create mode 100644 contrib/credential-osxkeychain/git-credential-osxkeychain.c\n\ndiff --git a/contrib/credential-osxkeychain/Makefile b/contrib/credential-osxkeychain/Makefile\nnew file mode 100644\nindex 0000000000..dc6bbbc3f9\n--- /dev/null\n+++ b/contrib/credential-osxkeychain/Makefile\n@@ -0,0 +1,35 @@\n+all:: git-credential-osxkeychain\n+\n+CC = gcc\n+RM = rm -f\n+CFLAGS = -O2 -Wall -I../.. -DSHA1_HEADER='<openssl/sha.h>'\n+GIT_LIBS = ../../libgit.a ../../xdiff/lib.a\n+LIBS = $(GIT_LIBS) -lz -liconv -lcrypto -lssl\n+\n+QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n+QUIET_SUBDIR1 =\n+\n+ifneq ($(findstring $(MAKEFLAGS),w),w)\n+PRINT_DIR = --no-print-directory\n+else # \"make -w\"\n+NO_SUBDIR = :\n+endif\n+\n+ifneq ($(findstring $(MAKEFLAGS),s),s)\n+ifndef V\n+\tQUIET_CC      = @echo '   ' CC $@;\n+\tQUIET_LINK    = @echo '   ' LINK $@;\n+\tQUIET_SUBDIR0 = +@subdir=\n+\tQUIET_SUBDIR1 = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \\\n+\t                $(MAKE) $(PRINT_DIR) -C $$subdir\n+endif\n+endif\n+\n+git-credential-osxkeychain: git-credential-osxkeychain.o $(GIT_LIBS)\n+\t$(QUIET_LINK)$(CC) -o $@ $< $(LIBS) -Wl,-framework -Wl,Security\n+\n+git-credential-osxkeychain.o: git-credential-osxkeychain.c\n+\t$(QUIET_CC)$(CC) -c $(CFLAGS) $<\n+\n+clean:\n+\t$(RM) git-credential-osxkeychain git-credential-osxkeychain.o\ndiff --git a/contrib/credential-osxkeychain/git-credential-osxkeychain.c b/contrib/credential-osxkeychain/git-credential-osxkeychain.c\nnew file mode 100644\nindex 0000000000..64bcc636cb\n--- /dev/null\n+++ b/contrib/credential-osxkeychain/git-credential-osxkeychain.c\n@@ -0,0 +1,205 @@\n+/* Copyright 2011 Jay Soffian. All rights reserved.\n+ * FreeBSD License.\n+ *\n+ * A git credential helper that interfaces with the Mac OS X keychain\n+ * via the Security framework.\n+ */\n+#include <stdlib.h>\n+#include <Security/Security.h>\n+#include \"cache.h\"\n+#include \"credential.h\"\n+#include \"parse-options.h\"\n+\n+void emit_user_pass(char *username, char *password)\n+{\n+\tif (username)\n+\t\tprintf(\"username=%s\\n\", username);\n+\tif (password)\n+\t\tprintf(\"password=%s\\n\", password);\n+}\n+\n+char *username_from_keychain_item(SecKeychainItemRef item)\n+{\n+\tOSStatus status;\n+\tSecKeychainAttributeList list;\n+\tSecKeychainAttribute attr;\n+\tlist.count = 1;\n+\tlist.attr = &attr;\n+\tattr.tag = kSecAccountItemAttr;\n+\tchar *username;\n+\n+\tstatus = SecKeychainItemCopyContent(item, NULL, &list, NULL, NULL);\n+\tif (status != noErr)\n+\t\treturn NULL;\n+\tusername = xmalloc(attr.length + 1);\n+\tstrncpy(username, attr.data, attr.length);\n+\tusername[attr.length] = '\\0';\n+\tSecKeychainItemFreeContent(&list, NULL);\n+\treturn username;\n+}\n+\n+int find_internet_password(SecProtocolType protocol,\n+\t\t\t   char *hostname,\n+\t\t\t   char *username)\n+{\n+\tvoid *password_buf;\n+\tUInt32 password_len;\n+\tOSStatus status;\n+\tchar *password;\n+\tint free_username;\n+\tSecKeychainItemRef item;\n+\n+\tstatus = SecKeychainFindInternetPassword(\n+\t\t\tNULL,\n+\t\t\tstrlen(hostname), hostname,\n+\t\t\t0, NULL,\n+\t\t\tusername ? strlen(username) : 0, username,\n+\t\t\t0, NULL,\n+\t\t\t0,\n+\t\t\tprotocol,\n+\t\t\tkSecAuthenticationTypeDefault,\n+\t\t\t&password_len, &password_buf,\n+\t\t\t&item);\n+\tif (status != noErr)\n+\t\treturn -1;\n+\n+\tpassword = xmalloc(password_len + 1);\n+\tstrncpy(password, password_buf, password_len);\n+\tpassword[password_len] = '\\0';\n+\tSecKeychainItemFreeContent(NULL, password_buf);\n+\tif (!username) {\n+\t\tusername = username_from_keychain_item(item);\n+\t\tfree_username = 1;\n+\t}\n+\temit_user_pass(username, password);\n+\tif (free_username)\n+\t\tfree(username);\n+\tfree(password);\n+\treturn 0;\n+}\n+\n+void delete_internet_password(SecProtocolType protocol,\n+\t\t\t      char *hostname,\n+\t\t\t      char *username)\n+{\n+\tOSStatus status;\n+\tSecKeychainItemRef item;\n+\n+\tstatus = SecKeychainFindInternetPassword(\n+\t\t\tNULL,\n+\t\t\tstrlen(hostname), hostname,\n+\t\t\t0, NULL,\n+\t\t\tusername ? strlen(username) : 0, username,\n+\t\t\t0, NULL,\n+\t\t\t0,\n+\t\t\tprotocol,\n+\t\t\tkSecAuthenticationTypeDefault,\n+\t\t\t0, NULL,\n+\t\t\t&item);\n+\tif (status != noErr)\n+\t\treturn;\n+\tSecKeychainItemDelete(item);\n+}\n+\n+void add_internet_password(SecProtocolType protocol,\n+\t\t\t   char *hostname,\n+\t\t\t   char *username,\n+\t\t\t   char *password,\n+\t\t\t   char *comment)\n+{\n+\tstruct strbuf label = STRBUF_INIT;\n+\tOSStatus status;\n+\tSecKeychainItemRef item;\n+\tSecKeychainAttributeList list;\n+\tSecKeychainAttribute attr;\n+\tlist.count = 1;\n+\tlist.attr = &attr;\n+\tstatus = SecKeychainAddInternetPassword(\n+\t\t\tNULL,\n+\t\t\tstrlen(hostname), hostname,\n+\t\t\t0, NULL,\n+\t\t\tstrlen(username), username,\n+\t\t\t0, NULL,\n+\t\t\t0,\n+\t\t\tprotocol,\n+\t\t\tkSecAuthenticationTypeDefault,\n+\t\t\tstrlen(password), password,\n+\t\t\t&item);\n+\tif (status != noErr)\n+\t\treturn;\n+\n+\t/* set the comment */\n+\tattr.tag = kSecCommentItemAttr;\n+\tattr.data = comment;\n+\tattr.length = strlen(comment);\n+\tSecKeychainItemModifyContent(item, &list, 0, NULL);\n+\n+\t/* override the label */\n+\tstrbuf_addf(&label, \"%s (%s)\", hostname, username);\n+\tattr.tag = kSecLabelItemAttr;\n+\tattr.data = label.buf;\n+\tattr.length = label.len;\n+\tSecKeychainItemModifyContent(item, &list, 0, NULL);\n+}\n+\n+int main(int argc, const char **argv)\n+{\n+\tconst char * const usage[] = {\n+\t\t\"git credential-osxkeychain [options]\",\n+\t\tNULL\n+\t};\n+\tstruct credential c = { NULL };\n+\tint reject = 0;\n+\tSecProtocolType protocol;\n+\tchar *hostname;\n+\tstruct option options[] = {\n+\t\tOPT_BOOLEAN(0, \"reject\", &reject,\n+\t\t\t    \"reject a stored credential\"),\n+\t\tOPT_STRING(0, \"username\", &c.username, \"name\",\n+\t\t\t   \"an existing username\"),\n+\t\tOPT_STRING(0, \"description\", &c.description, \"desc\",\n+\t\t\t   \"human-readable description of the credential\"),\n+\t\tOPT_STRING(0, \"unique\", &c.unique, \"token\",\n+\t\t\t   \"a unique context for the credential [REQUIRED]\"),\n+\t\tOPT_END()\n+\t};\n+\targc = parse_options(argc, argv, NULL, options, usage, 0);\n+\tif (argc)\n+\t\tusage_with_options(usage, options);\n+\n+\tif (!c.unique)\n+\t\tdie(_(\"--unique option required\"));\n+\thostname = strchr(c.unique, ':');\n+\tif (!hostname)\n+\t\tdie(_(\"Invalid token: '%s'\"), c.unique);\n+\t*hostname++ = '\\0';\n+\n+\t/* \"GitHub for Mac\" compatibility */\n+\tif (!strcmp(hostname, \"github.com\"))\n+\t\thostname = \"github.com/mac\";\n+\n+\tif (!strcmp(c.unique, \"https\")) {\n+\t\tprotocol = kSecProtocolTypeHTTPS;\n+\t} else if (!strcmp(c.unique, \"http\")) {\n+\t\tprotocol = kSecProtocolTypeHTTP;\n+\t}\n+\telse\n+\t\tdie(_(\"Unsupported protocol: '%s'\"), c.unique);\n+\n+\t/* if this is a rejection delete the existing creds */\n+\tif (reject) {\n+\t\tdelete_internet_password(protocol, hostname, c.username);\n+\t\treturn 0;\n+\t}\n+\n+\t/* otherwise look for a matching keychain item */\n+\tif (!find_internet_password(protocol, hostname, c.username))\n+\t\treturn 0;\n+\n+\t/* no keychain item found, prompt the user and store the result */\n+\tcredential_getpass(&c);\n+\tadd_internet_password(protocol, hostname, c.username, c.password,\n+\t\t\t      c.description ? c.description : \"default\");\n+\temit_user_pass(c.username, c.password);\n+\treturn 0;\n+}\n-- \n1.7.7.rc1.1.g011e1\n"},{"id":"175484","messageId":"CAG+J_DzfcNfp8r6g09DChUrUxgDbFC3O=zi0FSuD5eS+XwAjbw@mail.gmail.com","threadId":"28387","inReplyTo":"1316023117-84755-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] contrib: add a credential helper for Mac OS X's keychain","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-09-14T18:19:41Z","receivedAt":"2011-09-14T18:19:41Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Sep 14, 2011 at 1:58 PM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> diff --git a/contrib/credential-osxkeychain/git-credential-osxkeychain.c b/contrib/credential-osxkeychain/git-credential-osxkeychain.c\n> new file mode 100644\n> index 0000000000..64bcc636cb\n> --- /dev/null\n> +++ b/contrib/credential-osxkeychain/git-credential-osxkeychain.c\n>\n> [...]\n>\n> +int find_internet_password(SecProtocolType protocol,\n> +                          char *hostname,\n> +                          char *username)\n> +{\n> +       void *password_buf;\n> +       UInt32 password_len;\n> +       OSStatus status;\n> +       char *password;\n> +       int free_username;\n\nint free_username = 0;\n\nj.\n"},{"id":"175519","messageId":"1316040926-89429-1-git-send-email-jaysoffian@gmail.com","threadId":"28387","inReplyTo":"1316023117-84755-1-git-send-email-jaysoffian@gmail.com","subject":"[PATCH] credential-osxkeychain: load Security framework dynamically","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-09-14T22:55:26Z","receivedAt":"2011-09-14T22:55:26Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Use dlopen() / dysym() instead of dynmically linking to the\nSecurity framework. A followup commit will refactor things such\nthat git-credential-osxkeychain can be hardlinked to git.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\n> Hmm, maybe it can be if I dlopen the security framework instead of linking\n> against it.\n\nSomething like this. I'm going to pause here for feedback. Is the (not yet\nexistant) followup commit referenced above allowing git-credential-osxkeychain\nto be a hard link to git a worthwhile endeavor? Or would a better approach be\nto make git-credential-osxkeychain.c not use any git code?\n\n contrib/credential-osxkeychain/Makefile            |   12 +++-\n .../credential-osxkeychain/generate_security.py    |   73 ++++++++++++++++++++\n 2 files changed, 82 insertions(+), 3 deletions(-)\n create mode 100755 contrib/credential-osxkeychain/generate_security.py\n\ndiff --git a/contrib/credential-osxkeychain/Makefile b/contrib/credential-osxkeychain/Makefile\nindex dc6bbbc3f9..001d695cb8 100644\n--- a/contrib/credential-osxkeychain/Makefile\n+++ b/contrib/credential-osxkeychain/Makefile\n@@ -25,11 +25,17 @@ ifndef V\n endif\n endif\n \n-git-credential-osxkeychain: git-credential-osxkeychain.o $(GIT_LIBS)\n-\t$(QUIET_LINK)$(CC) -o $@ $< $(LIBS) -Wl,-framework -Wl,Security\n+git-credential-osxkeychain: git-credential-osxkeychain.o security.o $(GIT_LIBS)\n+\t$(QUIET_LINK)$(CC) -o $@ $< security.o $(LIBS)\n \n git-credential-osxkeychain.o: git-credential-osxkeychain.c\n \t$(QUIET_CC)$(CC) -c $(CFLAGS) $<\n \n+security.o: security.c\n+\t$(QUIET_CC)$(CC) -c $(CFLAGS) $<\n+\n+security.c: generate_security.py\n+\tpython generate_security.py\n+\n clean:\n-\t$(RM) git-credential-osxkeychain git-credential-osxkeychain.o\n+\t$(RM) git-credential-osxkeychain git-credential-osxkeychain.o security.?\ndiff --git a/contrib/credential-osxkeychain/generate_security.py b/contrib/credential-osxkeychain/generate_security.py\nnew file mode 100755\nindex 0000000000..db94672e95\n--- /dev/null\n+++ b/contrib/credential-osxkeychain/generate_security.py\n@@ -0,0 +1,73 @@\n+#!/usr/bin/python\n+\n+import re\n+\n+func_decls = \"\"\"\n+OSStatus SecKeychainAddInternetPassword(SecKeychainRef keychain, UInt32 serverNameLength, const char *serverName, UInt32 securityDomainLength, const char *securityDomain, UInt32 accountNameLength, const char *accountName, UInt32 pathLength, const char *path, UInt16 port, SecProtocolType protocol, SecAuthenticationType authenticationType, UInt32 passwordLength, const void *passwordData, SecKeychainItemRef *itemRef);\n+OSStatus SecKeychainFindInternetPassword(CFTypeRef keychainOrArray, UInt32 serverNameLength, const char *serverName, UInt32 securityDomainLength, const char *securityDomain, UInt32 accountNameLength, const char *accountName, UInt32 pathLength, const char *path, UInt16 port, SecProtocolType protocol, SecAuthenticationType authenticationType, UInt32 *passwordLength, void **passwordData, SecKeychainItemRef *itemRef);\n+OSStatus SecKeychainItemCopyContent(SecKeychainItemRef itemRef, SecItemClass *itemClass, SecKeychainAttributeList *attrList, UInt32 *length, void **outData);\n+OSStatus SecKeychainItemDelete(SecKeychainItemRef itemRef);\n+OSStatus SecKeychainItemFreeContent(SecKeychainAttributeList *attrList, void *data);\n+OSStatus SecKeychainItemModifyContent(SecKeychainItemRef itemRef, const SecKeychainAttributeList *attrList, UInt32 length, const void *data);\n+\"\"\"\n+\n+header = r\"\"\"\n+#include <dlfcn.h>\n+#include <Security/Security.h>\n+#include \"cache.h\"\n+\n+const char *security_framework =\n+\t\"/System/Library/Frameworks/Security.framework/Security\";\n+\n+void *load_security()\n+{\n+\tstatic void *security;\n+\tif (!security) {\n+\t\tif (!(security = dlopen(security_framework, RTLD_LAZY)))\n+\t\t\tdie(_(\"dlopen(\\\"%s\\\") failed: %s\"),\n+\t\t\t    security_framework, dlerror());\n+\t}\n+\treturn security;\n+}\n+\"\"\"\n+\n+func_tmpl = \"\"\"\n+%(func_decl)s\n+{\n+\t%(func_rv)s (*func)(%(arg_types)s) =\n+\t\tdlsym(load_security(), \"%(func_name)s\");\n+\tif (!func)\n+\t\tdie(_(\"dlsym(%(func_name)s) failed: %%s\"), dlerror());\n+\treturn func(%(arg_names)s);\n+}\n+\"\"\"\n+\n+def generate_func(decl):\n+\tfunc_rv, func_name, func_args = re.search(\n+\t\tr'^(.*?)\\s+([^(]+)\\((.*)\\);$', decl).groups()\n+\tfunc_args = [s.strip() for s in func_args.split(',')]\n+\targ_types = []\n+\targ_names = []\n+\tfor arg in func_args:\n+\t\targ_type, arg_name = re.search(r'^(.*?)([a-zA-Z]+)$', arg).groups()\n+\t\targ_types.append(arg_type.strip())\n+\t\targ_names.append(arg_name.strip())\n+\treturn func_tmpl % dict(\n+\t\tfunc_decl=decl.rstrip(';'),\n+\t\tfunc_name=func_name,\n+\t\tfunc_rv=func_rv,\n+\t\targ_types=', '.join(arg_types),\n+\t\targ_names=', '.join(arg_names),\n+\t)\n+\n+def main():\n+\tf = open('security.c', 'w')\n+\tf.write(header)\n+\tfor decl in func_decls.splitlines():\n+\t\tdecl = decl.strip()\n+\t\tif decl:\n+\t\t\tf.write(generate_func(decl))\n+\tf.close()\n+\n+if __name__ == '__main__':\n+\tmain()\n-- \n1.7.7.rc1.1.g011e1\n"},{"id":"175520","messageId":"20110914230816.GA5754@sigill.intra.peff.net","threadId":"28387","inReplyTo":"1316040926-89429-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] credential-osxkeychain: load Security framework dynamically","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-14T23:08:16Z","receivedAt":"2011-09-14T23:08:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 14, 2011 at 06:55:26PM -0400, Jay Soffian wrote:\n\n> Something like this. I'm going to pause here for feedback. Is the (not yet\n> existant) followup commit referenced above allowing git-credential-osxkeychain\n> to be a hard link to git a worthwhile endeavor? Or would a better approach be\n> to make git-credential-osxkeychain.c not use any git code?\n\nTo be honest, I was surprised to see you linking against git. I had\nalways envisioned OS-specific helpers as living outside of the git.git\nrepo.  That's why I provided git-credential-getpass; it should be the\nonly part of git that helpers really want to reuse.\n\nWhat are you getting from git that is useful? From my skim of your\npatch, it looks like xmalloc/die, parse_options, and credential_getpass.\n\nThe first can be replaced with a few trivial lines of code. The second\nis overkill, I think. The helper code will always hand you the\n\"--option=value\" form, and I always intended it to stay that way\n(whether that is well documented, I'm not sure). But a simple loop with\nstrncmps would be fine.\n\nThe hardest part is credential_getpass. You can call \"git\ncredential-getpass\", but you'll have to read the output yourself (though\nit's quite simple to parse; see read_credential_response).\n\nI'm not a fan of cutting and pasting code, and this will add a number of\nlines to your C program. But I feel like keeping the build completely\nseparate from core git is probably a good boundary (especially because\nthis will not be getting built or tested all the time, as most of the\ncore code is).\n\n-Peff\n"},{"id":"175523","messageId":"7vhb4esogg.fsf@alter.siamese.dyndns.org","threadId":"28387","inReplyTo":"1316040926-89429-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] credential-osxkeychain: load Security framework dynamically","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-14T23:18:07Z","receivedAt":"2011-09-14T23:18:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> Something like this. I'm going to pause here for feedback. Is the (not yet\n> existant) followup commit referenced above allowing git-credential-osxkeychain\n> to be a hard link to git a worthwhile endeavor? Or would a better approach be\n> to make git-credential-osxkeychain.c not use any git code?\n\nMost definitely the latter, I would think.\n\nThe whole point of making the Git credential code talk with a defined\ninterface with external programs is so that these keychain helpers can be\nwritten independently from the rest of Git.\n\nIf the reason why your keychain helper benefits from linking with the rest\nof Git is because some pieces of information you need in order to respond\nto the requests from credential interface is hard to get if your helper is\nbuilt as an independent program, that is a sign that we are not exposing\nenough information to scripts, iow, the failure in the design of the\ncredential interface. If that is the case (and I doubt it is), we would\nneed to fix the interface (either the credential interface, or perhaps\n\"git config\") so that such an independent program does not have to peek\ninside the internals of Git.\n\nIf the reason is because you want to reuse some generic C API we have that\nare not necessarily tied to Git (e.g. strbuf, string-list, etc.), on the\nother hand, please resist the temptation to do so. It would not help your\nprogram to serve as an example of independent external keychain helpers,\ni.e. a demonstration of how simple to write them.\n"},{"id":"175528","messageId":"CAG+J_Dxenu7GDsTzUCCQiYg38Vto8CtD8ODX1J9mA8o1n2_YJA@mail.gmail.com","threadId":"28387","inReplyTo":"20110914230816.GA5754@sigill.intra.peff.net","subject":"Re: [PATCH] credential-osxkeychain: load Security framework dynamically","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-09-14T23:56:42Z","receivedAt":"2011-09-14T23:56:42Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Sep 14, 2011 at 7:08 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, Sep 14, 2011 at 06:55:26PM -0400, Jay Soffian wrote:\n>\n> To be honest, I was surprised to see you linking against git. I had\n> always envisioned OS-specific helpers as living outside of the git.git\n> repo.  That's why I provided git-credential-getpass; it should be the\n> only part of git that helpers really want to reuse.\n\nOkay, I think I was led astray by the fact that\ncredential-{cache,store}.c (at least the latter of which is meant as\nnothing more than an example helper right?) links with git.\n\n> What are you getting from git that is useful? From my skim of your\n> patch, it looks like xmalloc/die, parse_options, and credential_getpass.\n\nCorrect.\n\n> The first can be replaced with a few trivial lines of code. The second\n> is overkill, I think. The helper code will always hand you the\n> \"--option=value\" form, and I always intended it to stay that way\n> (whether that is well documented, I'm not sure). But a simple loop with\n> strncmps would be fine.\n>\n> The hardest part is credential_getpass. You can call \"git\n> credential-getpass\", but you'll have to read the output yourself (though\n> it's quite simple to parse; see read_credential_response).\n>\n> I'm not a fan of cutting and pasting code, and this will add a number of\n> lines to your C program. But I feel like keeping the build completely\n> separate from core git is probably a good boundary (especially because\n> this will not be getting built or tested all the time, as most of the\n> core code is).\n\nOkay.\n\nj.\n"},{"id":"175530","messageId":"20110915001605.GB6238@sigill.intra.peff.net","threadId":"28387","inReplyTo":"CAG+J_Dxenu7GDsTzUCCQiYg38Vto8CtD8ODX1J9mA8o1n2_YJA@mail.gmail.com","subject":"Re: [PATCH] credential-osxkeychain: load Security framework dynamically","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-15T00:16:05Z","receivedAt":"2011-09-15T00:16:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 14, 2011 at 07:56:42PM -0400, Jay Soffian wrote:\n\n> Okay, I think I was led astray by the fact that\n> credential-{cache,store}.c (at least the latter of which is meant as\n> nothing more than an example helper right?) links with git.\n\nNo, credential-store is meant to be used. It's just that it has a\nsecurity tradeoff that makes it the wrong choice for most cases. So it's\nmeant to be used sparingly. :)\n\nAs for those helpers being linked against git, I guess it doesn't make\nthem the best example code. But I wanted them to be always available as\na lowest common denominator (because even if you have a fancy local\nkeychain, it is likely that you'll end up at some point using git across\nan ssh connection, and I wanted to provide _something_ there).\n\nNot having any external dependencies, those helpers don't pollute our\ncode base too much. Building and testing them with the rest of git keeps\nthe code fresh and unbroken. Maybe it would be better if they provided a\nclearer separation as an example. I'm open to that if people think it's\nworth splitting them out. I suspect I could write credential-store as\nsomething like 10 lines of perl.\n\n-Peff\n"}]}