Skip to content

Commit ac4c7cb

Browse files
bk2204gitster
authored andcommitted
credential: add support for multistage credential rounds
Over HTTP, NTLM and Kerberos require two rounds of authentication on the client side. It's possible that there are custom authentication schemes that also implement this same approach. Since these are tricky schemes to implement and the HTTP library in use may not always handle them gracefully on all systems, it would be helpful to allow the credential helper to implement them instead for increased portability and robustness. To allow this to happen, add a boolean flag, continue, that indicates that instead of failing when we get a 401, we should retry another round of authentication. However, this necessitates some changes in our current credential code so that we can make this work. Keep the state[] headers between iterations, but only use them to send to the helper and only consider the new ones we read from the credential helper to be valid on subsequent iterations. That avoids us passing stale data when we finally approve or reject the credential. Similarly, clear the multistage and wwwauth[] values appropriately so that we don't pass stale data or think we're trying a multiround response when we're not. Remove the credential values so that we can actually fill a second time with new responses. Limit the number of iterations of reauthentication we do to 3. This means that if there's a problem, we'll terminate with an error message instead of retrying indefinitely and not informing the user (and possibly conducting a DoS on the server). In our tests, handle creating multiple response output files from our helper so we can verify that each of the messages sent is correct. Signed-off-by: brian m. carlson <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 37417b7 commit ac4c7cb

File tree

6 files changed

+184
-35
lines changed

6 files changed

+184
-35
lines changed

Documentation/git-credential.txt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,19 @@ provided on input.
222222
This value should not be sent unless the appropriate capability (see below) is
223223
provided on input.
224224

225+
`continue`::
226+
This is a boolean value, which, if enabled, indicates that this
227+
authentication is a non-final part of a multistage authentication step. This
228+
is common in protocols such as NTLM and Kerberos, where two rounds of client
229+
authentication are required, and setting this flag allows the credential
230+
helper to implement the multistage authentication step. This flag should
231+
only be sent if a further stage is required; that is, if another round of
232+
authentication is expected.
233+
+
234+
This value should not be sent unless the appropriate capability (see below) is
235+
provided on input. This attribute is 'one-way' from a credential helper to
236+
pass information to Git (or other programs invoking `git credential`).
237+
225238
`wwwauth[]`::
226239

227240
When an HTTP response is received by Git that includes one or more

builtin/credential.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ int cmd_credential(int argc, const char **argv, const char *prefix UNUSED)
2222

2323
if (!strcmp(op, "fill")) {
2424
credential_fill(&c, 0);
25+
credential_next_state(&c);
2526
credential_write(&c, stdout, CREDENTIAL_OP_RESPONSE);
2627
} else if (!strcmp(op, "approve")) {
2728
credential_set_all_capabilities(&c, CREDENTIAL_OP_HELPER);

credential.c

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,23 @@ void credential_clear(struct credential *c)
3131
string_list_clear(&c->helpers, 0);
3232
strvec_clear(&c->wwwauth_headers);
3333
strvec_clear(&c->state_headers);
34+
strvec_clear(&c->state_headers_to_send);
3435

3536
credential_init(c);
3637
}
3738

