From: Jeff King <hidden> Date: 2016-06-15 22:52:36
Here's the latest re-roll of the credential helpers series. I think this
one is probably ready to go to 'next'.
It's rebased on the latest tip of 'master' (applying it to an older
commit will get you a minor textual conflict in strbuf.c). It
incorporates the erase-safety we discussed, fixes a few commit message
typos, and tweaks the test scripts to make testing the external OS X
helper a little easier.
[01/13]: test-lib: add test_config_global variant
[02/13]: t5550: fix typo
[03/13]: introduce credentials API
[04/13]: credential: add function for parsing url components
[05/13]: http: use credential API to get passwords
[06/13]: credential: apply helper config
[07/13]: credential: add credential.*.username
[08/13]: credential: make relevance of http path configurable
[09/13]: docs: end-user documentation for the credential subsystem
[10/13]: credentials: add "cache" helper
[11/13]: strbuf: add strbuf_add*_urlencode
[12/13]: credentials: add "store" helper
[13/13]: t: add test harness for external credential helpers
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
The point of test_config is to simultaneously set a config
variable and register its cleanup handler, like:
test_config core.foo bar
However, it stupidly assumes that $1 contained the name of
the variable, which means it won't work for:
test_config --global core.foo bar
We could try to parse the command-line ourselves and figure
out which parts need to be fed to test_unconfig. But since
this is likely the most common variant, it's much simpler
and less error-prone to simply add a new function.
Signed-off-by: Jeff King <redacted>
---
t/test-lib.sh | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
@@ -379,6 +379,11 @@ test_config () {gitconfig"$@"}+test_config_global(){+test_when_finished"test_unconfig --global '$1'"&&+gitconfig--global"$@"+}+# Use test_set_prereq to tell that a particular prerequisite is available.# The prerequisite can later be checked for in two ways:#
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
This didn't have an impact, because it was just setting up
an "expect" file that happened to be identical to the one in
the test before it.
Signed-off-by: Jeff King <redacted>
---
t/t5550-http-fetch.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
@@ -66,7 +66,7 @@ test_expect_success 'cloning password-protected repository can fail' ' test_expect_success'http auth can use user/pass in URL''>askpass-query&&-echowrong>askpass-reponse&&+echowrong>askpass-response&&gitclone"$HTTPD_URL_USER_PASS/auth/repo.git"clone-auth-none&&test_cmpaskpass-expect-noneaskpass-query'
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
There are a few places in git that need to get a username
and password credential from the user; the most notable one
is HTTP authentication for smart-http pushing.
Right now the only choices for providing credentials are to
put them plaintext into your ~/.netrc, or to have git prompt
you (either on the terminal or via an askpass program). The
former is not very secure, and the latter is not very
convenient.
Unfortunately, there is no "always best" solution for
password management. The details will depend on the tradeoff
you want between security and convenience, as well as how
git can integrate with other security systems (e.g., many
operating systems provide a keychain or password wallet for
single sign-on).
This patch provides an abstract notion of credentials as a
data item, and provides three basic operations:
- fill (i.e., acquire from external storage or from the
user)
- approve (mark a credential as "working" for further
storage)
- reject (mark a credential as "not working", so it can
be removed from storage)
These operations can be backed by external helper processes
that interact with system- or user-specific secure storage.
Signed-off-by: Jeff King <redacted>
---
.gitignore | 1 +
Documentation/technical/api-credentials.txt | 241 +++++++++++++++++++++++++++
Makefile | 3 +
credential.c | 234 ++++++++++++++++++++++++++
credential.h | 28 +++
t/lib-credential.sh | 33 ++++
t/t0300-credentials.sh | 195 ++++++++++++++++++++++
test-credential.c | 38 +++++
8 files changed, 773 insertions(+), 0 deletions(-)
create mode 100644 Documentation/technical/api-credentials.txt
create mode 100644 credential.c
create mode 100644 credential.h
create mode 100755 t/lib-credential.sh
create mode 100755 t/t0300-credentials.sh
create mode 100644 test-credential.c
@@ -0,0 +1,241 @@+credentials API+===============++The credentials API provides an abstracted way of gathering username and+password credentials from the user (even though credentials in the wider+world can take many forms, in this document the word "credential" always+refers to a username and password pair).++Data Structures+---------------++`struct credential`::++ This struct represents a single username/password combination+ along with any associated context. All string fields should be+ heap-allocated (or NULL if they are not known or not applicable).+ The meaning of the individual context fields is the same as+ their counterparts in the helper protocol; see the section below+ for a description of each field.+++The `helpers` member of the struct is a `string_list` of helpers. Each+string specifies an external helper which will be run, in order, to+either acquire or store credentials. See the section on credential+helpers below.+++This struct should always be initialized with `CREDENTIAL_INIT` or+`credential_init`.+++Functions+---------++`credential_init`::++ Initialize a credential structure, setting all fields to empty.++`credential_clear`::++ Free any resources associated with the credential structure,+ returning it to a pristine initialized state.++`credential_fill`::++ Instruct the credential subsystem to fill the username and+ password fields of the passed credential struct by first+ consulting helpers, then asking the user. After this function+ returns, the username and password fields of the credential are+ guaranteed to be non-NULL. If an error occurs, the function will+ die().++`credential_reject`::++ Inform the credential subsystem that the provided credentials+ have been rejected. This will cause the credential subsystem to+ notify any helpers of the rejection (which allows them, for+ example, to purge the invalid credentials from storage). It+ will also free() the username and password fields of the+ credential and set them to NULL (readying the credential for+ another call to `credential_fill`). Any errors from helpers are+ ignored.++`credential_approve`::++ Inform the credential subsystem that the provided credentials+ were successfully used for authentication. This will cause the+ credential subsystem to notify any helpers of the approval, so+ that they may store the result to be used again. Any errors+ from helpers are ignored.++Example+-------++The example below shows how the functions of the credential API could be+used to login to a fictitious "foo" service on a remote host:++-----------------------------------------------------------------------+int foo_login(struct foo_connection *f)+{+ int status;+ /*+ * Create a credential with some context; we don't yet know the+ * username or password.+ */++ struct credential c = CREDENTIAL_INIT;+ c.protocol = xstrdup("foo");+ c.host = xstrdup(f->hostname);++ /*+ * Fill in the username and password fields by contacting+ * helpers and/or asking the user. The function will die if it+ * fails.+ */+ credential_fill(&c);++ /*+ * Otherwise, we have a username and password. Try to use it.+ */+ status = send_foo_login(f, c.username, c.password);+ switch (status) {+ case FOO_OK:+ /* It worked. Store the credential for later use. */+ credential_accept(&c);+ break;+ case FOO_BAD_LOGIN:+ /* Erase the credential from storage so we don't try it+ * again. */+ credential_reject(&c);+ break;+ default:+ /*+ * Some other error occured. We don't know if the+ * credential is good or bad, so report nothing to the+ * credential subsystem.+ */+ }++ /* Free any associated resources. */+ credential_clear(&c);++ return status;+}+-----------------------------------------------------------------------+++Credential Helpers+------------------++Credential helpers are programs executed by git to fetch or save+credentials from and to long-term storage (where "long-term" is simply+longer than a single git process; e.g., credentials may be stored+in-memory for a few minutes, or indefinitely on disk).++Each helper is specified by a single string. The string is transformed+by git into a command to be executed using these rules:++ 1. If the helper string begins with "!", it is considered a shell+ snippet, and everything after the "!" becomes the command.++ 2. Otherwise, if the helper string begins with an absolute path, the+ verbatim helper string becomes the command.++ 3. Otherwise, the string "git credential-" is prepended to the helper+ string, and the result becomes the command.++The resulting command then has an "operation" argument appended to it+(see below for details), and the result is executed by the shell.++Here are some example specifications:++----------------------------------------------------+# run "git credential-foo"+foo++# same as above, but pass an argument to the helper+foo --bar=baz++# the arguments are parsed by the shell, so use shell+# quoting if necessary+foo --bar="whitespace arg"++# you can also use an absolute path, which will not use the git wrapper+/path/to/my/helper --with-arguments++# or you can specify your own shell snippet+!f() { echo "password=`cat $HOME/.secret`"; }; f+----------------------------------------------------++Generally speaking, rule (3) above is the simplest for users to specify.+Authors of credential helpers should make an effort to assist their+users by naming their program "git-credential-$NAME", and putting it in+the $PATH or $GIT_EXEC_PATH during installation, which will allow a user+to enable it with `git config credential.helper $NAME`.++When a helper is executed, it will have one "operation" argument+appended to its command line, which is one of:++`get`::++ Return a matching credential, if any exists.++`store`::++ Store the credential, if applicable to the helper.++`erase`::++ Remove a matching credential, if any, from the helper's storage.++The details of the credential will be provided on the helper's stdin+stream. The credential is split into a set of named attributes.+Attributes are provided to the helper, one per line. Each attribute is+specified by a key-value pair, separated by an `=` (equals) sign,+followed by a newline. The key may contain any bytes except `=`,+newline, or NUL. The value may contain any bytes except newline or NUL.+In both cases, all bytes are treated as-is (i.e., there is no quoting,+and one cannot transmit a value with newline or NUL in it). The list of+attributes is terminated by a blank line or end-of-file.++Git will send the following attributes (but may not send all of+them for a given credential; for example, a `host` attribute makes no+sense when dealing with a non-network protocol):++`protocol`::++ The protocol over which the credential will be used (e.g.,+ `https`).++`host`::++ The remote hostname for a network credential.++`path`::++ The path with which the credential will be used. E.g., for+ accessing a remote https repository, this will be the+ repository's path on the server.++`username`::++ The credential's username, if we already have one (e.g., from a+ URL, from the user, or from a previously run helper).++`password`::++ The credential's password, if we are asking it to be stored.++For a `get` operation, the helper should produce a list of attributes+on stdout in the same format. A helper is free to produce a subset, or+even no values at all if it has nothing useful to provide. Any provided+attributes will overwrite those already known about by git.++For a `store` or `erase` operation, the helper's output is ignored.+If it fails to perform the requested operation, it may complain to+stderr to inform the user. If it does not support the requested+operation (e.g., a read-only store), it should silently ignore the+request.++If a helper receives any other operation, it should silently ignore the+request. This leaves room for future operations to be added (older+helpers will just ignore the new requests).
@@ -0,0 +1,234 @@+#include"cache.h"+#include"credential.h"+#include"string-list.h"+#include"run-command.h"++voidcredential_init(structcredential*c)+{+memset(c,0,sizeof(*c));+c->helpers.strdup_strings=1;+}++voidcredential_clear(structcredential*c)+{+free(c->protocol);+free(c->host);+free(c->path);+free(c->username);+free(c->password);+string_list_clear(&c->helpers,0);++credential_init(c);+}++staticvoidcredential_describe(structcredential*c,structstrbuf*out)+{+if(!c->protocol)+return;+strbuf_addf(out,"%s://",c->protocol);+if(c->username&&*c->username)+strbuf_addf(out,"%s@",c->username);+if(c->host)+strbuf_addstr(out,c->host);+if(c->path)+strbuf_addf(out,"/%s",c->path);+}++staticchar*credential_ask_one(constchar*what,structcredential*c)+{+structstrbufdesc=STRBUF_INIT;+structstrbufprompt=STRBUF_INIT;+char*r;++credential_describe(c,&desc);+if(desc.len)+strbuf_addf(&prompt,"%s for '%s': ",what,desc.buf);+else+strbuf_addf(&prompt,"%s: ",what);++/* FIXME: for usernames, we should do something less magical that+*actuallyechoesthecharacters.However,weneedtoreadfrom+*/dev/ttyandnotstdio,whichisnotportable(butgetpasswilldo+*itforus).http.cusesthesameworkaround.*/+r=git_getpass(prompt.buf);++strbuf_release(&desc);+strbuf_release(&prompt);+returnxstrdup(r);+}++staticvoidcredential_getpass(structcredential*c)+{+if(!c->username)+c->username=credential_ask_one("Username",c);+if(!c->password)+c->password=credential_ask_one("Password",c);+}++intcredential_read(structcredential*c,FILE*fp)+{+structstrbufline=STRBUF_INIT;++while(strbuf_getline(&line,fp,'\n')!=EOF){+char*key=line.buf;+char*value=strchr(key,'=');++if(!line.len)+break;++if(!value){+warning("invalid credential line: %s",key);+strbuf_release(&line);+return-1;+}+*value++='\0';++if(!strcmp(key,"username")){+free(c->username);+c->username=xstrdup(value);+}elseif(!strcmp(key,"password")){+free(c->password);+c->password=xstrdup(value);+}elseif(!strcmp(key,"protocol")){+free(c->protocol);+c->protocol=xstrdup(value);+}elseif(!strcmp(key,"host")){+free(c->host);+c->host=xstrdup(value);+}elseif(!strcmp(key,"path")){+free(c->path);+c->path=xstrdup(value);+}+/*+*Ignoreotherlines;wedon'tknowwhattheymean,but+*thisfuture-proofsuswhenlaterversionsofgitdo+*learnnewlines,andthehelpersareupdatedtomatch.+*/+}++strbuf_release(&line);+return0;+}++staticvoidcredential_write_item(FILE*fp,constchar*key,constchar*value)+{+if(!value)+return;+fprintf(fp,"%s=%s\n",key,value);+}++staticvoidcredential_write(conststructcredential*c,FILE*fp)+{+credential_write_item(fp,"protocol",c->protocol);+credential_write_item(fp,"host",c->host);+credential_write_item(fp,"path",c->path);+credential_write_item(fp,"username",c->username);+credential_write_item(fp,"password",c->password);+}++staticintrun_credential_helper(structcredential*c,+constchar*cmd,+intwant_output)+{+structchild_processhelper;+constchar*argv[]={NULL,NULL};+FILE*fp;++memset(&helper,0,sizeof(helper));+argv[0]=cmd;+helper.argv=argv;+helper.use_shell=1;+helper.in=-1;+if(want_output)+helper.out=-1;+else+helper.no_stdout=1;++if(start_command(&helper)<0)+return-1;++fp=xfdopen(helper.in,"w");+credential_write(c,fp);+fclose(fp);++if(want_output){+intr;+fp=xfdopen(helper.out,"r");+r=credential_read(c,fp);+fclose(fp);+if(r<0){+finish_command(&helper);+return-1;+}+}++if(finish_command(&helper))+return-1;+return0;+}++staticintcredential_do(structcredential*c,constchar*helper,+constchar*operation)+{+structstrbufcmd=STRBUF_INIT;+intr;++if(helper[0]=='!')+strbuf_addstr(&cmd,helper+1);+elseif(is_absolute_path(helper))+strbuf_addstr(&cmd,helper);+else+strbuf_addf(&cmd,"git credential-%s",helper);++strbuf_addf(&cmd," %s",operation);+r=run_credential_helper(c,cmd.buf,!strcmp(operation,"get"));++strbuf_release(&cmd);+returnr;+}++voidcredential_fill(structcredential*c)+{+inti;++if(c->username&&c->password)+return;++for(i=0;i<c->helpers.nr;i++){+credential_do(c,c->helpers.items[i].string,"get");+if(c->username&&c->password)+return;+}++credential_getpass(c);+if(!c->username&&!c->password)+die("unable to get password from user");+}++voidcredential_approve(structcredential*c)+{+inti;++if(c->approved)+return;+if(!c->username||!c->password)+return;++for(i=0;i<c->helpers.nr;i++)+credential_do(c,c->helpers.items[i].string,"store");+c->approved=1;+}++voidcredential_reject(structcredential*c)+{+inti;++for(i=0;i<c->helpers.nr;i++)+credential_do(c,c->helpers.items[i].string,"erase");++free(c->username);+c->username=NULL;+free(c->password);+c->password=NULL;+c->approved=0;+}
@@ -0,0 +1,33 @@+#!/bin/sh++# Try a set of credential helpers; the expected stdin,+# stdout and stderr should be provided on stdin,+# separated by "--".+check(){+read_chunk>stdin&&+read_chunk>expect-stdout&&+read_chunk>expect-stderr&&+test-credential"$@"<stdin>stdout2>stderr&&+test_cmpexpect-stdoutstdout&&+test_cmpexpect-stderrstderr+}++read_chunk(){+whilereadline;do+case"$line"in+--)break;;+*)echo"$line";;+esac+done+}+++cat>askpass<<\EOF+#!/bin/sh+echo>&2askpass:$*+what=`echo$1|cut-d" "-f1|trA-Za-z|tr-cda-z`+echo"askpass-$what"+EOF+chmod+xaskpass+GIT_ASKPASS="$PWD/askpass"+exportGIT_ASKPASS
@@ -0,0 +1,195 @@+#!/bin/sh++test_description='basic credential helper tests'+../test-lib.sh+."$TEST_DIRECTORY"/lib-credential.sh++test_expect_success'setup helper scripts''+cat>dump<<-\EOF&&+whoami=`echo$0|seds/.*git-credential-//`+echo>&2"$whoami: $*"+whileIFS==readkeyvalue;do+echo>&2"$whoami: $key=$value"+eval"$key=$value"+done+EOF++cat>git-credential-useless<<-\EOF&&+#!/bin/sh+../dump+exit0+EOF+chmod+xgit-credential-useless&&++cat>git-credential-verbatim<<-\EOF&&+#!/bin/sh+user=$1;shift+pass=$1;shift+../dump+test-z"$user"||echousername=$user+test-z"$pass"||echopassword=$pass+EOF+chmod+xgit-credential-verbatim&&++PATH="$PWD:$PATH"+'++test_expect_success'credential_fill invokes helper''+checkfill"verbatim foo bar"<<-\EOF+--+username=foo+password=bar+--+verbatim:get+EOF+'++test_expect_success'credential_fill invokes multiple helpers''+checkfilluseless"verbatim foo bar"<<-\EOF+--+username=foo+password=bar+--+useless:get+verbatim:get+EOF+'++test_expect_success'credential_fill stops when we get a full response''+checkfill"verbatim one two""verbatim three four"<<-\EOF+--+username=one+password=two+--+verbatim:get+EOF+'++test_expect_success'credential_fill continues through partial response''+checkfill"verbatim one \"\"""verbatim two three"<<-\EOF+--+username=two+password=three+--+verbatim:get+verbatim:get+verbatim:username=one+EOF+'++test_expect_success'credential_fill passes along metadata''+checkfill"verbatim one two"<<-\EOF+protocol=ftp+host=example.com+path=foo.git+--+username=one+password=two+--+verbatim:get+verbatim:protocol=ftp+verbatim:host=example.com+verbatim:path=foo.git+EOF+'++test_expect_success'credential_approve calls all helpers''+checkapproveuseless"verbatim one two"<<-\EOF+username=foo+password=bar+--+--+useless:store+useless:username=foo+useless:password=bar+verbatim:store+verbatim:username=foo+verbatim:password=bar+EOF+'++test_expect_success'do not bother storing password-less credential''+checkapproveuseless<<-\EOF+username=foo+--+--+EOF+'+++test_expect_success'credential_reject calls all helpers''+checkrejectuseless"verbatim one two"<<-\EOF+username=foo+password=bar+--+--+useless:erase+useless:username=foo+useless:password=bar+verbatim:erase+verbatim:username=foo+verbatim:password=bar+EOF+'++test_expect_success'usernames can be preserved''+checkfill"verbatim \"\" three"<<-\EOF+username=one+--+username=one+password=three+--+verbatim:get+verbatim:username=one+EOF+'++test_expect_success'usernames can be overridden''+checkfill"verbatim two three"<<-\EOF+username=one+--+username=two+password=three+--+verbatim:get+verbatim:username=one+EOF+'++test_expect_success'do not bother completing already-full credential''+checkfill"verbatim three four"<<-\EOF+username=one+password=two+--+username=one+password=two+--+EOF+'++# We can't test the basic terminal password prompt here because+# getpass() tries too hard to find the real terminal. But if our+# askpass helper is run, we know the internal getpass is working.+test_expect_success'empty helper list falls back to internal getpass''+checkfill<<-\EOF+--+username=askpass-username+password=askpass-password+--+askpass:Username:+askpass:Password:+EOF+'++test_expect_success'internal getpass does not ask for known username''+checkfill<<-\EOF+username=foo+--+username=foo+password=askpass-password+--+askpass:Password:+EOF+'++test_done
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
All of the components of a credential struct can be found in
a URL. For example, the URL:
http://foo:bar@example.com/repo.git
contains:
protocol=http
host=example.com
path=repo.git
username=foo
password=bar
We want to be able to turn URLs into broken-down credential
structs so that we know two things:
1. Which parts of the username/password we still need
2. What the context of the request is (for prompting or
as a key for storing credentials).
This code is based on http_auth_init in http.c, but needed a
few modifications in order to get all of the components that
the credential object is interested in.
Once the http code is switched over to the credential API,
then http_auth_init can just go away.
Signed-off-by: Jeff King <redacted>
---
Documentation/technical/api-credentials.txt | 4 ++
credential.c | 52 +++++++++++++++++++++++++++
credential.h | 1 +
3 files changed, 57 insertions(+), 0 deletions(-)
@@ -67,6 +67,10 @@ Functions that they may store the result to be used again. Any errors from helpers are ignored.+`credential_from_url`::++ Parse a URL into broken-down credential fields.+ Example -------
@@ -232,3 +233,54 @@ void credential_reject(struct credential *c)c->password=NULL;c->approved=0;}++voidcredential_from_url(structcredential*c,constchar*url)+{+constchar*at,*colon,*cp,*slash,*host,*proto_end;++credential_clear(c);++/*+*Matchoneof:+*(1)proto://<host>/...+*(2)proto://<user>@<host>/...+*(3)proto://<user>:<pass>@<host>/...+*/+proto_end=strstr(url,"://");+if(!proto_end)+return;+cp=proto_end+3;+at=strchr(cp,'@');+colon=strchr(cp,':');+slash=strchrnul(cp,'/');++if(!at||slash<=at){+/* Case (1) */+host=cp;+}+elseif(!colon||at<=colon){+/* Case (2) */+c->username=url_decode_mem(cp,at-cp);+host=at+1;+}else{+/* Case (3) */+c->username=url_decode_mem(cp,colon-cp);+c->password=url_decode_mem(colon+1,at-(colon+1));+host=at+1;+}++if(proto_end-url>0)+c->protocol=xmemdupz(url,proto_end-url);+if(slash-host>0)+c->host=url_decode_mem(host,slash-host);+/* Trim leading and trailing slashes from path */+while(*slash=='/')+slash++;+if(*slash){+char*p;+c->path=url_decode(slash);+p=c->path+strlen(c->path)-1;+while(p>c->path&&*p=='/')+*p--='\0';+}+}
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
The functionality for credential storage helpers is already
there; we just need to give the users a way to turn it on.
This patch provides a "credential.helper" configuration
variable which allows the user to provide one or more helper
strings.
Rather than simply matching credential.helper, we will also
compare URLs in subsection headings to the current context.
This means you can apply configuration to a subset of
credentials. For example:
[credential "https://example.com"]
helper = foo
would match a request for "https://example.com/foo.git", but
not one for "https://kernel.org/foo.git".
This is overkill for the "helper" variable, since users are
unlikely to want different helpers for different sites (and
since helpers run arbitrary code, they could do the matching
themselves anyway).
However, future patches will add new config variables where
this extra feature will be more useful.
Signed-off-by: Jeff King <redacted>
---
credential.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
credential.h | 5 +++-
t/t0300-credentials.sh | 42 +++++++++++++++++++++++++++++++++
t/t5550-http-fetch.sh | 12 +++++++++
4 files changed, 119 insertions(+), 1 deletions(-)
@@ -192,4 +192,46 @@ test_expect_success 'internal getpass does not ask for known username' 'EOF'+HELPER="!f() {+cat>/dev/null+echousername=foo+echopassword=bar+};f"+test_expect_success'respect configured credentials''+test_configcredential.helper"$HELPER"&&+checkfill<<-\EOF+--+username=foo+password=bar+--+EOF+'++test_expect_success'match configured credential''+test_configcredential.https://example.com.helper"$HELPER"&&+checkfill<<-\EOF+protocol=https+host=example.com+path=repo.git+--+username=foo+password=bar+--+EOF+'++test_expect_success'do not match configured credential''+test_configcredential.https://foo.helper"$HELPER"&&+checkfill<<-\EOF+protocol=https+host=bar+--+username=askpass-username+password=askpass-password+--+askpass:Usernamefor'\''https://bar'\'':+askpass:Passwordfor'\''https://askpass-username@bar'\'':+EOF+'+ test_done
@@ -101,6 +101,18 @@ test_expect_success 'http auth can request both user and pass' 'expect_askpassbothuser@host'+test_expect_success'http auth respects credential helper config''+test_config_globalcredential.helper"!f() {+cat>/dev/null+echousername=user@host+echopassword=user@host+};f" &&+>askpass-query&&+echowrong>askpass-response&&+gitclone"$HTTPD_URL/auth/repo.git"clone-auth-helper&&+expect_askpassnone+'+ test_expect_success'fetch changes via http''echocontent>>file&&gitcommit-a-mtwo&&
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
This patch converts the http code to use the new credential
API, both for http authentication as well as for getting
certificate passwords.
Most of the code change is simply variable naming (the
passwords are now contained inside the credential struct)
or deletion of obsolete code (the credential code handles
URL parsing and prompting for us).
The behavior should be the same, with one exception: the
credential code will prompt with a description based on the
credential components. Therefore, the old prompt of:
Username for 'example.com':
Password for 'example.com':
now looks like:
Username for 'https://example.com/repo.git':
Password for 'https://user@example.com/repo.git':
Note that we include more information in each line,
specifically:
1. We now include the protocol. While more noisy, this is
an important part of knowing what you are accessing
(especially if you care about http vs https).
2. We include the username in the password prompt. This is
not a big deal when you have just been prompted for it,
but the username may also come from the remote's URL
(and after future patches, from configuration or
credential helpers). In that case, it's a nice
reminder of the user for which you're giving the
password.
3. We include the path component of the URL. In many
cases, the user won't care about this and it's simply
noise (i.e., they'll use the same credential for a
whole site). However, that is part of a larger
question, which is whether path components should be
part of credential context, both for prompting and for
lookup by storage helpers. That issue will be addressed
as a whole in a future patch.
Similarly, for unlocking certificates, we used to say:
Certificate Password for 'example.com':
and we now say:
Password for 'cert:///path/to/certificate':
Showing the path to the client certificate makes more sense,
as that is what you are unlocking, not "example.com".
Signed-off-by: Jeff King <redacted>
---
http.c | 113 +++++++++++--------------------------------------
t/t5550-http-fetch.sh | 38 ++++++++++++-----
2 files changed, 52 insertions(+), 99 deletions(-)
@@ -241,18 +221,14 @@ static void init_curl_http_auth(CURL *result)staticinthas_cert_password(void){-if(ssl_cert_password!=NULL)-return1;if(ssl_cert==NULL||ssl_cert_password_required!=1)return0;-/* Only prompt the user once. */-ssl_cert_password_required=-1;-ssl_cert_password=git_getpass_with_description("Certificate Password",description);-if(ssl_cert_password!=NULL){-ssl_cert_password=xstrdup(ssl_cert_password);-return1;-}else-return0;+if(!cert_auth.password){+cert_auth.protocol=xstrdup("cert");+cert_auth.path=xstrdup(ssl_cert);+credential_fill(&cert_auth);+}+return1;}staticCURL*get_curl_handle(void)
@@ -279,7 +255,7 @@ static int has_cert_password(void)if(ssl_cert!=NULL)curl_easy_setopt(result,CURLOPT_SSLCERT,ssl_cert);if(has_cert_password())-curl_easy_setopt(result,CURLOPT_KEYPASSWD,ssl_cert_password);+curl_easy_setopt(result,CURLOPT_KEYPASSWD,cert_auth.password);#if LIBCURL_VERSION_NUM >= 0x070903if(ssl_key!=NULL)curl_easy_setopt(result,CURLOPT_SSLKEY,ssl_key);
@@ -321,42 +297,6 @@ static int has_cert_password(void)returnresult;}-staticvoidhttp_auth_init(constchar*url)-{-constchar*at,*colon,*cp,*slash,*host;--cp=strstr(url,"://");-if(!cp)-return;--/*-*Ok,theURLlookslike"proto://something".Whichone?-*"proto://<user>:<pass>@<host>/...",-*"proto://<user>@<host>/...",orjust-*"proto://<host>/..."?-*/-cp+=3;-at=strchr(cp,'@');-colon=strchr(cp,':');-slash=strchrnul(cp,'/');-if(!at||slash<=at){-/* No credentials, but we may have to ask for some later */-host=cp;-}-elseif(!colon||at<=colon){-/* Only username */-user_name=url_decode_mem(cp,at-cp);-user_pass=NULL;-host=at+1;-}else{-user_name=url_decode_mem(cp,colon-cp);-user_pass=url_decode_mem(colon+1,at-(colon+1));-host=at+1;-}--description=url_decode_mem(host,slash-host);-}-staticvoidset_from_env(constchar**var,constchar*envname){constchar*val=getenv(envname);
@@ -837,17 +777,11 @@ static int http_request(const char *url, void *result, int target, int options)elseif(missing_target(&results))ret=HTTP_MISSING_TARGET;elseif(results.http_code==401){-if(user_name&&user_pass){+if(http_auth.username&&http_auth.password){+credential_reject(&http_auth);ret=HTTP_NOAUTH;}else{-/*-*git_getpassisneededherebecauseitsverylikelystdin/stdoutare-*pipestoourparentprocess.Soweinsteadneedtouse/dev/tty,-*butthatisnon-portable.Usinggit_getpass()canatleastbestubbed-*onotherplatformswithadifferentimplementationif/whennecessary.-*/-if(!user_name)-user_name=xstrdup(git_getpass_with_description("Username",description));+credential_fill(&http_auth);init_curl_http_auth(slot->curl);ret=HTTP_REAUTH;}
@@ -866,6 +800,9 @@ static int http_request(const char *url, void *result, int target, int options)curl_slist_free_all(headers);strbuf_release(&buf);+if(ret==HTTP_OK)+credential_approve(&http_auth);+returnret;}
@@ -49,40 +49,56 @@ test_expect_success 'setup askpass helpers' 'EOFchmod+xaskpass&&GIT_ASKPASS="$PWD/askpass"&&-exportGIT_ASKPASS&&->askpass-expect-none&&-echo"askpass: Password for '\''$HTTPD_DEST'\'': ">askpass-expect-pass&&-{echo"askpass: Username for '\''$HTTPD_DEST'\'': "&&-cataskpass-expect-pass-}>askpass-expect-both-'+exportGIT_ASKPASS+'++expect_askpass(){+dest=$HTTPD_DEST/auth/repo.git+{+case"$1"in+none)+;;+pass)+echo"askpass: Password for 'http://$2@$dest': "+;;+both)+echo"askpass: Username for 'http://$dest': "+echo"askpass: Password for 'http://$2@$dest': "+;;+*)+false+;;+esac+}>askpass-expect&&+test_cmpaskpass-expectaskpass-query+} test_expect_success'cloning password-protected repository can fail''>askpass-query&&echowrong>askpass-response&&test_must_failgitclone"$HTTPD_URL/auth/repo.git"clone-auth-fail&&-test_cmpaskpass-expect-bothaskpass-query+expect_askpassbothwrong' test_expect_success'http auth can use user/pass in URL''>askpass-query&&echowrong>askpass-response&&gitclone"$HTTPD_URL_USER_PASS/auth/repo.git"clone-auth-none&&-test_cmpaskpass-expect-noneaskpass-query+expect_askpassnone' test_expect_success'http auth can use just user in URL''>askpass-query&&echouser@host>askpass-response&&gitclone"$HTTPD_URL_USER/auth/repo.git"clone-auth-pass&&-test_cmpaskpass-expect-passaskpass-query+expect_askpasspassuser@host' test_expect_success'http auth can request both user and pass''>askpass-query&&echouser@host>askpass-response&&gitclone"$HTTPD_URL/auth/repo.git"clone-auth-both&&-test_cmpaskpass-expect-bothaskpass-query+expect_askpassbothuser@host' test_expect_success'fetch changes via http''
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
Credential helpers can help users avoid having to type their
username and password over and over. However, some users may
not want a helper for their password, or they may be running
a helper which caches for a short time. In this case, it is
convenient to provide the non-secret username portion of
their credential via config.
Signed-off-by: Jeff King <redacted>
---
credential.c | 4 ++++
t/t0300-credentials.sh | 13 +++++++++++++
t/t5550-http-fetch.sh | 16 ++++++++++++++++
3 files changed, 33 insertions(+), 0 deletions(-)
@@ -234,4 +234,17 @@ test_expect_success 'do not match configured credential' 'EOF'+test_expect_success'pull username from config''+test_configcredential.https://example.com.usernamefoo&&+checkfill<<-\EOF+protocol=https+host=example.com+--+username=foo+password=askpass-password+--+askpass:Passwordfor'\''https://foo@example.com'\'':+EOF+'+ test_done
@@ -113,6 +113,22 @@ test_expect_success 'http auth respects credential helper config' 'expect_askpassnone'+test_expect_success'http auth can get username from config''+test_config_global"credential.$HTTPD_URL.username"user@host&&+>askpass-query&&+echouser@host>askpass-response&&+gitclone"$HTTPD_URL/auth/repo.git"clone-auth-user&&+expect_askpasspassuser@host+'++test_expect_success'configured username does not override URL''+test_config_global"credential.$HTTPD_URL.username"wrong&&+>askpass-query&&+echouser@host>askpass-response&&+gitclone"$HTTPD_URL_USER/auth/repo.git"clone-auth-user2&&+expect_askpasspassuser@host+'+ test_expect_success'fetch changes via http''echocontent>>file&&gitcommit-a-mtwo&&
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
When parsing a URL into a credential struct, we carefully
record each part of the URL, including the path on the
remote host, and use the result as part of the credential
context.
This had two practical implications:
1. Credential helpers which store a credential for later
access are likely to use the "path" portion as part of
the storage key. That means that a request to
https://example.com/foo.git
would not use the same credential that was stored in an
earlier request for:
https://example.com/bar.git
2. The prompt shown to the user includes all relevant
context, including the path.
In most cases, however, users will have a single password
per host. The behavior in (1) will be inconvenient, and the
prompt in (2) will be overly long.
This patch introduces a config option to toggle the
relevance of http paths. When turned on, we use the path as
before. When turned off, we drop the path component from the
context: helpers don't see it, and it does not appear in the
prompt.
This is nothing you couldn't do with a clever credential
helper at the start of your stack, like:
[credential "http://"]
helper = "!f() { grep -v ^path= ; }; f"
helper = your_real_helper
But doing this:
[credential]
useHttpPath = false
is way easier and more readable. Furthermore, since most
users will want the "off" behavior, that is the new default.
Users who want it "on" can set the variable (either for all
credentials, or just for a subset using
credential.*.useHttpPath).
Signed-off-by: Jeff King <redacted>
---
credential.c | 14 ++++++++++++++
credential.h | 3 ++-
t/t0300-credentials.sh | 29 +++++++++++++++++++++++++++++
t/t5550-http-fetch.sh | 2 +-
4 files changed, 46 insertions(+), 2 deletions(-)
@@ -247,4 +247,33 @@ test_expect_success 'pull username from config' 'EOF'+test_expect_success'http paths can be part of context''+checkfill"verbatim foo bar"<<-\EOF&&+protocol=https+host=example.com+path=foo.git+--+username=foo+password=bar+--+verbatim:get+verbatim:protocol=https+verbatim:host=example.com+EOF+test_configcredential.https://example.com.useHttpPathtrue&&+checkfill"verbatim foo bar"<<-\EOF+protocol=https+host=example.com+path=foo.git+--+username=foo+password=bar+--+verbatim:get+verbatim:protocol=https+verbatim:host=example.com+verbatim:path=foo.git+EOF+'+ test_done
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
The credential API and helper format is already defined in
technical/api-credentials.txt. This presents the end-user
view.
Signed-off-by: Jeff King <redacted>
---
Documentation/Makefile | 1 +
Documentation/config.txt | 23 +++++
Documentation/gitcredentials.txt | 171 ++++++++++++++++++++++++++++++++++++++
3 files changed, 195 insertions(+), 0 deletions(-)
create mode 100644 Documentation/gitcredentials.txt
@@ -834,6 +834,29 @@ commit.template:: "{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the specified user's home directory.+credential.helper::+ Specify an external helper to be called when a username or+ password credential is needed; the helper may consult external+ storage to avoid prompting the user for the credentials. See+ linkgit:gitcredentials[7] for details.++credential.useHttpPath::+ When acquiring credentials, consider the "path" component of an http+ or https URL to be important. Defaults to false. See+ linkgit:gitcredentials[7] for more information.++credential.username::+ If no username is set for a network authentication, use this username+ by default. See credential.<context>.* below, and+ linkgit:gitcredentials[7].++credential.<url>.*::+ Any of the credential.* options above can be applied selectively to+ some credentials. For example "credential.https://example.com.username"+ would set the default username only for https connections to+ example.com. See linkgit:gitcredentials[7] for details on how URLs are+ matched.+ include::diff-config.txt[] difftool.<tool>.path::
@@ -0,0 +1,171 @@+gitcredentials(7)+=================++NAME+----+gitcredentials - providing usernames and passwords to git++SYNOPSIS+--------+------------------+git config credential.https://example.com.username myusername+git config credential.helper "$helper $options"+------------------++DESCRIPTION+-----------++Git will sometimes need credentials from the user in order to perform+operations; for example, it may need to ask for a username and password+in order to access a remote repository over HTTP. This manual describes+the mechanisms git uses to request these credentials, as well as some+features to avoid inputting these credentials repeatedly.++REQUESTING CREDENTIALS+----------------------++Without any credential helpers defined, git will try the following+strategies to ask the user for usernames and passwords:++1. If the `GIT_ASKPASS` environment variable is set, the program+ specified by the variable is invoked. A suitable prompt is provided+ to the program on the command line, and the user's input is read+ from its standard output.++2. Otherwise, if the `core.askpass` configuration variable is set, its+ value is used as above.++3. Otherwise, if the `SSH_ASKPASS` environment variable is set, its+ value is used as above.++4. Otherwise, the user is prompted on the terminal.++AVOIDING REPETITION+-------------------++It can be cumbersome to input the same credentials over and over. Git+provides two methods to reduce this annoyance:++1. Static configuration of usernames for a given authentication context.++2. Credential helpers to cache or store passwords, or to interact with+ a system password wallet or keychain.++The first is simple and appropriate if you do not have secure storage available+for a password. It is generally configured by adding this to your config:++---------------------------------------+[credential "https://example.com"]+ username = me+---------------------------------------++Credential helpers, on the other hand, are external programs from which git can+request both usernames and passwords; they typically interface with secure+storage provided by the OS or other programs.++To use a helper, you must first select one to use. Git does not yet+include any credential helpers, but you may have third-party helpers+installed; search for `credential-*` in the output of `git help -a`, and+consult the documentation of individual helpers. Once you have selected+a helper, you can tell git to use it by putting its name into the+credential.helper variable.++1. Find a helper.+++-------------------------------------------+$ git help -a | grep credential-+credential-foo+-------------------------------------------++2. Read its description.+++-------------------------------------------+$ git help credential-foo+-------------------------------------------++3. Tell git to use it.+++-------------------------------------------+$ git config --global credential.helper foo+-------------------------------------------++If there are multiple instances of the `credential.helper` configuration+variable, each helper will be tried in turn, and may provide a username,+password, or nothing. Once git has acquired both a username and a+password, no more helpers will be tried.+++CREDENTIAL CONTEXTS+-------------------++Git considers each credential to have a context defined by a URL. This context+is used to look up context-specific configuration, and is passed to any+helpers, which may use it as an index into secure storage.++For instance, imagine we are accessing `https://example.com/foo.git`. When git+looks into a config file to see if a section matches this context, it will+consider the two a match if the context is a more-specific subset of the+pattern in the config file. For example, if you have this in your config file:++--------------------------------------+[credential "https://example.com"]+ username = foo+--------------------------------------++then we will match: both protocols are the same, both hosts are the same, and+the "pattern" URL does not care about the path component at all. However, this+context would not match:++--------------------------------------+[credential "https://kernel.org"]+ username = foo+--------------------------------------++because the hostnames differ. Nor would it match `foo.example.com`; git+compares hostnames exactly, without considering whether two hosts are part of+the same domain. Likewise, a config entry for `http://example.com` would not+match: git compares the protocols exactly.+++CONFIGURATION OPTIONS+---------------------++Options for a credential context can be configured either in+`credential.\*` (which applies to all credentials), or+`credential.<url>.\*`, where <url> matches the context as described+above.++The following options are available in either location:++helper::++ The name of an external credential helper, and any associated options.+ If the helper name is not an absolute path, then the string `git+ credential-` is prepended. The resulting string is executed by the+ shell (so, for example, setting this to `foo --option=bar` will execute+ `git credential-foo --option=bar` via the shell. See the manual of+ specific helpers for examples of their use.++username::++ A default username, if one is not provided in the URL.++useHttpPath::++ By default, git does not consider the "path" component of an http URL+ to be worth matching via external helpers. This means that a credential+ stored for `https://example.com/foo.git` will also be used for+ `https://example.com/bar.git`. If you do want to distinguish these+ cases, set this option to `true`.+++CUSTOM HELPERS+--------------++You can write your own custom helpers to interface with any system in+which you keep credentials. See the documentation for git's+link:technical/api-credentials.html[credentials API] for details.++GIT+---+Part of the linkgit:git[1] suite
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
If you access repositories over smart-http using http
authentication, then it can be annoying to have git ask you
for your password repeatedly. We cache credentials in
memory, of course, but git is composed of many small
programs. Having to input your password for each one can be
frustrating.
This patch introduces a credential helper that will cache
passwords in memory for a short period of time.
Signed-off-by: Jeff King <redacted>
---
Since the previous round, the tests have been tweaked to remove the use
of the "ftp" protocol. While this worked just fine for our cache and
store helpers, git doesn't actually generate protocol lines like that,
and some helpers may not understand it (e.g., the OS X helper has to
convert the protocol strings into numeric constants, and doesn't bother
converting things that git doesn't generate). And we use the same
battery of tests for the internal helpers as for external ones.
.gitignore | 2 +
Documentation/git-credential-cache--daemon.txt | 26 +++
Documentation/git-credential-cache.txt | 77 +++++++
Documentation/gitcredentials.txt | 17 +-
Makefile | 3 +
credential-cache--daemon.c | 269 ++++++++++++++++++++++++
credential-cache.c | 120 +++++++++++
git-compat-util.h | 1 +
t/lib-credential.sh | 221 +++++++++++++++++++
t/t0301-credential-cache.sh | 18 ++
unix-socket.c | 56 +++++
unix-socket.h | 7 +
12 files changed, 812 insertions(+), 5 deletions(-)
create mode 100644 Documentation/git-credential-cache--daemon.txt
create mode 100644 Documentation/git-credential-cache.txt
create mode 100644 credential-cache--daemon.c
create mode 100644 credential-cache.c
create mode 100755 t/t0301-credential-cache.sh
create mode 100644 unix-socket.c
create mode 100644 unix-socket.h
@@ -0,0 +1,26 @@+git-credential-cache--daemon(1)+===============================++NAME+----+git-credential-cache--daemon - temporarily store user credentials in memory++SYNOPSIS+--------+[verse]+git credential-cache--daemon <socket>++DESCRIPTION+-----------++NOTE: You probably don't want to invoke this command yourself; it is+started automatically when you use linkgit:git-credential-cache[1].++This command listens on the Unix domain socket specified by `<socket>`+for `git-credential-cache` clients. Clients may store and retrieve+credentials. Each credential is held for a timeout specified by the+client; once no credentials are held, the daemon exits.++GIT+---+Part of the linkgit:git[1] suite
@@ -0,0 +1,77 @@+git-credential-cache(1)+=======================++NAME+----+git-credential-cache - helper to temporarily store passwords in memory++SYNOPSIS+--------+-----------------------------+git config credential.helper 'cache [options]'+-----------------------------++DESCRIPTION+-----------++This command caches credentials in memory for use by future git+programs. The stored credentials never touch the disk, and are forgotten+after a configurable timeout. The cache is accessible over a Unix+domain socket, restricted to the current user by filesystem permissions.++You probably don't want to invoke this command directly; it is meant to+be used as a credential helper by other parts of git. See+linkgit:gitcredentials[7] or `EXAMPLES` below.++OPTIONS+-------++--timeout <seconds>::++ Number of seconds to cache credentials (default: 900).++--socket <path>::++ Use `<path>` to contact a running cache daemon (or start a new+ cache daemon if one is not started). Defaults to+ `~/.git-credential-cache/socket`. If your home directory is on a+ network-mounted filesystem, you may need to change this to a+ local filesystem.++CONTROLLING THE DAEMON+----------------------++If you would like the daemon to exit early, forgetting all cached+credentials before their timeout, you can issue an `exit` action:++--------------------------------------+git credential-cache exit+--------------------------------------++EXAMPLES+--------++The point of this helper is to reduce the number of times you must type+your username or password. For example:++------------------------------------+$ git config credential.helper cache+$ git push http://example.com/repo.git+Username: <type your username>+Password: <type your password>++[work for 5 more minutes]+$ git push http://example.com/repo.git+[your credentials are used automatically]+------------------------------------++You can provide options via the credential.helper configuration+variable (this example drops the cache time to 5 minutes):++-------------------------------------------------------+$ git config credential.helper 'cache --timeout=300'+-------------------------------------------------------++GIT+---+Part of the linkgit:git[1] suite
@@ -63,11 +63,18 @@ Credential helpers, on the other hand, are external programs from which git can request both usernames and passwords; they typically interface with secure storage provided by the OS or other programs.-To use a helper, you must first select one to use. Git does not yet-include any credential helpers, but you may have third-party helpers-installed; search for `credential-*` in the output of `git help -a`, and-consult the documentation of individual helpers. Once you have selected-a helper, you can tell git to use it by putting its name into the+To use a helper, you must first select one to use. Git currently+includes the following helpers:++cache::++ Cache credentials in memory for a short period of time. See+ linkgit:git-credential-cache[1] for details.++You may also have third-party helpers installed; search for+`credential-*` in the output of `git help -a`, and consult the+documentation of individual helpers. Once you have selected a helper,+you can tell git to use it by putting its name into the credential.helper variable. 1. Find a helper.
@@ -0,0 +1,269 @@+#include"cache.h"+#include"credential.h"+#include"unix-socket.h"+#include"sigchain.h"++staticconstchar*socket_path;++staticvoidcleanup_socket(void)+{+if(socket_path)+unlink(socket_path);+}++staticvoidcleanup_socket_on_signal(intsig)+{+cleanup_socket();+sigchain_pop(sig);+raise(sig);+}++structcredential_cache_entry{+structcredentialitem;+unsignedlongexpiration;+};+staticstructcredential_cache_entry*entries;+staticintentries_nr;+staticintentries_alloc;++staticvoidcache_credential(structcredential*c,inttimeout)+{+structcredential_cache_entry*e;++ALLOC_GROW(entries,entries_nr+1,entries_alloc);+e=&entries[entries_nr++];++/* take ownership of pointers */+memcpy(&e->item,c,sizeof(*c));+memset(c,0,sizeof(*c));+e->expiration=time(NULL)+timeout;+}++staticstructcredential_cache_entry*lookup_credential(conststructcredential*c)+{+inti;+for(i=0;i<entries_nr;i++){+structcredential*e=&entries[i].item;+if(credential_match(c,e))+return&entries[i];+}+returnNULL;+}++staticvoidremove_credential(conststructcredential*c)+{+structcredential_cache_entry*e;++e=lookup_credential(c);+if(e)+e->expiration=0;+}++staticintcheck_expirations(void)+{+staticunsignedlongwait_for_entry_until;+inti=0;+unsignedlongnow=time(NULL);+unsignedlongnext=(unsignedlong)-1;++/*+*Initiallygivetheclient30secondstoactuallycontactus+*andstoreacredentialbeforewedecidethere'snopointin+*keepingthedaemonaround.+*/+if(!wait_for_entry_until)+wait_for_entry_until=now+30;++while(i<entries_nr){+if(entries[i].expiration<=now){+entries_nr--;+credential_clear(&entries[i].item);+if(i!=entries_nr)+memcpy(&entries[i],&entries[entries_nr],sizeof(*entries));+/*+*Stickaround30secondsincaseanewcredential+*showsup(e.g.,becausewejustremovedafailed+*one,andwewillsoongetthecorrectone).+*/+wait_for_entry_until=now+30;+}+else{+if(entries[i].expiration<next)+next=entries[i].expiration;+i++;+}+}++if(!entries_nr){+if(wait_for_entry_until<=now)+return0;+next=wait_for_entry_until;+}++returnnext-now;+}++staticintread_request(FILE*fh,structcredential*c,+structstrbuf*action,int*timeout){+staticstructstrbufitem=STRBUF_INIT;+constchar*p;++strbuf_getline(&item,fh,'\n');+p=skip_prefix(item.buf,"action=");+if(!p)+returnerror("client sent bogus action line: %s",item.buf);+strbuf_addstr(action,p);++strbuf_getline(&item,fh,'\n');+p=skip_prefix(item.buf,"timeout=");+if(!p)+returnerror("client sent bogus timeout line: %s",item.buf);+*timeout=atoi(p);++if(credential_read(c,fh)<0)+return-1;+return0;+}++staticvoidserve_one_client(FILE*in,FILE*out)+{+structcredentialc=CREDENTIAL_INIT;+structstrbufaction=STRBUF_INIT;+inttimeout=-1;++if(read_request(in,&c,&action,&timeout)<0)+/* ignore error */;+elseif(!strcmp(action.buf,"get")){+structcredential_cache_entry*e=lookup_credential(&c);+if(e){+fprintf(out,"username=%s\n",e->item.username);+fprintf(out,"password=%s\n",e->item.password);+}+}+elseif(!strcmp(action.buf,"exit"))+exit(0);+elseif(!strcmp(action.buf,"erase"))+remove_credential(&c);+elseif(!strcmp(action.buf,"store")){+if(timeout<0)+warning("cache client didn't specify a timeout");+elseif(!c.username||!c.password)+warning("cache client gave us a partial credential");+else{+remove_credential(&c);+cache_credential(&c,timeout);+}+}+else+warning("cache client sent unknown action: %s",action.buf);++credential_clear(&c);+strbuf_release(&action);+}++staticintserve_cache_loop(intfd)+{+structpollfdpfd;+unsignedlongwakeup;++wakeup=check_expirations();+if(!wakeup)+return0;++pfd.fd=fd;+pfd.events=POLLIN;+if(poll(&pfd,1,1000*wakeup)<0){+if(errno!=EINTR)+die_errno("poll failed");+return1;+}++if(pfd.revents&POLLIN){+intclient,client2;+FILE*in,*out;++client=accept(fd,NULL,NULL);+if(client<0){+warning("accept failed: %s",strerror(errno));+return1;+}+client2=dup(client);+if(client2<0){+warning("dup failed: %s",strerror(errno));+close(client);+return1;+}++in=xfdopen(client,"r");+out=xfdopen(client2,"w");+serve_one_client(in,out);+fclose(in);+fclose(out);+}+return1;+}++staticvoidserve_cache(constchar*socket_path)+{+intfd;++fd=unix_stream_listen(socket_path);+if(fd<0)+die_errno("unable to bind to '%s'",socket_path);++printf("ok\n");+fclose(stdout);++while(serve_cache_loop(fd))+;/* nothing */++close(fd);+unlink(socket_path);+}++staticconstcharpermissions_advice[]=+"The permissions on your socket directory are too loose; other\n"+"users may be able to read your cached credentials. Consider running:\n"+"\n"+" chmod 0700 %s";+staticvoidcheck_socket_directory(constchar*path)+{+structstatst;+char*path_copy=xstrdup(path);+char*dir=dirname(path_copy);++if(!stat(dir,&st)){+if(st.st_mode&077)+die(permissions_advice,dir);+free(path_copy);+return;+}++/*+*Wemustbesuretocreatethedirectorywiththecorrectmode,+*notjustchmoditafterthefact;otherwise,thereisarace+*conditioninwhichsomebodycanchdirtoit,sleep,thentrytoopen+*ourprotectedsocket.+*/+if(safe_create_leading_directories_const(dir)<0)+die_errno("unable to create directories for '%s'",dir);+if(mkdir(dir,0700)<0)+die_errno("unable to mkdir '%s'",dir);+free(path_copy);+}++intmain(intargc,constchar**argv)+{+socket_path=argv[1];++if(!socket_path)+die("usage: git-credential-cache--daemon <socket_path>");+check_socket_directory(socket_path);++atexit(cleanup_socket);+sigchain_push_common(cleanup_socket_on_signal);++serve_cache(socket_path);++return0;+}
@@ -0,0 +1,120 @@+#include"cache.h"+#include"credential.h"+#include"string-list.h"+#include"parse-options.h"+#include"unix-socket.h"+#include"run-command.h"++#define FLAG_SPAWN 0x1+#define FLAG_RELAY 0x2++staticintsend_request(constchar*socket,conststructstrbuf*out)+{+intgot_data=0;+intfd=unix_stream_connect(socket);++if(fd<0)+return-1;++if(write_in_full(fd,out->buf,out->len)<0)+die_errno("unable to write to cache daemon");+shutdown(fd,SHUT_WR);++while(1){+charin[1024];+intr;++r=read_in_full(fd,in,sizeof(in));+if(r==0)+break;+if(r<0)+die_errno("read error from cache daemon");+write_or_die(1,in,r);+got_data=1;+}+returngot_data;+}++staticvoidspawn_daemon(constchar*socket)+{+structchild_processdaemon;+constchar*argv[]={NULL,NULL,NULL};+charbuf[128];+intr;++memset(&daemon,0,sizeof(daemon));+argv[0]="git-credential-cache--daemon";+argv[1]=socket;+daemon.argv=argv;+daemon.no_stdin=1;+daemon.out=-1;++if(start_command(&daemon))+die_errno("unable to start cache daemon");+r=read_in_full(daemon.out,buf,sizeof(buf));+if(r<0)+die_errno("unable to read result code from cache daemon");+if(r!=3||memcmp(buf,"ok\n",3))+die("cache daemon did not start: %.*s",r,buf);+close(daemon.out);+}++staticvoiddo_cache(constchar*socket,constchar*action,inttimeout,+intflags)+{+structstrbufbuf=STRBUF_INIT;++strbuf_addf(&buf,"action=%s\n",action);+strbuf_addf(&buf,"timeout=%d\n",timeout);+if(flags&FLAG_RELAY){+if(strbuf_read(&buf,0,0)<0)+die_errno("unable to relay credential");+}++if(!send_request(socket,&buf))+return;+if(flags&FLAG_SPAWN){+spawn_daemon(socket);+send_request(socket,&buf);+}+strbuf_release(&buf);+}++intmain(intargc,constchar**argv)+{+char*socket_path=NULL;+inttimeout=900;+constchar*op;+constchar*constusage[]={+"git credential-cache [options] <action>",+NULL+};+structoptionoptions[]={+OPT_INTEGER(0,"timeout",&timeout,+"number of seconds to cache credentials"),+OPT_STRING(0,"socket",&socket_path,"path",+"path of cache-daemon socket"),+OPT_END()+};++argc=parse_options(argc,argv,NULL,options,usage,0);+if(!argc)+usage_with_options(usage,options);+op=argv[0];++if(!socket_path)+socket_path=expand_user_path("~/.git-credential-cache/socket");+if(!socket_path)+die("unable to find a suitable socket path; use --socket");++if(!strcmp(op,"exit"))+do_cache(socket_path,op,timeout,0);+elseif(!strcmp(op,"get")||!strcmp(op,"erase"))+do_cache(socket_path,op,timeout,FLAG_RELAY);+elseif(!strcmp(op,"store"))+do_cache(socket_path,op,timeout,FLAG_RELAY|FLAG_SPAWN);+else+;/* ignore unknown operation */++return0;+}
@@ -21,6 +21,227 @@ read_chunk() {done}+# Clear any residual data from previous tests. We only+# need this when testing third-party helpers which read and+# write outside of our trash-directory sandbox.+#+# Don't bother checking for success here, as it is+# outside the scope of tests and represents a best effort to+# clean up after ourselves.+helper_test_clean(){+reject$1httpsexample.comstore-user+reject$1httpsexample.comuser1+reject$1httpsexample.comuser2+reject$1httppath.tlduser+reject$1httpstimeout.tlduser+}++reject(){+(+echoprotocol=$2+echohost=$3+echousername=$4+)|test-credentialreject$1+}++helper_test(){+HELPER=$1++test_expect_success"helper ($HELPER) has no existing data"'+checkfill$HELPER<<-\EOF+protocol=https+host=example.com+--+username=askpass-username+password=askpass-password+--+askpass:Usernamefor'\''https://example.com'\'':+askpass:Passwordfor'\''https://askpass-username@example.com'\'':+EOF+'++test_expect_success"helper ($HELPER) stores password"'+checkapprove$HELPER<<-\EOF+protocol=https+host=example.com+username=store-user+password=store-pass+EOF+'++test_expect_success"helper ($HELPER) can retrieve password"'+checkfill$HELPER<<-\EOF+protocol=https+host=example.com+--+username=store-user+password=store-pass+--+EOF+'++test_expect_success"helper ($HELPER) requires matching protocol"'+checkfill$HELPER<<-\EOF+protocol=http+host=example.com+--+username=askpass-username+password=askpass-password+--+askpass:Usernamefor'\''http://example.com'\'':+askpass:Passwordfor'\''http://askpass-username@example.com'\'':+EOF+'++test_expect_success"helper ($HELPER) requires matching host"'+checkfill$HELPER<<-\EOF+protocol=https+host=other.tld+--+username=askpass-username+password=askpass-password+--+askpass:Usernamefor'\''https://other.tld'\'':+askpass:Passwordfor'\''https://askpass-username@other.tld'\'':+EOF+'++test_expect_success"helper ($HELPER) requires matching username"'+checkfill$HELPER<<-\EOF+protocol=https+host=example.com+username=other+--+username=other+password=askpass-password+--+askpass:Passwordfor'\''https://other@example.com'\'':+EOF+'++test_expect_success"helper ($HELPER) requires matching path"'+test_configcredential.usehttppathtrue&&+checkapprove$HELPER<<-\EOF&&+protocol=http+host=path.tld+path=foo.git+username=user+password=pass+EOF+checkfill$HELPER<<-\EOF+protocol=http+host=path.tld+path=bar.git+--+username=askpass-username+password=askpass-password+--+askpass:Usernamefor'\''http://path.tld/bar.git'\'':+askpass:Passwordfor'\''http://askpass-username@path.tld/bar.git'\'':+EOF+'++test_expect_success"helper ($HELPER) can forget host"'+checkreject$HELPER<<-\EOF&&+protocol=https+host=example.com+EOF+checkfill$HELPER<<-\EOF+protocol=https+host=example.com+--+username=askpass-username+password=askpass-password+--+askpass:Usernamefor'\''https://example.com'\'':+askpass:Passwordfor'\''https://askpass-username@example.com'\'':+EOF+'++test_expect_success"helper ($HELPER) can store multiple users"'+checkapprove$HELPER<<-\EOF&&+protocol=https+host=example.com+username=user1+password=pass1+EOF+checkapprove$HELPER<<-\EOF&&+protocol=https+host=example.com+username=user2+password=pass2+EOF+checkfill$HELPER<<-\EOF&&+protocol=https+host=example.com+username=user1+--+username=user1+password=pass1+EOF+checkfill$HELPER<<-\EOF+protocol=https+host=example.com+username=user2+--+username=user2+password=pass2+EOF+'++test_expect_success"helper ($HELPER) can forget user"'+checkreject$HELPER<<-\EOF&&+protocol=https+host=example.com+username=user1+EOF+checkfill$HELPER<<-\EOF+protocol=https+host=example.com+username=user1+--+username=user1+password=askpass-password+--+askpass:Passwordfor'\''https://user1@example.com'\'':+EOF+'++test_expect_success"helper ($HELPER) remembers other user"'+checkfill$HELPER<<-\EOF+protocol=https+host=example.com+username=user2+--+username=user2+password=pass2+EOF+'+}++helper_test_timeout(){+HELPER="$*"++test_expect_success"helper ($HELPER) times out"'+checkapprove"$HELPER"<<-\EOF&&+protocol=https+host=timeout.tld+username=user+password=pass+EOF+sleep2&&+checkfill"$HELPER"<<-\EOF+protocol=https+host=timeout.tld+--+username=askpass-username+password=askpass-password+--+askpass:Usernamefor'\''https://timeout.tld'\'':+askpass:Passwordfor'\''https://askpass-username@timeout.tld'\'':+EOF+'+} cat>askpass<<\EOF#!/bin/sh
@@ -0,0 +1,18 @@+#!/bin/sh++test_description='credential-cache tests'+../test-lib.sh+."$TEST_DIRECTORY"/lib-credential.sh++# don't leave a stale daemon running+trap'code=$?; git credential-cache exit; (exit $code); die'EXIT++helper_testcache+helper_test_timeoutcache--timeout=1++# we can't rely on our "trap" above working after test_done,+# as test_done will delete the trash directory containing+# our socket, leaving us with no way to access the daemon.+gitcredential-cacheexit++test_done
@@ -0,0 +1,56 @@+#include"cache.h"+#include"unix-socket.h"++staticintunix_stream_socket(void)+{+intfd=socket(AF_UNIX,SOCK_STREAM,0);+if(fd<0)+die_errno("unable to create socket");+returnfd;+}++staticvoidunix_sockaddr_init(structsockaddr_un*sa,constchar*path)+{+intsize=strlen(path)+1;+if(size>sizeof(sa->sun_path))+die("socket path is too long to fit in sockaddr");+memset(sa,0,sizeof(*sa));+sa->sun_family=AF_UNIX;+memcpy(sa->sun_path,path,size);+}++intunix_stream_connect(constchar*path)+{+intfd;+structsockaddr_unsa;++unix_sockaddr_init(&sa,path);+fd=unix_stream_socket();+if(connect(fd,(structsockaddr*)&sa,sizeof(sa))<0){+close(fd);+return-1;+}+returnfd;+}++intunix_stream_listen(constchar*path)+{+intfd;+structsockaddr_unsa;++unix_sockaddr_init(&sa,path);+fd=unix_stream_socket();++unlink(path);+if(bind(fd,(structsockaddr*)&sa,sizeof(sa))<0){+close(fd);+return-1;+}++if(listen(fd,5)<0){+close(fd);+return-1;+}++returnfd;+}
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
This just follows the rfc3986 rules for percent-encoding
url data into a strbuf.
Signed-off-by: Jeff King <redacted>
---
strbuf.c | 37 +++++++++++++++++++++++++++++++++++++
strbuf.h | 5 +++++
2 files changed, 42 insertions(+), 0 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
This is like "cache", except that we actually put the
credentials on disk. This can be terribly insecure, of
course, but we do what we can to protect them by filesystem
permissions, and we warn the user in the documentation.
This is not unlike using .netrc to store entries, but it's a
little more user-friendly. Instead of putting credentials in
place ahead of time, we transparently store them after
prompting the user for them once.
Signed-off-by: Jeff King <redacted>
---
This version has the "don't erase everything" protection we discussed.
.gitignore | 1 +
Documentation/git-credential-store.txt | 75 +++++++++++++++
Documentation/gitcredentials.txt | 5 +
Makefile | 1 +
credential-store.c | 157 ++++++++++++++++++++++++++++++++
t/t0302-credential-store.sh | 9 ++
6 files changed, 248 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-credential-store.txt
create mode 100644 credential-store.c
create mode 100755 t/t0302-credential-store.sh
@@ -0,0 +1,75 @@+git-credential-store(1)+=======================++NAME+----+git-credential-store - helper to store credentials on disk++SYNOPSIS+--------+-------------------+git config credential.helper 'store [options]'+-------------------++DESCRIPTION+-----------++NOTE: Using this helper will store your passwords unencrypted on disk,+protected only by filesystem permissions. If this is not an acceptable+security tradeoff, try linkgit:git-credential-cache[1], or find a helper+that integrates with secure storage provided by your operating system.++This command stores credentials indefinitely on disk for use by future+git programs.++You probably don't want to invoke this command directly; it is meant to+be used as a credential helper by other parts of git. See+linkgit:gitcredentials[7] or `EXAMPLES` below.++OPTIONS+-------++--store=<path>::++ Use `<path>` to store credentials. The file will have its+ filesystem permissions set to prevent other users on the system+ from reading it, but will not be encrypted or otherwise+ protected. Defaults to `~/.git-credentials`.++EXAMPLES+--------++The point of this helper is to reduce the number of times you must type+your username or password. For example:++------------------------------------------+$ git config credential.helper store+$ git push http://example.com/repo.git+Username: <type your username>+Password: <type your password>++[several days later]+$ git push http://example.com/repo.git+[your credentials are used automatically]+------------------------------------------++STORAGE FORMAT+--------------++The `.git-credentials` file is stored in plaintext. Each credential is+stored on its own line as a URL like:++------------------------------+https://user:pass@example.com+------------------------------++When git needs authentication for a particular URL context,+credential-store will consider that context a pattern to match against+each entry in the credentials file. If the protocol, hostname, and+username (if we already have one) match, then the password is returned+to git. See the discussion of configuration in linkgit:gitcredentials[7]+for more information.++GIT+---+Part of the linkgit:git[1] suite
@@ -71,6 +71,11 @@ cache:: Cache credentials in memory for a short period of time. See linkgit:git-credential-cache[1] for details.+store::++ Store credentials indefinitely on disk. See+ linkgit:git-credential-store[1] for details.+ You may also have third-party helpers installed; search for `credential-*` in the output of `git help -a`, and consult the documentation of individual helpers. Once you have selected a helper,
@@ -0,0 +1,157 @@+#include"cache.h"+#include"credential.h"+#include"string-list.h"+#include"parse-options.h"++staticstructlock_filecredential_lock;++staticvoidparse_credential_file(constchar*fn,+structcredential*c,+void(*match_cb)(structcredential*),+void(*other_cb)(structstrbuf*))+{+FILE*fh;+structstrbufline=STRBUF_INIT;+structcredentialentry=CREDENTIAL_INIT;++fh=fopen(fn,"r");+if(!fh){+if(errno!=ENOENT)+die_errno("unable to open %s",fn);+return;+}++while(strbuf_getline(&line,fh,'\n')!=EOF){+credential_from_url(&entry,line.buf);+if(entry.username&&entry.password&&+credential_match(c,&entry)){+if(match_cb){+match_cb(&entry);+break;+}+}+elseif(other_cb)+other_cb(&line);+}++credential_clear(&entry);+strbuf_release(&line);+fclose(fh);+}++staticvoidprint_entry(structcredential*c)+{+printf("username=%s\n",c->username);+printf("password=%s\n",c->password);+}++staticvoidprint_line(structstrbuf*buf)+{+strbuf_addch(buf,'\n');+write_or_die(credential_lock.fd,buf->buf,buf->len);+}++staticvoidrewrite_credential_file(constchar*fn,structcredential*c,+structstrbuf*extra)+{+if(hold_lock_file_for_update(&credential_lock,fn,0)<0)+die_errno("unable to get credential storage lock");+if(extra)+print_line(extra);+parse_credential_file(fn,c,NULL,print_line);+if(commit_lock_file(&credential_lock)<0)+die_errno("unable to commit credential store");+}++staticvoidstore_credential(constchar*fn,structcredential*c)+{+structstrbufbuf=STRBUF_INIT;++/*+*Sanitycheckthatwhatwearestoringisactuallysensible.+*Inparticular,wecan'tmakeaURLwithoutaprotocolfield.+*Withouteitherahostorpathname(dependingonthescheme),+*wehavenoprimarykey.Andwithoutausernameandpassword,+*wearenotactuallystoringacredential.+*/+if(!c->protocol||!(c->host||c->path)||+!c->username||!c->password)+return;++strbuf_addf(&buf,"%s://",c->protocol);+strbuf_addstr_urlencode(&buf,c->username,1);+strbuf_addch(&buf,':');+strbuf_addstr_urlencode(&buf,c->password,1);+strbuf_addch(&buf,'@');+if(c->host)+strbuf_addstr_urlencode(&buf,c->host,1);+if(c->path){+strbuf_addch(&buf,'/');+strbuf_addstr_urlencode(&buf,c->path,0);+}++rewrite_credential_file(fn,c,&buf);+strbuf_release(&buf);+}++staticvoidremove_credential(constchar*fn,structcredential*c)+{+/*+*Sanitycheckthatweactuallyhavesomethingtomatch+*against.Theinputwegetisarestrictivepattern,+*sotechnicallyablankcredentialmeans"erase everything".+*Butitistooeasytoaccidentallysendthis,sinceitisequivalent+*toemptyinput.Soexplicitlydisallowit,andrequirethatthe+*patternhavesomeactualcontenttomatch.+*/+if(c->protocol||c->host||c->path||c->username)+rewrite_credential_file(fn,c,NULL);+}++staticintlookup_credential(constchar*fn,structcredential*c)+{+parse_credential_file(fn,c,print_entry,NULL);+returnc->username&&c->password;+}++intmain(intargc,constchar**argv)+{+constchar*constusage[]={+"git credential-store [options] <action>",+NULL+};+constchar*op;+structcredentialc=CREDENTIAL_INIT;+char*file=NULL;+structoptionoptions[]={+OPT_STRING(0,"file",&file,"path",+"fetch and store credentials in <path>"),+OPT_END()+};++umask(077);++argc=parse_options(argc,argv,NULL,options,usage,0);+if(argc!=1)+usage_with_options(usage,options);+op=argv[0];++if(!file)+file=expand_user_path("~/.git-credentials");+if(!file)+die("unable to set up default path; use --file");++if(credential_read(&c,stdin)<0)+die("unable to read credential");++if(!strcmp(op,"get"))+lookup_credential(file,&c);+elseif(!strcmp(op,"erase"))+remove_credential(file,&c);+elseif(!strcmp(op,"store"))+store_credential(file,&c);+else+;/* Ignore unknown operation. */++return0;+}
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
We already have tests for the internal helpers, but it's
nice to give authors of external tools an easy way to
sanity-check their helpers.
If you have written the "git-credential-foo" helper, you can
do so with:
GIT_TEST_CREDENTIAL_HELPER=foo \
make t0303-credential-external.sh
This assumes that your helper is capable of both storing and
retrieving credentials (some helpers may be read-only, and
they will fail these tests).
If your helper supports time-based expiration with a
configurable timeout, you can test that feature like this:
GIT_TEST_CREDENTIAL_HELPER_TIMEOUT="foo --timeout=1" \
make t0303-credential-external.sh
Signed-off-by: Jeff King <redacted>
---
This version adds GIT_TEST_CREDENTIAL_HELPER_SETUP, which is an
unfortunate hack needed for the OS X helper (I'll post that patch in a
few minutes).
t/t0303-credential-external.sh | 39 +++++++++++++++++++++++++++++++++++++++
1 files changed, 39 insertions(+), 0 deletions(-)
create mode 100755 t/t0303-credential-external.sh
@@ -0,0 +1,39 @@+#!/bin/sh++test_description='external credential helper tests'+../test-lib.sh+."$TEST_DIRECTORY"/lib-credential.sh++pre_test(){+test-z"$GIT_TEST_CREDENTIAL_HELPER_SETUP"||+eval"$GIT_TEST_CREDENTIAL_HELPER_SETUP"++# clean before the test in case there is cruft left+# over from a previous run that would impact results+helper_test_clean"$GIT_TEST_CREDENTIAL_HELPER"+}++post_test(){+# clean afterwards so that we are good citizens+# and don't leave cruft in the helper's storage, which+# might be long-term system storage+helper_test_clean"$GIT_TEST_CREDENTIAL_HELPER"+}++iftest-z"$GIT_TEST_CREDENTIAL_HELPER";then+say"# skipping external helper tests (set GIT_TEST_CREDENTIAL_HELPER)"+else+pre_test+helper_test"$GIT_TEST_CREDENTIAL_HELPER"+post_test+fi++iftest-z"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT";then+say"# skipping external helper timeout tests"+else+pre_test+helper_test_timeout"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"+post_test+fi++test_done
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
On Sat, Dec 10, 2011 at 05:28:27AM -0500, Jeff King wrote:
Here's the latest re-roll of the credential helpers series. I think this
one is probably ready to go to 'next'.
And here's the prompt/terminal refactoring to go on top of it. This
isn't as well reviewed as the main credential series, but I think it's
reasonably solid.
Since the last round, I fixed the bone-head NO_DEV_TTY/HAVE_DEV_TTY
mixup that Jakub pointed out. And I split the "Linux has /dev/tty" patch
out so I could test to make sure it isn't accidentally turned on until
we actually enable it.
I also tested it on OS X, and it works fine there.
[1/9]: imap-send: avoid buffer overflow
[2/9]: imap-send: don't check return value of git_getpass
[3/9]: move git_getpass to its own source file
[4/9]: refactor git_getpass into generic prompt function
[5/9]: add generic terminal prompt function
[6/9]: prompt: use git_terminal_prompt
[7/9]: credential: use git_prompt instead of git_getpass
[8/9]: Makefile: linux has /dev/tty
[9/9]: Makefile: OS X has /dev/tty
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
We format the password prompt in an 80-character static
buffer. It contains the remote host and username, so it's
unlikely to overflow (or be exploitable by a remote
attacker), but there's no reason not to be careful and use
a strbuf.
Signed-off-by: Jeff King <redacted>
---
imap-send.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
git_getpass will always die() if we weren't able to get
input, so there's no point looking for NULL.
Signed-off-by: Jeff King <redacted>
---
imap-send.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
This is currently in connect.c, but really has nothing to
do with the git protocol itself. Let's make a new source
file all about prompting the user, which will make it
cleaner to refactor.
Signed-off-by: Jeff King <redacted>
---
Makefile | 2 ++
cache.h | 1 -
connect.c | 44 --------------------------------------------
credential.c | 1 +
imap-send.c | 1 +
prompt.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
prompt.h | 6 ++++++
7 files changed, 58 insertions(+), 45 deletions(-)
create mode 100644 prompt.c
create mode 100644 prompt.h
@@ -619,47 +619,3 @@ int finish_connect(struct child_process *conn)free(conn);returncode;}--char*git_getpass(constchar*prompt)-{-constchar*askpass;-structchild_processpass;-constchar*args[3];-staticstructstrbufbuffer=STRBUF_INIT;--askpass=getenv("GIT_ASKPASS");-if(!askpass)-askpass=askpass_program;-if(!askpass)-askpass=getenv("SSH_ASKPASS");-if(!askpass||!(*askpass)){-char*result=getpass(prompt);-if(!result)-die_errno("Could not read password");-returnresult;-}--args[0]=askpass;-args[1]=prompt;-args[2]=NULL;--memset(&pass,0,sizeof(pass));-pass.argv=args;-pass.out=-1;--if(start_command(&pass))-exit(1);--strbuf_reset(&buffer);-if(strbuf_read(&buffer,pass.out,20)<0)-die("failed to read password from %s\n",askpass);--close(pass.out);--if(finish_command(&pass))-exit(1);--strbuf_setlen(&buffer,strcspn(buffer.buf,"\r\n"));--returnbuffer.buf;-}
@@ -0,0 +1,48 @@+#include"cache.h"+#include"run-command.h"+#include"strbuf.h"+#include"prompt.h"++char*git_getpass(constchar*prompt)+{+constchar*askpass;+structchild_processpass;+constchar*args[3];+staticstructstrbufbuffer=STRBUF_INIT;++askpass=getenv("GIT_ASKPASS");+if(!askpass)+askpass=askpass_program;+if(!askpass)+askpass=getenv("SSH_ASKPASS");+if(!askpass||!(*askpass)){+char*result=getpass(prompt);+if(!result)+die_errno("Could not read password");+returnresult;+}++args[0]=askpass;+args[1]=prompt;+args[2]=NULL;++memset(&pass,0,sizeof(pass));+pass.argv=args;+pass.out=-1;++if(start_command(&pass))+exit(1);++strbuf_reset(&buffer);+if(strbuf_read(&buffer,pass.out,20)<0)+die("failed to read password from %s\n",askpass);++close(pass.out);++if(finish_command(&pass))+exit(1);++strbuf_setlen(&buffer,strcspn(buffer.buf,"\r\n"));++returnbuffer.buf;+}
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
This will allow callers to specify more options (e.g.,
leaving echo on). The original git_getpass becomes a slim
wrapper around the new function.
Signed-off-by: Jeff King <redacted>
---
prompt.c | 46 ++++++++++++++++++++++++++++++----------------
prompt.h | 3 +++
2 files changed, 33 insertions(+), 16 deletions(-)
@@ -3,26 +3,13 @@#include"strbuf.h"#include"prompt.h"-char*git_getpass(constchar*prompt)+staticchar*do_askpass(constchar*cmd,constchar*prompt){-constchar*askpass;structchild_processpass;constchar*args[3];staticstructstrbufbuffer=STRBUF_INIT;-askpass=getenv("GIT_ASKPASS");-if(!askpass)-askpass=askpass_program;-if(!askpass)-askpass=getenv("SSH_ASKPASS");-if(!askpass||!(*askpass)){-char*result=getpass(prompt);-if(!result)-die_errno("Could not read password");-returnresult;-}--args[0]=askpass;+args[0]=cmd;args[1]=prompt;args[2]=NULL;
@@ -35,7 +22,7 @@strbuf_reset(&buffer);if(strbuf_read(&buffer,pass.out,20)<0)-die("failed to read password from %s\n",askpass);+die("failed to get '%s' from %s\n",prompt,cmd);close(pass.out);
@@ -46,3 +33,30 @@returnbuffer.buf;}++char*git_prompt(constchar*prompt,intflags)+{+char*r;++if(flags&PROMPT_ASKPASS){+constchar*askpass;++askpass=getenv("GIT_ASKPASS");+if(!askpass)+askpass=askpass_program;+if(!askpass)+askpass=getenv("SSH_ASKPASS");+if(askpass&&*askpass)+returndo_askpass(askpass,prompt);+}++r=getpass(prompt);+if(!r)+die_errno("could not read '%s'",prompt);+returnr;+}++char*git_getpass(constchar*prompt)+{+returngit_prompt(prompt,PROMPT_ASKPASS);+}
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
When we need to prompt the user for input interactively, we
want to access their terminal directly. We can't rely on
stdio because it may be connected to pipes or files, rather
than the terminal. Instead, we use "getpass()", because it
abstracts the idea of prompting and reading from the
terminal. However, it has some problems:
1. It never echoes the typed characters, which makes it OK
for passwords but annoying for other input (like usernames).
2. Some implementations of getpass() have an extremely
small input buffer (e.g., Solaris 8 is reported to
support only 8 characters).
3. Some implementations of getpass() will fall back to
reading from stdin (e.g., glibc). We explicitly don't
want this, because our stdin may be connected to a pipe
speaking a particular protocol, and reading will
disrupt the protocol flow (e.g., the remote-curl
helper).
4. Some implementations of getpass() turn off signals, so
that hitting "^C" on the terminal does not break out of
the password prompt. This can be a mild annoyance.
Instead, let's provide an abstract "git_terminal_prompt"
function that addresses these concerns. This patch includes
an implementation based on /dev/tty, enabled by setting
HAVE_DEV_TTY. The fallback is to use getpass() as before.
Signed-off-by: Jeff King <redacted>
---
Makefile | 9 ++++++
compat/terminal.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
compat/terminal.h | 6 ++++
3 files changed, 96 insertions(+), 0 deletions(-)
create mode 100644 compat/terminal.c
create mode 100644 compat/terminal.h
@@ -227,6 +227,9 @@ all::## Define NO_REGEX if you have no or inferior regex support in your C library.#+# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the+# user.+## Define GETTEXT_POISON if you are debugging the choice of strings marked# for translation. In a GETTEXT_POISON build, you can turn all strings marked# for translation into gibberish by setting the GIT_GETTEXT_POISON variable
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
Our custom implementation of git_terminal_prompt has many
advantages over regular getpass(), as described in the prior
commit.
This also lets us implement a PROMPT_ECHO flag for callers
who want it.
Signed-off-by: Jeff King <redacted>
---
prompt.c | 3 ++-
prompt.h | 1 +
2 files changed, 3 insertions(+), 1 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
We use git_getpass to retrieve the username and password
from the terminal. However, git_getpass will not echo the
username as the user types. We can fix this by using the
more generic git_prompt, which underlies git_getpass but
lets us specify an "echo" option.
Signed-off-by: Jeff King <redacted>
---
credential.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
@@ -121,11 +122,7 @@ static void credential_describe(struct credential *c, struct strbuf *out)elsestrbuf_addf(&prompt,"%s: ",what);-/* FIXME: for usernames, we should do something less magical that-*actuallyechoesthecharacters.However,weneedtoreadfrom-*/dev/ttyandnotstdio,whichisnotportable(butgetpasswilldo-*itforus).http.cusesthesameworkaround.*/-r=git_getpass(prompt.buf);+r=git_prompt(prompt.buf,flags);strbuf_release(&desc);strbuf_release(&prompt);
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
Therefore we can turn on our custom prompt function instead
of relying on getpass.
Signed-off-by: Jeff King <redacted>
---
Makefile | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
We can use our enhanced getpass(). Tested by me.
Signed-off-by: Jeff King <redacted>
---
Makefile | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
On Sat, Dec 10, 2011 at 05:28:27AM -0500, Jeff King wrote:
Here's the latest re-roll of the credential helpers series. I think this
one is probably ready to go to 'next'.
It's rebased on the latest tip of 'master' (applying it to an older
commit will get you a minor textual conflict in strbuf.c). It
incorporates the erase-safety we discussed, fixes a few commit message
typos, and tweaks the test scripts to make testing the external OS X
helper a little easier.
And here is that helper.
-- >8 --
Subject: [PATCH] contrib: add credential helper for OS X Keychain
With this installed in your $PATH, you can store
git-over-http passwords in your keychain by doing:
git config credential.helper osxkeychain
The code is based in large part on the work of Jay Soffian,
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=osxkeychain \
GIT_TEST_CREDENTIAL_HELPER_SETUP="export HOME=$HOME" \
./t0303-credential-external.sh
The "HOME" setup is unfortunately necessary. The test
scripts set HOME to the trash directory, but this causes the
keychain API to complain.
Signed-off-by: Jeff King <redacted>
---
I tried clever workarounds for the HOME thing, like running "security
create-keychain" in the trash directory. But if I try to create the
"login" keychain, it says it's already there! So it's like we're in a
weird limbo where we both have and do not have access to the keychains.
I'm not too concerned with the hackishness of my solution, though; this
isn't even a part of the regular test scripts, but just a thing you can
do manually to test the helper from contrib.
Many thanks to Jay for the initial version; even though this version is
quite chopped-up, having somebody else provide a nice example of how to
use the SecKeychain functions made it much easier. I tried to trim the
code down to the bare essentials to glue our input into the SecKeychain
API, and to show good practices for other helpers to follow.
I dropped the python version, as it is redundant and I didn't feel like
porting it to the new interface. It could be added on top if somebody
feels like converting it as an exercise. :)
contrib/credential/osxkeychain/.gitignore | 1 +
contrib/credential/osxkeychain/Makefile | 14 ++
.../osxkeychain/git-credential-osxkeychain.c | 173 ++++++++++++++++++++
3 files changed, 188 insertions(+), 0 deletions(-)
create mode 100644 contrib/credential/osxkeychain/.gitignore
create mode 100644 contrib/credential/osxkeychain/Makefile
create mode 100644 contrib/credential/osxkeychain/git-credential-osxkeychain.c
From: Jakub Narebski <hidden> Date: 2016-06-15 22:52:36
Jeff King [off-list ref] writes:
There are a few places in git that need to get a username
and password credential from the user; the most notable one
is HTTP authentication for smart-http pushing.
A question: does it work also for access via SSH, either without
public key set up (i.e. 'keyboard-interactive'), or with encrypted
private key without ssh-agent set up?
It would probably require URL i.e. ssh://git.example.com/srv/scm/repo.git
or git+ssh://git.example.com/srv/scm/repo.git and not scp-like
address i.e. user@git.example.com:/srv/scm/repo.git, isn't it?
--
Jakub Narębski
From: Jakub Narebski <hidden> Date: 2016-06-15 22:52:36
Jeff King [off-list ref] writes:
[...]
This is nothing you couldn't do with a clever credential
helper at the start of your stack, like:
[credential "http://"]
helper = "!f() { grep -v ^path= ; }; f"
helper = your_real_helper
But doing this:
[credential]
useHttpPath = false
Shouldn't this be 'usePath' or 'usePathComponent' or 'useRepositoryPath',
etc.? Because if^W when remote helper for Subversion is complete, you
could have svn://svnserve.example.com/path/to/repo as an URL... which
would be not HTTP(S).
--
Jakub Narębski
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
On Sat, Dec 10, 2011 at 03:43:01AM -0800, Jakub Narebski wrote:
Jeff King [off-list ref] writes:
quoted
There are a few places in git that need to get a username
and password credential from the user; the most notable one
is HTTP authentication for smart-http pushing.
A question: does it work also for access via SSH, either without
public key set up (i.e. 'keyboard-interactive'), or with encrypted
private key without ssh-agent set up?
No. ssh handles its own password querying, and contacts the user
directly via the terminal. And there's not much point in using a
password helper with ssh; if you don't want to type your password, set
up a key and use ssh-agent.
It would probably require URL i.e. ssh://git.example.com/srv/scm/repo.git
or git+ssh://git.example.com/srv/scm/repo.git and not scp-like
address i.e. user@git.example.com:/srv/scm/repo.git, isn't it?
It's not a matter of recognizing the URL; it's that we hand off the
authentication problem to ssh, which takes care of it entirely itself.
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:52:36
On Sat, Dec 10, 2011 at 03:50:11AM -0800, Jakub Narebski wrote:
quoted
This is nothing you couldn't do with a clever credential
helper at the start of your stack, like:
[credential "http://"]
helper = "!f() { grep -v ^path= ; }; f"
helper = your_real_helper
But doing this:
[credential]
useHttpPath = false
Shouldn't this be 'usePath' or 'usePathComponent' or 'useRepositoryPath',
etc.? Because if^W when remote helper for Subversion is complete, you
could have svn://svnserve.example.com/path/to/repo as an URL... which
would be not HTTP(S).
It must be per-protocol, because paths will be relevant for some
protocols (e.g., unlocking certificates in the local filesystem).
So if anything, it would be "useNetworkPath" or something, if we wanted
to unify svn and http. But it would also be OK to have a separate flag
for http and svn, which is more flexible (and most users aren't going to
set it at all, so they won't notice).
-Peff
What is this `reserved` flag for, and when should one use it?
It would be nice to have a short comment...
It indicates whether one should encode rfc3986 reserved characters. You
want to use it when encoding the host, username, and password portions
of a URL (i.e., before the "/"), but not the path (since you don't want
to encode all of the slashes). If you were breaking down the path more
(e.g., into a "query" and "fragment" portion), you would care about more
specific quoting there, but we don't; we treat everything after the
slash as an opaque blob of path.
Patch to the strbuf api documentation is below. I think it should be
squashed into patch 12/13.
It is aligned correctly. When you ident by tabs, the "+" of the diff
gets soaked in the first tabstop, so it is off-by-one with respect to
non-tabbed input (it is off even more in the quoted section above,
because "> > +" gets soaked into the first tabstop). You can see your
version above also is misaligned when I quote it. :)
If you apply the diff, the result is as you expected.
-Peff
---
@@ -270,3 +270,14 @@ same behaviour as well. third argument can be used to set the environment which the editor is run in. If the buffer is NULL the editor is launched as usual but the file's contents are not read into the buffer upon completion.++`strbuf_add_urlencode`::++ Copy data to the end of the buffer, percent-encoding it as per+ rfc3986. If the reserved flag is non-zero, then characters in+ the rfc3986 reserved list are percent-encoded; otherwise, they+ are copied literally. Characters in the rfc3986 unreserved list+ are always copied literally. All other characters are+ percent-encoded. Typically, one would use the reserved flag for+ the host and user-info sections of a URL, but leave special+ characters in the path untouched.
From: Johannes Sixt <hidden> Date: 2016-06-15 22:52:37
Signed-off-by: Johannes Sixt <redacted>
---
These two patches go on top of jk/credentials.
Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
From: Johannes Sixt <hidden> Date: 2016-06-15 22:52:37
Introduce a configuration option NO_UNIX_SOCKETS to exclude code that
depends on Unix sockets and use it in MSVC and MinGW builds.
Notice that unix-socket.h was missing from LIB_H before; fix that, too.
Signed-off-by: Johannes Sixt <redacted>
---
All or most of the tests introduced by the series fail on Windows.
What is the preferred way to exclude them? Mark the all with a
prerequisite? Or exit early at the beginning of the test file?
Makefile | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
@@ -143,6 +143,8 @@ all::## Define NO_IPV6 if you lack IPv6 support and getaddrinfo().#+# Define NO_UNIX_SOCKETS if your system does not offer unix sockets.+## Define NO_SOCKADDR_STORAGE if your platform does not have struct# sockaddr_storage.#
Unless I've bungled something, these should be no-ops, shouldn't they?
The most recent version of the prompt series has platforms opting into
the replacement with HAVE_DEV_TTY.
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:52:37
On Mon, Dec 12, 2011 at 10:12:56PM +0100, Johannes Sixt wrote:
Introduce a configuration option NO_UNIX_SOCKETS to exclude code that
depends on Unix sockets and use it in MSVC and MinGW builds.
Makes sense, thanks.
Notice that unix-socket.h was missing from LIB_H before; fix that, too.
Oops, thanks.
All or most of the tests introduced by the series fail on Windows.
What is the preferred way to exclude them? Mark the all with a
prerequisite? Or exit early at the beginning of the test file?
The cache daemon is the only thing that uses sockets. I would suggest
just exiting early from its script (patch below).
The rest of the tests should pass under Windows. If they don't, then
they should be fixed (I'm happy to work on that, with the limitation
that I don't actually have a Windows box, and if at all possible I'd
like to keep it that way).
In theory we should also disable the documentation for credential-cache.
But that means surgery not only to Documentation/Makefile, but figuring
out how to pass the flags down to the actual asciidoc process (since
gitcredentials(7) mentions it in the text). Certainly possible, but I
don't know if it's worth the effort or not.
---
From: Junio C Hamano <hidden> Date: 2016-06-15 22:52:37
I'll queue a single patch that is a squash between 2/2 and Peff's test
updates between "credentials: add "cache" helper" and "strbuf: add
strbuf_add*_urlencode" in the series.
Thanks.
From: Johannes Sixt <hidden> Date: 2016-06-15 22:52:37
Am 13.12.2011 01:45, schrieb Junio C Hamano:
I'll queue a single patch that is a squash between 2/2 and Peff's test
updates between "credentials: add "cache" helper" and "strbuf: add
strbuf_add*_urlencode" in the series.
Thanks. The resulting series builds fine on Windows and passes/skips the
new tests in the expected manner.
-- Hannes
It could, but it's a no-op as-is, as term_fd will be -1.
I agree it might be a little more obvious to put it there (I think what
happened is my initial revision did not look at "echo" ever again, and
then that conditional was added later when I realized that the "!echo"
case needed us to print the newline manually).
And why no sigchain_pop() for the signal handler?
Because I used sigchain_push_common, which has no pop_common analog. But
it's OK, because calling restore_term sets term_fd to -1, making further
calls a no-op. So leaving the handler in place is fine.
Another option would be to add sigchain_pop_common, which pops the
same signals from push_common.
-Peff
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:52:39
peff@peff.net wrote on Thu, 15 Dec 2011 08:39 -0500:
I agree it might be a little more obvious to put it there (I think what
happened is my initial revision did not look at "echo" ever again, and
then that conditional was added later when I realized that the "!echo"
case needed us to print the newline manually).
quoted
And why no sigchain_pop() for the signal handler?
Because I used sigchain_push_common, which has no pop_common analog. But
it's OK, because calling restore_term sets term_fd to -1, making further
calls a no-op. So leaving the handler in place is fine.
Another option would be to add sigchain_pop_common, which pops the
same signals from push_common.
Thanks for the detailed explanation. It was indeed the lack of
symmetry that set me off. I missed the term_fd check for both.
-- Pete
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:52:45
Hi,
Jeff King wrote:
This patch introduces a credential helper that will cache
passwords in memory for a short period of time.
Signed-off-by: Jeff King <redacted>
[...]
t/t0301-credential-cache.sh | 18 ++
This test is failing for me:
expecting success:
check fill $HELPER <<-\EOF
protocol=https
host=example.com
--
username=askpass-username
password=askpass-password
--
askpass: Username for 'https://example.com':
askpass: Password for 'https://askpass-username@example.com':
EOF
--- expect-stderr 2012-01-10 00:07:02.398365248 +0000
+++ stderr 2012-01-10 00:07:02.418364996 +0000
@@ -1,2 +1,3 @@
+fatal: socket path is too long to fit in sockaddr
askpass: Username for 'https://example.com':
askpass: Password for 'https://askpass-username@example.com':
not ok - 1 helper (cache) has no existing data
I didn't notice until recently because typically the cwd is short.
The sun_path[] array on this machine is 108 bytes; POSIX informs me
that some platforms make it as small as 96 bytes and there's no
guaranteed lower limit. The machines[*] triggering it were running
tests in a chroot, hence the long path.
So you should be able to reproduce this with
longpath=foo/bar; # > 6 chars
longpath=$longpath/$longpath/$longpath/$longpath; # > 24
longpath=$longpath/$longpath/$longpath/$longpath; # > 96
longpath=/tmp/$longpath/$longpath
mkdir -p $longpath
sh t0301-credential-cache.sh --root=$longpath
Ideas?
---
[*] https://buildd.debian.org/status/logs.php?pkg=git&ver=1%3A1.7.9~rc0-1
credential-cache.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
@@ -102,10 +102,18 @@ int main(int argc, const char **argv)usage_with_options(usage,options);op=argv[0];-if(!socket_path)-socket_path=expand_user_path("~/.git-credential-cache/socket");-if(!socket_path)-die("unable to find a suitable socket path; use --socket");+if(!socket_path){+char*home=expand_user_path("~");+if(!home)+die("unable to find a suitable socket path; use --socket");++if(!chdir(home))+socket_path=".git-credential-cache/socket";+elseif(errno==ENOENT&&!strcmp(op,"exit"))+return0;+else+die("cannot enter home directory");+}if(!strcmp(op,"exit"))do_cache(socket_path,op,timeout,0);
From: Jeff King <hidden> Date: 2016-06-15 22:52:45
On Mon, Jan 09, 2012 at 07:50:38PM -0600, Jonathan Nieder wrote:
--- expect-stderr 2012-01-10 00:07:02.398365248 +0000
+++ stderr 2012-01-10 00:07:02.418364996 +0000
@@ -1,2 +1,3 @@
+fatal: socket path is too long to fit in sockaddr
askpass: Username for 'https://example.com':
askpass: Password for 'https://askpass-username@example.com':
not ok - 1 helper (cache) has no existing data
I didn't notice until recently because typically the cwd is short.
The sun_path[] array on this machine is 108 bytes; POSIX informs me
that some platforms make it as small as 96 bytes and there's no
guaranteed lower limit. The machines[*] triggering it were running
tests in a chroot, hence the long path.
Ugh. Some days I really love working on Unix systems. And then some days
you run across junk like this.
Presumably Linux has kept to 108 to avoid breaking older programs. So
it's not as if we can assume it will be changed soon, or just write this
off as a limitation for old crappy systems.
Googling around, I've seen some indication that you can simply
over-allocate the sockaddr_un and pass a longer length to connect.
However that just seems to yield EINVAL on Linux (and even if it did
work on Linux, I'd be surprised if it worked everywhere).
Which really leaves us with only one option: chdir and bind to a
relative path, as you did below.
- if (!socket_path)
- socket_path = expand_user_path("~/.git-credential-cache/socket");
- if (!socket_path)
- die("unable to find a suitable socket path; use --socket");
+ if (!socket_path) {
+ char *home = expand_user_path("~");
+ if (!home)
+ die("unable to find a suitable socket path; use --socket");
+
+ if (!chdir(home))
+ socket_path = ".git-credential-cache/socket";
+ else if (errno == ENOENT && !strcmp(op, "exit"))
+ return 0;
+ else
+ die("cannot enter home directory");
+ }
I think this is the right approach, but the wrong place to do it. Your
patch only helps people using the default path (so passing
--socket=$longpath would still be broken). And we'd need a similar fix
for the binding side in credential-cache--daemon.
Here's a patch which passes your $longpath test.
-- >8 --
Subject: [PATCH] unix-socket: handle long socket pathnames
On many systems, the sockaddr_un.sun_path field is quite
small. Even on Linux, it is only 108 characters. A user of
the credential-cache daemon can easily surpass this,
especially if their home directory is in a deep directory
tree (since the default location expands ~/.git-credentials).
We can hack around this in the unix-socket.[ch] code by
doing a chdir() to the enclosing directory, feeding the
relative basename to the socket functions, and then
restoring the working directory.
This introduces several new possible error cases for
creating a socket, including an irrecoverable one in the
case that we can't restore the working directory. In the
case of the credential-cache code, we could perhaps get away
with simply chdir()-ing to the socket directory and never
coming back. However, I'd rather do it at the lower level
for a few reasons:
1. It keeps the hackery behind an opaque interface instead
of polluting the main program logic.
2. A hack in credential-cache won't help any unix-socket
users who come along later.
3. The chdir trickery isn't that likely to fail (basically
it's only a problem if your cwd is missing or goes away
while you're running). And because we only enable the
hack when we get a too-long name, it can only fail in
cases that would have failed under the previous code
anyway.
Signed-off-by: Jeff King <redacted>
---
unix-socket.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 69 insertions(+), 5 deletions(-)
@@ -9,27 +9,86 @@ static int unix_stream_socket(void)returnfd;}-staticvoidunix_sockaddr_init(structsockaddr_un*sa,constchar*path)+staticintchdir_len(constchar*orig,intlen)+{+char*path=xmemdupz(orig,len);+intr=chdir(path);+free(path);+returnr;+}++structunix_sockaddr_context{+charorig_dir[PATH_MAX];+};++staticvoidunix_sockaddr_cleanup(structunix_sockaddr_context*ctx)+{+if(!ctx->orig_dir[0])+return;+/*+*Ifwefail,wecan'tjustreturnanerror,sincewehave+*movedthecwdofthewholeprocess,whichcouldconfusecalling+*code.Wearebetterofftojustdie.+*/+if(chdir(ctx->orig_dir)<0)+die("unable to restore original working directory");+}++staticintunix_sockaddr_init(structsockaddr_un*sa,constchar*path,+structunix_sockaddr_context*ctx){intsize=strlen(path)+1;-if(size>sizeof(sa->sun_path))-die("socket path is too long to fit in sockaddr");++ctx->orig_dir[0]='\0';+if(size>sizeof(sa->sun_path)){+constchar*slash=find_last_dir_sep(path);+constchar*dir;+intdir_len;++if(!slash){+errno=ENAMETOOLONG;+return-1;+}++dir=path;+dir_len=slash-path;++path=slash+1;+size=strlen(path)+1;+if(size>sizeof(sa->sun_path)){+errno=ENAMETOOLONG;+return-1;+}++if(!getcwd(ctx->orig_dir,sizeof(ctx->orig_dir))){+errno=ENAMETOOLONG;+return-1;+}+if(chdir_len(dir,dir_len)<0)+return-1;+}+memset(sa,0,sizeof(*sa));sa->sun_family=AF_UNIX;memcpy(sa->sun_path,path,size);+return0;}intunix_stream_connect(constchar*path){intfd;structsockaddr_unsa;+structunix_sockaddr_contextctx;-unix_sockaddr_init(&sa,path);+if(unix_sockaddr_init(&sa,path,&ctx)<0)+return-1;fd=unix_stream_socket();if(connect(fd,(structsockaddr*)&sa,sizeof(sa))<0){+unix_sockaddr_cleanup(&ctx);close(fd);return-1;}+unix_sockaddr_cleanup(&ctx);returnfd;}
@@ -37,20 +96,25 @@ int unix_stream_listen(const char *path){intfd;structsockaddr_unsa;+structunix_sockaddr_contextctx;-unix_sockaddr_init(&sa,path);+if(unix_sockaddr_init(&sa,path,&ctx)<0)+return-1;fd=unix_stream_socket();unlink(path);if(bind(fd,(structsockaddr*)&sa,sizeof(sa))<0){+unix_sockaddr_cleanup(&ctx);close(fd);return-1;}if(listen(fd,5)<0){+unix_sockaddr_cleanup(&ctx);close(fd);return-1;}+unix_sockaddr_cleanup(&ctx);returnfd;}
From: Jeff King <hidden> Date: 2016-06-15 22:52:45
On Mon, Jan 09, 2012 at 11:44:30PM -0500, Jeff King wrote:
Subject: [PATCH] unix-socket: handle long socket pathnames
And I think this should go on top. You were lucky enough that I used a
die() in the original code for your condition (because I thought it was
a "this could never happen, right?" condition). Had it simply returned
an error, the cache-daemon would have silently failed to do anything,
which would have been much more confusing for you. :)
We probably should have done this as part of Clemens' 98c2924, but I
didn't think of it then (but the same reasoning applies to both
patches).
-- >8 --
Subject: [PATCH] credential-cache: report more daemon connection errors
Originally, this code remained relatively silent when we
failed to connect to the cache. The idea was that it was
simply a cache, and we didn't want to bother the user with
temporary failures (the worst case is that we would simply
ask their password again).
However, if you have a configuration failure or other
problem, it is helpful for the daemon to report those
problems. Git will happily ignore the failed error code, but
the extra information to stderr can help the user diagnose
the problem.
Signed-off-by: Jeff King <redacted>
---
credential-cache.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
@@ -71,10 +71,14 @@ static void do_cache(const char *socket, const char *action, int timeout,die_errno("unable to relay credential");}-if(send_request(socket,&buf)<0&&(flags&FLAG_SPAWN)){-spawn_daemon(socket);-if(send_request(socket,&buf)<0)+if(send_request(socket,&buf)<0){+if(errno!=ENOENT)die_errno("unable to connect to cache daemon");+if(flags&FLAG_SPAWN){+spawn_daemon(socket);+if(send_request(socket,&buf)<0)+die_errno("unable to connect to cache daemon");+}}strbuf_release(&buf);}
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:52:45
Jeff King wrote:
2. A hack in credential-cache won't help any unix-socket
users who come along later.
3. The chdir trickery isn't that likely to fail (basically
it's only a problem if your cwd is missing or goes away
while you're running). And because we only enable the
hack when we get a too-long name, it can only fail in
cases that would have failed under the previous code
anyway.
Signed-off-by: Jeff King <redacted>
A part of me worries that this assumption (3), and the additional
assumption that nothing running concurrently cares about the cwd,
might come back to bite us when the future (2) arrives. But I don't
see a better approach.
Follow-on ideas: on platforms that support it, it would be nice to use
ctx->orig_dirfd = open(".", O_RDONLY);
if (ctx->orig_dirfd < 0)
... error handling ...
...
if (ctx->orig_dirfd >= 0) {
if (fchdir(ctx->orig_dirfd))
die_errno("unable to restore original working directory");
close(ctx->orig_dirfd);
}
quoted hunk
--- a/unix-socket.c+++ b/unix-socket.c
@@ -9,27 +9,86 @@ static int unix_stream_socket(void)
[...]
+ dir = path;
+ dir_len = slash - path;
[...]
+ if (chdir_len(dir, dir_len) < 0)
+ return -1;
Could avoid some complication by eliminating the dir_len var.
if (chdir_len(dir, slash - dir))
return -1;
Neither seems worth holding up the patch, so
Reviewed-by: Jonathan Nieder <redacted>
From: Jeff King <hidden> Date: 2016-06-15 22:52:45
On Tue, Jan 10, 2012 at 11:44:21AM -0600, Jonathan Nieder wrote:
Jeff King wrote:
quoted
2. A hack in credential-cache won't help any unix-socket
users who come along later.
3. The chdir trickery isn't that likely to fail (basically
it's only a problem if your cwd is missing or goes away
while you're running). And because we only enable the
hack when we get a too-long name, it can only fail in
cases that would have failed under the previous code
anyway.
Signed-off-by: Jeff King <redacted>
A part of me worries that this assumption (3), and the additional
assumption that nothing running concurrently cares about the cwd,
might come back to bite us when the future (2) arrives. But I don't
see a better approach.
The problem is that when future (2) arrives, a credential-cache specific
hack won't be helping it at all. :)
To be honest, I don't really expect a lot of future unix-socket users.
It's not portable, and most of our IPC happens over pipes. The design of
the cache daemon is very specific in requiring a true
many-clients-to-one-server model, but also caring deeply about access
controls (making TCP sockets less good[1]).
[1] One could in theory do a loopback TCP socket and provide a random
token read from an access-controlled file. But that ends up a lot
more complicated and introduces new attack surfaces. Which is a bad
thing for security-sensitive code like this.
Follow-on ideas: on platforms that support it, it would be nice to use
ctx->orig_dirfd = open(".", O_RDONLY);
if (ctx->orig_dirfd < 0)
... error handling ...
...
if (ctx->orig_dirfd >= 0) {
if (fchdir(ctx->orig_dirfd))
die_errno("unable to restore original working directory");
close(ctx->orig_dirfd);
}
Yeah, I started by using fchdir, but noticed that we don't use it
anywhere else and didn't want to create a portability problem. It does
fix the "somebody deleted your cwd while you were gone from it" problem.
But if you have no cwd at all, the open call will still fail.
Still, it would be slightly more robust. I wonder how portable fchdir
is in practice (I guess we could always fall back to the getcwd code
path). Do you want to prepare a patch on top?
[...]
quoted
+ dir = path;
+ dir_len = slash - path;
[...]
quoted
+ if (chdir_len(dir, dir_len) < 0)
+ return -1;
Could avoid some complication by eliminating the dir_len var.
if (chdir_len(dir, slash - dir))
return -1;
Ah, yeah. Originally I had written it so that "slash" didn't survive
untouched to there, but in the current code, that would work.
Junio, if you haven't merged it to next yet, it might be worth squashing
in the patch below. Otherwise I wouldn't worry about it.
Neither seems worth holding up the patch, so
Reviewed-by: Jonathan Nieder <redacted>
Thanks for the review (and for, as usual, a well-written bug report).
---
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:52:46
Jeff King wrote:
Still, it would be slightly more robust. I wonder how portable fchdir
is in practice (I guess we could always fall back to the getcwd code
path). Do you want to prepare a patch on top?
I've been wanting to get around to doing something similar for setup.c
for a while. I'm happy enough to forget about it for now. ;-)
Thanks again for the fix. Here's another quick nit.
-- >8 --
Subject: unix-socket: do not let close() or chdir() clobber errno during cleanup
unix_stream_connect and unix_stream_listen return -1 on error, with
errno set by the failing underlying call to allow the caller to write
a useful diagnosis.
Unfortunately the error path involves a few system calls itself, such
as close(), that can themselves touch errno.
This is not as worrisome as it might sound. If close() fails, this
just means substituting one meaningful error message for another,
which is perfectly fine. However, when the call _succeeds_, it is
allowed to (and sometimes might) clobber errno along the way with some
undefined value, so it is good higiene to save errno and restore it
immediately before returning to the caller. Do so.
Signed-off-by: Jonathan Nieder <redacted>
---
unix-socket.c | 39 ++++++++++++++++++++++-----------------
1 files changed, 22 insertions(+), 17 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 22:52:49
On Mon, Jan 09, 2012 at 11:57:33PM -0500, Jeff King wrote:
Subject: [PATCH] credential-cache: report more daemon connection errors
Originally, this code remained relatively silent when we
failed to connect to the cache. The idea was that it was
simply a cache, and we didn't want to bother the user with
temporary failures (the worst case is that we would simply
ask their password again).
However, if you have a configuration failure or other
problem, it is helpful for the daemon to report those
problems. Git will happily ignore the failed error code, but
the extra information to stderr can help the user diagnose
the problem.
This actually has a minor regression, fixed below.
-- >8 --
Subject: [PATCH] credential-cache: ignore "connection refused" errors
The credential-cache helper will try to connect to its
daemon over a unix socket. Originally, a failure to do so
was silently ignored, and we would either give up (if
performing a "get" or "erase" operation), or spawn a new
daemon (for a "store" operation).
But since 8ec6c8d, we try to report more errors. We detect a
missing daemon by checking for ENOENT on our connection
attempt. If the daemon is missing, we continue as before
(giving up or spawning a new daemon). For any other error,
we die and report the problem.
However, checking for ENOENT is not sufficient for a missing
daemon. We might also get ECONNREFUSED if a dead daemon
process left a stale socket. This generally shouldn't
happen, as the daemon cleans up after itself, but the daemon
may not always be given a chance to do so (e.g., power loss,
"kill -9").
The resulting state is annoying not just because the helper
outputs an extra useless message, but because it actually
blocks the helper from spawning a new daemon to replace the
stale socket.
Fix it by checking for ECONNREFUSED.
Signed-off-by: Jeff King <redacted>
---
If we really want to go belt-and-suspenders, the logic should perhaps be
changed to:
if (send_request(socket, &buf < 0) {
/* if we're starting a new one, who cares why it didn't work */
if (flags & FLAG_SPAWN) {
spawn_daemon(socket);
if (send_request(socket, &buf) < 0)
die_errno("unable to connect to spawned daemon");
}
/* otherwise, report any non-minor errors */
else if(errno != ENOENT && errno != ECONNREFUSED)
die_errno("unable to connect to cache daemon");
/* otherwise we are just missing the daemon, and we can ignore */
}
but that implies there is some condition besides ENOENT and ECONNREFUSED
where actually starting a new daemon (which will try to unlink whatever
is there now!) would be a good idea. I'd rather be conservative and
see if anybody reports a real-world case.
credential-cache.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)