credential: gate new fields on capability

We support the new credential and authtype fields, but we lack a way to
indicate to a credential helper that we'd like them to be used.  Without
some sort of indication, the credential helper doesn't know if it should
try to provide us a username and password, or a pre-encoded credential.
For example, the helper might prefer a more restricted Bearer token if
pre-encoded credentials are possible, but might have to fall back to
more general username and password if not.

Let's provide a simple way to indicate whether Git (or, for that matter,
the helper) is capable of understanding the authtype and credential
fields.  We send this capability when we generate a request, and the
other side may reply to indicate to us that it does, too.

For now, don't enable sending capabilities for the HTTP code.  In a
future commit, we'll introduce appropriate handling for that code,
which requires more in-depth work.

The logic for determining whether a capability is supported may seem
complex, but it is not.  At each stage, we emit the capability to the
following stage if all preceding stages have declared it.  Thus, if the
caller to git credential fill didn't declare it, then we won't send it
to the helper, and if fill's caller did send but the helper doesn't
understand it, then we won't send it on in the response.  If we're an
internal user, then we know about all capabilities and will request
them.

For "git credential approve" and "git credential reject", we set the
helper capability before calling the helper, since we assume that the
input we're getting from the external program comes from a previous call
to "git credential fill", and thus we'll invoke send a capability to the
helper if and only if we got one from the standard input, which is the
correct behavior.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c
index 3a6a750a8eb320bb3622184843ede3d2b9884385..ccbcf99ac1966655bb8216d9ca56721f30e8f9fa 100644 (file)
--- a/builtin/credential-cache--daemon.c
+++ b/builtin/credential-cache--daemon.c
@@ -115,7 +115,7 @@ static int read_request(FILE *fh, struct credential *c,
                return error("client sent bogus timeout line: %s", item.buf);
        *timeout = atoi(p);
 
-       if (credential_read(c, fh) < 0)
+       if (credential_read(c, fh, CREDENTIAL_OP_HELPER) < 0)
                return -1;
        return 0;
 }

diff --git a/builtin/credential-store.c b/builtin/credential-store.c
index 4a492411bbf3d2a7f0a34ada0bd127ae97cf5552..494c8093321e1dac30c019d178096efb26050874 100644 (file)
--- a/builtin/credential-store.c
+++ b/builtin/credential-store.c
@@ -205,7 +205,7 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)
        if (!fns.nr)
                die("unable to set up default path; use --file");
 
-       if (credential_read(&c, stdin) < 0)
+       if (credential_read(&c, stdin, CREDENTIAL_OP_HELPER) < 0)
                die("unable to read credential");
 
        if (!strcmp(op, "get"))

diff --git a/builtin/credential.c b/builtin/credential.c
index 70107529876372c693fa15bbf6aff54ca63b4ce7..643bf0b5e4b691a331405d5fb8a1c2169678d109 100644 (file)
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -17,15 +17,17 @@ int cmd_credential(int argc, const char **argv, const char *prefix UNUSED)
                usage(usage_msg);
        op = argv[1];
 
-       if (credential_read(&c, stdin) < 0)
+       if (credential_read(&c, stdin, CREDENTIAL_OP_INITIAL) < 0)
                die("unable to read credential from stdin");
 
        if (!strcmp(op, "fill")) {
-               credential_fill(&c);
-               credential_write(&c, stdout);
+               credential_fill(&c, 0);
+               credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
        } else if (!strcmp(op, "approve")) {
+               credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
                credential_approve(&c);
        } else if (!strcmp(op, "reject")) {
+               credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);
                credential_reject(&c);
        } else {
                usage(usage_msg);

diff --git a/credential.c b/credential.c
index c521822e5a6eddc786a230d7c49c3c23e55e0882..f5396629df6affb1f639b3dec1e8ce99201511b2 100644 (file)
--- a/credential.c
+++ b/credential.c
@@ -34,6 +34,29 @@ void credential_clear(struct credential *c)
        credential_init(c);
 }
 