39+
void credential_next_state(struct credential *c)
40+
{
41+
strvec_clear(&c->state_headers_to_send);
42+
SWAP(c->state_headers, c->state_headers_to_send);
43+
}
44+
45+
void credential_clear_secrets(struct credential *c)
46+
{
47+
FREE_AND_NULL(c->password);
48+
FREE_AND_NULL(c->credential);
49+
}
50+
3851
static void credential_set_capability(struct credential_capability *capa,
3952
enum credential_op_type op_type)
4053
{
@@ -302,6 +315,8 @@ int credential_read(struct credential *c, FILE *fp,
302315
credential_set_capability(&c->capa_authtype, op_type);
303316
else if (!strcmp(value, "state"))
304317
credential_set_capability(&c->capa_state, op_type);
318+
} else if (!strcmp(key, "continue")) {
319+
c->multistage = !!git_config_bool("continue", value);
305320
} else if (!strcmp(key, "password_expiry_utc")) {
306321
errno = 0;
307322
c->password_expiry_utc = parse_timestamp(value, NULL, 10);
@@ -369,8 +384,10 @@ void credential_write(const struct credential *c, FILE *fp,
369384
for (size_t i = 0; i < c->wwwauth_headers.nr; i++)
370385
credential_write_item(fp, "wwwauth[]", c->wwwauth_headers.v[i], 0);
371386
if (credential_has_capability(&c->capa_state, op_type)) {
372-
for (size_t i = 0; i < c->state_headers.nr; i++)
373-
credential_write_item(fp, "state[]", c->state_headers.v[i], 0);
387+
if (c->multistage)
388+
credential_write_item(fp, "continue", "1", 0);
389+
for (size_t i = 0; i < c->state_headers_to_send.nr; i++)
390+
credential_write_item(fp, "state[]", c->state_headers_to_send.v[i], 0);
374391
}
375392
}
376393

@@ -441,6 +458,9 @@ void credential_fill(struct credential *c, int all_capabilities)
441458
if ((c->username && c->password) || c->credential)
442459
return;
443460

461+
credential_next_state(c);
462+
c->multistage = 0;
463+
444464
credential_apply_config(c);
445465
if (all_capabilities)
446466
credential_set_all_capabilities(c, CREDENTIAL_OP_INITIAL);
@@ -453,8 +473,10 @@ void credential_fill(struct credential *c, int all_capabilities)
453473
/* Reset expiry to maintain consistency */
454474
c->password_expiry_utc = TIME_MAX;
455475
}
456-
if ((c->username && c->password) || c->credential)
476+
if ((c->username && c->password) || c->credential) {
477+
strvec_clear(&c->wwwauth_headers);
457478
return;
479+
}
458480
if (c->quit)
459481
die("credential helper '%s' told us to quit",
460482
c->helpers.items[i].string);
@@ -474,6 +496,8 @@ void credential_approve(struct credential *c)
474496
if (((!c->username || !c->password) && !c->credential) || c->password_expiry_utc < time(NULL))
475497
return;
476498

499+
credential_next_state(c);
500+
477501
credential_apply_config(c);
478502

479503
for (i = 0; i < c->helpers.nr; i++)
@@ -485,6 +509,8 @@ void credential_reject(struct credential *c)
485509
{
486510
int i;
487511

512+
credential_next_state(c);
513+
488514
credential_apply_config(c);
489515

490516
for (i = 0; i < c->helpers.nr; i++)

credential.h

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,15 @@ struct credential {
145145
struct strvec wwwauth_headers;
146146

147147
/**
148-
* A `strvec` of state headers from credential helpers.
148+
* A `strvec` of state headers received from credential helpers.
149149
*/
150150
struct strvec state_headers;
151151

152+
/**
153+
* A `strvec` of state headers to send to credential helpers.
154+
*/
155+
struct strvec state_headers_to_send;
156+
152157
/**
153158
* Internal use only. Keeps track of if we previously matched against a
154159
* WWW-Authenticate header line in order to re-fold future continuation
@@ -159,6 +164,7 @@ struct credential {
159164
unsigned approved:1,
160165
ephemeral:1,
161166
configured:1,
167+
multistage: 1,
162168
quit:1,
163169
use_http_path:1,
164170
username_from_proto:1;
@@ -187,6 +193,7 @@ struct credential {
187193
.password_expiry_utc = TIME_MAX, \
188194
.wwwauth_headers = STRVEC_INIT, \
189195
.state_headers = STRVEC_INIT, \
196+
.state_headers_to_send = STRVEC_INIT, \
190197
}
191198

192199
/* Initialize a credential structure, setting all fields to empty. */
@@ -238,6 +245,24 @@ void credential_reject(struct credential *);
238245
void credential_set_all_capabilities(struct credential *c,
239246
enum credential_op_type op_type);
240247

248+
/**
249+
* Clear the secrets in this credential, but leave other data intact.
250+
*
251+
* This is useful for resetting credentials in preparation for a subsequent
252+
* stage of filling.
253+
*/
254+
void credential_clear_secrets(struct credential *c);
255+
256+
/**
257+
* Prepares the credential for the next iteration of the helper protocol by
258+
* updating the state headers to send with the ones read by the last iteration
259+
* of the protocol.
260+
*
261+
* Except for internal callers, this should be called exactly once between
262+
* reading credentials with `credential_fill` and writing them.
263+
*/
264+
void credential_next_state(struct credential *c);
265+
241266
int credential_read(struct credential *, FILE *,
242267
enum credential_op_type);
243268
void credential_write(const struct credential *, FILE *,

http.c

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1780,6 +1780,10 @@ static int handle_curl_result(struct slot_results *results)
17801780
else if (results->http_code == 401) {
17811781
if ((http_auth.username && http_auth.password) ||\
17821782
(http_auth.authtype && http_auth.credential)) {
1783+
if (http_auth.multistage) {
1784+
credential_clear_secrets(&http_auth);
1785+
return HTTP_REAUTH;
1786+
}
17831787
credential_reject(&http_auth);
17841788
return HTTP_NOAUTH;
17851789
} else {
@@ -2177,6 +2181,7 @@ static int http_request_reauth(const char *url,
21772181
void *result, int target,
21782182
struct http_get_options *options)
21792183
{
2184+
int i = 3;
21802185
int ret = http_request(url, result, target, options);
21812186

21822187
if (ret != HTTP_OK && ret != HTTP_REAUTH)
@@ -2190,35 +2195,35 @@ static int http_request_reauth(const char *url,
21902195
}
21912196
}
21922197

2193-
if (ret != HTTP_REAUTH)
2194-
return ret;
2195-
2196-
/*
2197-
* The previous request may have put cruft into our output stream; we
2198-
* should clear it out before making our next request.
2199-
*/
2200-
switch (target) {
2201-
case HTTP_REQUEST_STRBUF:
2202-
strbuf_reset(result);
2203-
break;
2204-
case HTTP_REQUEST_FILE:
2205-
if (fflush(result)) {
2206-
error_errno("unable to flush a file");
2207-
return HTTP_START_FAILED;
2208-
}
2209-
rewind(result);
2210-
if (ftruncate(fileno(result), 0) < 0) {
2211-
error_errno("unable to truncate a file");
2212-
return HTTP_START_FAILED;
2198+
while (ret == HTTP_REAUTH && --i) {
2199+
/*
2200+
* The previous request may have put cruft into our output stream; we
2201+
* should clear it out before making our next request.
2202+
*/
2203+
switch (target) {
2204+
case HTTP_REQUEST_STRBUF:
2205+
strbuf_reset(result);
2206+
break;
2207+
case HTTP_REQUEST_FILE:
2208+
if (fflush(result)) {
2209+
error_errno("unable to flush a file");
2210+
return HTTP_START_FAILED;
2211+
}
2212+
rewind(result);
2213+
if (ftruncate(fileno(result), 0) < 0) {
2214+
error_errno("unable to truncate a file");
2215+
return HTTP_START_FAILED;
2216+
}
2217+
break;
2218+
default:
2219+
BUG("Unknown http_request target");
22132220
}
2214-
break;
2215-
default:
2216-
BUG("Unknown http_request target");
2217-
}
22182221

2219-
credential_fill(&http_auth, 1);
2222+
credential_fill(&http_auth, 1);
22202223

2221-
return http_request(url, result, target, options);
2224+
ret = http_request(url, result, target, options);
2225+
}
2226+
return ret;
22222227
}
22232228

22242229
int http_get_strbuf(const char *url,

t/t5563-simple-http-auth.sh

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,17 @@ test_expect_success 'setup_credential_helper' '
2121
CREDENTIAL_HELPER="$TRASH_DIRECTORY/bin/git-credential-test-helper" &&
2222
write_script "$CREDENTIAL_HELPER" <<-\EOF
2323
cmd=$1
24-
teefile=$cmd-query.cred
24+
teefile=$cmd-query-temp.cred
2525
catfile=$cmd-reply.cred
2626
sed -n -e "/^$/q" -e "p" >>$teefile
27+
state=$(sed -ne "s/^state\[\]=helper://p" "$teefile")
28+
if test -z "$state"
29+
then
30+
mv "$teefile" "$cmd-query.cred"
31+
else
32+
mv "$teefile" "$cmd-query-$state.cred"
33+
catfile="$cmd-reply-$state.cred"
34+
fi
2735
if test "$cmd" = "get"
2836
then
2937
cat $catfile
@@ -32,13 +40,15 @@ test_expect_success 'setup_credential_helper' '
3240
'
3341

3442
set_credential_reply () {
35-
cat >"$TRASH_DIRECTORY/$1-reply.cred"
43+
local suffix="$(test -n "$2" && echo "-$2")"
44+
cat >"$TRASH_DIRECTORY/$1-reply$suffix.cred"
3645
}
3746

3847
expect_credential_query () {
39-
cat >"$TRASH_DIRECTORY/$1-expect.cred" &&
40-
test_cmp "$TRASH_DIRECTORY/$1-expect.cred" \
41-
"$TRASH_DIRECTORY/$1-query.cred"
48+
local suffix="$(test -n "$2" && echo "-$2")"
49+
cat >"$TRASH_DIRECTORY/$1-expect$suffix.cred" &&
50+
test_cmp "$TRASH_DIRECTORY/$1-expect$suffix.cred" \
51+
"$TRASH_DIRECTORY/$1-query$suffix.cred"
4252
}
4353

4454
per_test_cleanup () {
@@ -479,4 +489,73 @@ test_expect_success 'access using bearer auth with invalid credentials' '
479489
EOF
480490
'
481491

492+
test_expect_success 'access using three-legged auth' '
493+
test_when_finished "per_test_cleanup" &&
494+
495+
set_credential_reply get <<-EOF &&
496+
capability[]=authtype
497+
capability[]=state
498+
authtype=Multistage
499+
credential=YS1naXQtdG9rZW4=
500+
state[]=helper:foobar
501+
continue=1
502+
EOF
503+
504+
set_credential_reply get foobar <<-EOF &&
505+
capability[]=authtype
506+
capability[]=state
507+
authtype=Multistage
508+
credential=YW5vdGhlci10b2tlbg==
509+
state[]=helper:bazquux
510+
EOF
511+
512+
cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
513+
id=1 creds=Multistage YS1naXQtdG9rZW4=
514+
id=2 creds=Multistage YW5vdGhlci10b2tlbg==
515+
EOF
516+
517+
CHALLENGE="$HTTPD_ROOT_PATH/custom-auth.challenge" &&
518+
519+
cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
520+
id=1 status=401 response=WWW-Authenticate: Multistage challenge="456"
521+
id=1 status=401 response=WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0
522+
id=2 status=200
523+
id=default response=WWW-Authenticate: Multistage challenge="123"
524+
id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0
525+
EOF
526+
527+
test_config_global credential.helper test-helper &&
528+
git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
529+
530+
expect_credential_query get <<-EOF &&
531+
capability[]=authtype
532+
capability[]=state
533+
protocol=http
534+
host=$HTTPD_DEST
535+
wwwauth[]=Multistage challenge="123"
536+
wwwauth[]=Bearer authorize_uri="id.example.com" p=1 q=0
537+
EOF
538+
539+
expect_credential_query get foobar <<-EOF &&
540+
capability[]=authtype
541+
capability[]=state
542+
authtype=Multistage
543+
protocol=http
544+
host=$HTTPD_DEST
545+
wwwauth[]=Multistage challenge="456"
546+
wwwauth[]=Bearer authorize_uri="id.example.com" p=1 q=0
547+
state[]=helper:foobar
548+
EOF
549+
550+
expect_credential_query store bazquux <<-EOF
551+
capability[]=authtype
552+
capability[]=state
553+
authtype=Multistage
554+
credential=YW5vdGhlci10b2tlbg==
555+
protocol=http
556+
host=$HTTPD_DEST
557+
state[]=helper:bazquux
558+
EOF
559+
'
560+
482561
test_done

0 commit comments

Comments
 (0)