+static void credential_set_capability(struct credential_capability *capa,
+                                     enum credential_op_type op_type)
+{
+       switch (op_type) {
+       case CREDENTIAL_OP_INITIAL:
+               capa->request_initial = 1;
+               break;
+       case CREDENTIAL_OP_HELPER:
+               capa->request_helper = 1;
+               break;
+       case CREDENTIAL_OP_RESPONSE:
+               capa->response = 1;
+               break;
+       }
+}
+
+
+void credential_set_all_capabilities(struct credential *c,
+                                    enum credential_op_type op_type)
+{
+       credential_set_capability(&c->capa_authtype, op_type);
+}
+
 int credential_match(const struct credential *want,
                     const struct credential *have, int match_password)
 {
@@ -210,7 +233,26 @@ static void credential_getpass(struct credential *c)
                                                 PROMPT_ASKPASS);
 }
 
-int credential_read(struct credential *c, FILE *fp)
+static int credential_has_capability(const struct credential_capability *capa,
+                                    enum credential_op_type op_type)
+{
+       /*
+        * We're checking here if each previous step indicated that we had the
+        * capability.  If it did, then we want to pass it along; conversely, if
+        * it did not, we don't want to report that to our caller.
+        */
+       switch (op_type) {
+       case CREDENTIAL_OP_HELPER:
+               return capa->request_initial;
+       case CREDENTIAL_OP_RESPONSE:
+               return capa->request_initial && capa->request_helper;
+       default:
+               return 0;
+       }
+}
+
+int credential_read(struct credential *c, FILE *fp,
+                   enum credential_op_type op_type)
 {
        struct strbuf line = STRBUF_INIT;
 
@@ -249,6 +291,8 @@ int credential_read(struct credential *c, FILE *fp)
                        c->path = xstrdup(value);
                } else if (!strcmp(key, "wwwauth[]")) {
                        strvec_push(&c->wwwauth_headers, value);
+               } else if (!strcmp(key, "capability[]") && !strcmp(value, "authtype")) {
+                       credential_set_capability(&c->capa_authtype, op_type);
                } else if (!strcmp(key, "password_expiry_utc")) {
                        errno = 0;
                        c->password_expiry_utc = parse_timestamp(value, NULL, 10);
@@ -288,14 +332,19 @@ static void credential_write_item(FILE *fp, const char *key, const char *value,
        fprintf(fp, "%s=%s\n", key, value);
 }
 
-void credential_write(const struct credential *c, FILE *fp)
+void credential_write(const struct credential *c, FILE *fp,
+                     enum credential_op_type op_type)
 {
+       if (credential_has_capability(&c->capa_authtype, op_type)) {
+               credential_write_item(fp, "capability[]", "authtype", 0);
+               credential_write_item(fp, "authtype", c->authtype, 0);
+               credential_write_item(fp, "credential", c->credential, 0);
+       }
        credential_write_item(fp, "protocol", c->protocol, 1);
        credential_write_item(fp, "host", c->host, 1);
        credential_write_item(fp, "path", c->path, 0);
        credential_write_item(fp, "username", c->username, 0);
        credential_write_item(fp, "password", c->password, 0);
-       credential_write_item(fp, "credential", c->credential, 0);
        credential_write_item(fp, "oauth_refresh_token", c->oauth_refresh_token, 0);
        if (c->password_expiry_utc != TIME_MAX) {
                char *s = xstrfmt("%"PRItime, c->password_expiry_utc);
@@ -304,7 +353,6 @@ void credential_write(const struct credential *c, FILE *fp)
        }
        for (size_t i = 0; i < c->wwwauth_headers.nr; i++)
                credential_write_item(fp, "wwwauth[]", c->wwwauth_headers.v[i], 0);
-       credential_write_item(fp, "authtype", c->authtype, 0);
 }
 
 static int run_credential_helper(struct credential *c,
@@ -327,14 +375,14 @@ static int run_credential_helper(struct credential *c,
 
        fp = xfdopen(helper.in, "w");
        sigchain_push(SIGPIPE, SIG_IGN);
-       credential_write(c, fp);
+       credential_write(c, fp, want_output ? CREDENTIAL_OP_HELPER : CREDENTIAL_OP_RESPONSE);
        fclose(fp);
        sigchain_pop(SIGPIPE);
 
        if (want_output) {
                int r;
                fp = xfdopen(helper.out, "r");
-               r = credential_read(c, fp);
+               r = credential_read(c, fp, CREDENTIAL_OP_HELPER);
                fclose(fp);
                if (r < 0) {
                        finish_command(&helper);
@@ -367,7 +415,7 @@ static int credential_do(struct credential *c, const char *helper,
        return r;
 }
 
-void credential_fill(struct credential *c)
+void credential_fill(struct credential *c, int all_capabilities)
 {
        int i;
 
@@ -375,6 +423,8 @@ void credential_fill(struct credential *c)
                return;
 
        credential_apply_config(c);
+       if (all_capabilities)
+               credential_set_all_capabilities(c, CREDENTIAL_OP_INITIAL);
 
        for (i = 0; i < c->helpers.nr; i++) {
                credential_do(c, c->helpers.items[i].string, "get");

diff --git a/credential.h b/credential.h
index 9db892cf4ddcede05e6d3886f79cfd688f15e5ab..b524fdba598f0b3412f324a93860fcfac7c72d0c 100644 (file)
--- a/credential.h
+++ b/credential.h
@@ -93,6 +93,27 @@
  * -----------------------------------------------------------------------
  */
 
+/*
+ * These values define the kind of operation we're performing and the
+ * capabilities at each stage.  The first is either an external request (via git
+ * credential fill) or an internal request (e.g., via the HTTP) code.  The
+ * second is the call to the credential helper, and the third is the response
+ * we're providing.
+ *
+ * At each stage, we will emit the capability only if the previous stage
+ * supported it.
+ */
+enum credential_op_type {
+       CREDENTIAL_OP_INITIAL  = 1,
+       CREDENTIAL_OP_HELPER   = 2,
+       CREDENTIAL_OP_RESPONSE = 3,
+};
+
+struct credential_capability {
+       unsigned request_initial:1,
+                request_helper:1,
+                response:1;
+};
 
 /**
  * This struct represents a single username/password combination
@@ -136,6 +157,8 @@ struct credential {
                 use_http_path:1,
                 username_from_proto:1;
 
+       struct credential_capability capa_authtype;
+
        char *username;
        char *password;
        char *credential;
@@ -174,8 +197,11 @@ void credential_clear(struct credential *);
  * returns, the username and password fields of the credential are
  * guaranteed to be non-NULL. If an error occurs, the function will
  * die().
+ *
+ * If all_capabilities is set, this is an internal user that is prepared
+ * to deal with all known capabilities, and we should advertise that fact.
  */
-void credential_fill(struct credential *);
+void credential_fill(struct credential *, int all_capabilities);
 
 /**
  * Inform the credential subsystem that the provided credentials
@@ -198,8 +224,16 @@ void credential_approve(struct credential *);
  */
 void credential_reject(struct credential *);
 
-int credential_read(struct credential *, FILE *);
-void credential_write(const struct credential *, FILE *);
+/**
+ * Enable all of the supported credential flags in this credential.
+ */
+void credential_set_all_capabilities(struct credential *c,
+                                    enum credential_op_type op_type);
+
+int credential_read(struct credential *, FILE *,
+                   enum credential_op_type);
+void credential_write(const struct credential *, FILE *,
+                     enum credential_op_type);
 
 /*
  * Parse a url into a credential struct, replacing any existing contents.

diff --git a/http.c b/http.c
index 22639b09742a336ef72e8c9f6405294e95f90bb2..54ddff03fb5fcdd126ed326845127125e403a175 100644 (file)
--- a/http.c
+++ b/http.c
@@ -569,7 +569,7 @@ static void init_curl_http_auth(CURL *result)
                return;
        }
 
-       credential_fill(&http_auth);
+       credential_fill(&http_auth, 0);
 
        curl_easy_setopt(result, CURLOPT_USERNAME, http_auth.username);
        curl_easy_setopt(result, CURLOPT_PASSWORD, http_auth.password);
@@ -596,7 +596,7 @@ static void init_curl_proxy_auth(CURL *result)
 {
        if (proxy_auth.username) {
                if (!proxy_auth.password)
-                       credential_fill(&proxy_auth);
+                       credential_fill(&proxy_auth, 0);
                set_proxyauth_name_password(result);
        }
 
@@ -630,7 +630,7 @@ static int has_cert_password(void)
                cert_auth.host = xstrdup("");
                cert_auth.username = xstrdup("");
                cert_auth.path = xstrdup(ssl_cert);
-               credential_fill(&cert_auth);
+               credential_fill(&cert_auth, 0);
        }
        return 1;
 }
@@ -645,7 +645,7 @@ static int has_proxy_cert_password(void)
                proxy_cert_auth.host = xstrdup("");
                proxy_cert_auth.username = xstrdup("");
                proxy_cert_auth.path = xstrdup(http_proxy_ssl_cert);
-               credential_fill(&proxy_cert_auth);
+               credential_fill(&proxy_cert_auth, 0);
        }
        return 1;
 }
@@ -2190,7 +2190,7 @@ static int http_request_reauth(const char *url,
                BUG("Unknown http_request target");
        }
 
-       credential_fill(&http_auth);
+       credential_fill(&http_auth, 0);
 
        return http_request(url, result, target, options);
 }

diff --git a/imap-send.c b/imap-send.c
index f2e1947e63815c80e45f648f0ce37cd6341f825b..8c89e866b6c91176e0a8cf304eb2b6ab696eb9cc 100644 (file)
--- a/imap-send.c
+++ b/imap-send.c
@@ -944,7 +944,7 @@ static void server_fill_credential(struct imap_server_conf *srvc, struct credent
        cred->username = xstrdup_or_null(srvc->user);
        cred->password = xstrdup_or_null(srvc->pass);
 
-       credential_fill(cred);
+       credential_fill(cred, 1);
 
        if (!srvc->user)
                srvc->user = xstrdup(cred->username);

diff --git a/remote-curl.c b/remote-curl.c
index e37eaa17b70e76159e8f5566b9fcb2653f153307..f96bda2431a8b066e8ba1eb47980bc10b5dca9d0 100644 (file)
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -926,7 +926,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
                do {
                        err = probe_rpc(rpc, &results);
                        if (err == HTTP_REAUTH)
-                               credential_fill(&http_auth);
+                               credential_fill(&http_auth, 0);
                } while (err == HTTP_REAUTH);
                if (err != HTTP_OK)
                        return -1;
@@ -1044,7 +1044,7 @@ retry:
        rpc->any_written = 0;
        err = run_slot(slot, NULL);
        if (err == HTTP_REAUTH && !large_request) {
-               credential_fill(&http_auth);
+               credential_fill(&http_auth, 0);
                curl_slist_free_all(headers);
                goto retry;
        }

diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 400f6bdbca13c2a93cb8521e21e08fbd4e1eb3bd..daf330ddd817db2dc2bdcf03a35b9a02f2a8bd2a 100755 (executable)
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -12,7 +12,13 @@ test_expect_success 'setup helper scripts' '
        IFS==
        while read key value; do
                echo >&2 "$whoami: $key=$value"
-               eval "$key=$value"
+               if test -z "${key%%*\[\]}"
+               then
+                       key=${key%%\[\]}
+                       eval "$key=\"\$$key $value\""
+               else
+                       eval "$key=$value"
+               fi
        done
        IFS=$OIFS
        EOF
@@ -35,6 +41,16 @@ test_expect_success 'setup helper scripts' '
        test -z "$pass" || echo password=$pass
        EOF
 
+       write_script git-credential-verbatim-cred <<-\EOF &&
+       authtype=$1; shift
+       credential=$1; shift
+       . ./dump
+       echo capability[]=authtype
+       test -z "${capability##*authtype*}" || exit 0
+       test -z "$authtype" || echo authtype=$authtype
+       test -z "$credential" || echo credential=$credential
+       EOF
+
        write_script git-credential-verbatim-with-expiry <<-\EOF &&
        user=$1; shift
        pass=$1; shift
@@ -64,6 +80,26 @@ test_expect_success 'credential_fill invokes helper' '
        EOF
 '
 
+test_expect_success 'credential_fill invokes helper with credential' '
+       check fill "verbatim-cred Bearer token" <<-\EOF
+       capability[]=authtype
+       protocol=http
+       host=example.com
+       --
+       capability[]=authtype
+       authtype=Bearer
+       credential=token
+       protocol=http
+       host=example.com
+       --
+       verbatim-cred: get
+       verbatim-cred: capability[]=authtype
+       verbatim-cred: protocol=http
+       verbatim-cred: host=example.com
+       EOF
+'
+
+
 test_expect_success 'credential_fill invokes multiple helpers' '
        check fill useless "verbatim foo bar" <<-\EOF
        protocol=http
@@ -83,6 +119,42 @@ test_expect_success 'credential_fill invokes multiple helpers' '
        EOF
 '
 
+test_expect_success 'credential_fill response does not get capabilities when helpers are incapable' '
+       check fill useless "verbatim foo bar" <<-\EOF
+       capability[]=authtype
+       protocol=http
+       host=example.com
+       --
+       protocol=http
+       host=example.com
+       username=foo
+       password=bar
+       --
+       useless: get
+       useless: capability[]=authtype
+       useless: protocol=http
+       useless: host=example.com
+       verbatim: get
+       verbatim: capability[]=authtype
+       verbatim: protocol=http
+       verbatim: host=example.com
+       EOF
+'
+
+test_expect_success 'credential_fill response does not get capabilities when caller is incapable' '
+       check fill "verbatim-cred Bearer token" <<-\EOF
+       protocol=http
+       host=example.com
+       --
+       protocol=http
+       host=example.com
+       --
+       verbatim-cred: get
+       verbatim-cred: protocol=http
+       verbatim-cred: host=example.com
+       EOF
+'
+
 test_expect_success 'credential_fill stops when we get a full response' '
        check fill "verbatim one two" "verbatim three four" <<-\EOF
        protocol=http
@@ -99,6 +171,25 @@ test_expect_success 'credential_fill stops when we get a full response' '
        EOF
 '
 
+test_expect_success 'credential_fill thinks a credential is a full response' '
+       check fill "verbatim-cred Bearer token" "verbatim three four" <<-\EOF
+       capability[]=authtype
+       protocol=http
+       host=example.com
+       --
+       capability[]=authtype
+       authtype=Bearer
+       credential=token
+       protocol=http
+       host=example.com
+       --
+       verbatim-cred: get
+       verbatim-cred: capability[]=authtype
+       verbatim-cred: protocol=http
+       verbatim-cred: host=example.com
+       EOF
+'
+
 test_expect_success 'credential_fill continues through partial response' '
        check fill "verbatim one \"\"" "verbatim two three" <<-\EOF
        protocol=http
@@ -175,6 +266,20 @@ test_expect_success 'credential_fill passes along metadata' '
        EOF
 '
 
+test_expect_success 'credential_fill produces no credential without capability' '
+       check fill "verbatim-cred Bearer token" <<-\EOF
+       protocol=http
+       host=example.com
+       --
+       protocol=http
+       host=example.com
+       --
+       verbatim-cred: get
+       verbatim-cred: protocol=http
+       verbatim-cred: host=example.com
+       EOF
+'
+
 test_expect_success 'credential_approve calls all helpers' '
        check approve useless "verbatim one two" <<-\EOF
        protocol=http