diff --git a/Documentation/config/credential.txt b/Documentation/config/credential.txt index 512f31876e..9cadca7f73 100644 --- a/Documentation/config/credential.txt +++ b/Documentation/config/credential.txt @@ -14,6 +14,17 @@ credential.useHttpPath:: or https URL to be important. Defaults to false. See linkgit:gitcredentials[7] for more information. +credential.sanitizePrompt:: + By default, user names and hosts that are shown as part of the + password prompt are not allowed to contain control characters (they + will be URL-encoded by default). Configure this setting to `false` to + override that behavior. + +credential.protectProtocol:: + By default, Carriage Return characters are not allowed in the protocol + that is used when Git talks to a credential helper. This setting allows + users to override this default. + credential.username:: If no username is set for a network authentication, use this username by default. See credential..* below, and diff --git a/credential.c b/credential.c index f32011343f..b76a730901 100644 --- a/credential.c +++ b/credential.c @@ -67,6 +67,10 @@ static int credential_config_callback(const char *var, const char *value, } else if (!strcmp(key, "usehttppath")) c->use_http_path = git_config_bool(var, value); + else if (!strcmp(key, "sanitizeprompt")) + c->sanitize_prompt = git_config_bool(var, value); + else if (!strcmp(key, "protectprotocol")) + c->protect_protocol = git_config_bool(var, value); return 0; } @@ -164,7 +168,8 @@ static void credential_format(struct credential *c, struct strbuf *out) strbuf_addch(out, '@'); } if (c->host) - strbuf_addstr(out, c->host); + strbuf_add_percentencode(out, c->host, + STRBUF_ENCODE_HOST_AND_PORT); if (c->path) { strbuf_addch(out, '/'); strbuf_add_percentencode(out, c->path, 0); @@ -178,7 +183,10 @@ static char *credential_ask_one(const char *what, struct credential *c, struct strbuf prompt = STRBUF_INIT; char *r; - credential_describe(c, &desc); + if (c->sanitize_prompt) + credential_format(c, &desc); + else + credential_describe(c, &desc); if (desc.len) strbuf_addf(&prompt, "%s for '%s': ", what, desc.buf); else @@ -256,7 +264,8 @@ int credential_read(struct credential *c, FILE *fp) return 0; } -static void credential_write_item(FILE *fp, const char *key, const char *value, +static void credential_write_item(const struct credential *c, + FILE *fp, const char *key, const char *value, int required) { if (!value && required) @@ -265,19 +274,23 @@ static void credential_write_item(FILE *fp, const char *key, const char *value, return; if (strchr(value, '\n')) die("credential value for %s contains newline", key); + if (c->protect_protocol && strchr(value, '\r')) + die("credential value for %s contains carriage return\n" + "If this is intended, set `credential.protectProtocol=false`", + key); fprintf(fp, "%s=%s\n", key, value); } void credential_write(const struct credential *c, FILE *fp) { - 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(c, fp, "protocol", c->protocol, 1); + credential_write_item(c, fp, "host", c->host, 1); + credential_write_item(c, fp, "path", c->path, 0); + credential_write_item(c, fp, "username", c->username, 0); + credential_write_item(c, fp, "password", c->password, 0); if (c->password_expiry_utc != TIME_MAX) { char *s = xstrfmt("%"PRItime, c->password_expiry_utc); - credential_write_item(fp, "password_expiry_utc", s, 0); + credential_write_item(c, fp, "password_expiry_utc", s, 0); free(s); } } diff --git a/credential.h b/credential.h index 935b28a70f..2c0b39a925 100644 --- a/credential.h +++ b/credential.h @@ -119,7 +119,9 @@ struct credential { configured:1, quit:1, use_http_path:1, - username_from_proto:1; + username_from_proto:1, + sanitize_prompt:1, + protect_protocol:1; char *username; char *password; @@ -132,6 +134,8 @@ struct credential { #define CREDENTIAL_INIT { \ .helpers = STRING_LIST_INIT_DUP, \ .password_expiry_utc = TIME_MAX, \ + .sanitize_prompt = 1, \ + .protect_protocol = 1, \ } /* Initialize a credential structure, setting all fields to empty. */ diff --git a/strbuf.c b/strbuf.c index c383f41a3c..756b96c561 100644 --- a/strbuf.c +++ b/strbuf.c @@ -492,7 +492,9 @@ void strbuf_add_percentencode(struct strbuf *dst, const char *src, int flags) unsigned char ch = src[i]; if (ch <= 0x1F || ch >= 0x7F || (ch == '/' && (flags & STRBUF_ENCODE_SLASH)) || - strchr(URL_UNSAFE_CHARS, ch)) + ((flags & STRBUF_ENCODE_HOST_AND_PORT) ? + !isalnum(ch) && !strchr("-.:[]", ch) : + !!strchr(URL_UNSAFE_CHARS, ch))) strbuf_addf(dst, "%%%02X", (unsigned char)ch); else strbuf_addch(dst, ch); diff --git a/strbuf.h b/strbuf.h index f6dbb9681e..f9f8bb0381 100644 --- a/strbuf.h +++ b/strbuf.h @@ -380,6 +380,7 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb, void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src); #define STRBUF_ENCODE_SLASH 1 +#define STRBUF_ENCODE_HOST_AND_PORT 2 /** * Append the contents of a string to a strbuf, percent-encoding any characters diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index c66d91e82d..168ae76550 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -45,6 +45,10 @@ test_expect_success 'setup helper scripts' ' test -z "$pexpiry" || echo password_expiry_utc=$pexpiry EOF + write_script git-credential-cntrl-in-username <<-\EOF && + printf "username=\\007latrix Lestrange\\n" + EOF + PATH="$PWD:$PATH" ' @@ -514,6 +518,19 @@ test_expect_success 'match percent-encoded values in username' ' EOF ' +test_expect_success 'match percent-encoded values in hostname' ' + test_config "credential.https://a%20b%20c/.helper" "$HELPER" && + check fill <<-\EOF + url=https://a b c/ + -- + protocol=https + host=a b c + username=foo + password=bar + -- + EOF +' + test_expect_success 'fetch with multiple path components' ' test_unconfig credential.helper && test_config credential.https://example.com/foo/repo.git.helper "verbatim foo bar" && @@ -703,6 +720,22 @@ test_expect_success 'url parser rejects embedded newlines' ' test_cmp expect stderr ' +test_expect_success 'url parser rejects embedded carriage returns' ' + test_config credential.helper "!true" && + test_must_fail git credential fill 2>stderr <<-\EOF && + url=https://example%0d.com/ + EOF + cat >expect <<-\EOF && + fatal: credential value for host contains carriage return + If this is intended, set `credential.protectProtocol=false` + EOF + test_cmp expect stderr && + GIT_ASKPASS=true \ + git -c credential.protectProtocol=false credential fill <<-\EOF + url=https://example%0d.com/ + EOF +' + test_expect_success 'host-less URLs are parsed as empty host' ' check fill "verbatim foo bar" <<-\EOF url=cert:///path/to/cert.pem @@ -812,4 +845,20 @@ test_expect_success 'credential config with partial URLs' ' test_i18ngrep "skipping credential lookup for key" stderr ' +BEL="$(printf '\007')" + +test_expect_success 'interactive prompt is sanitized' ' + check fill cntrl-in-username <<-EOF + protocol=https + host=example.org + -- + protocol=https + host=example.org + username=${BEL}latrix Lestrange + password=askpass-password + -- + askpass: Password for ${SQ}https://%07latrix%20Lestrange@example.org${SQ}: + EOF +' + test_done diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh index d0211cd8be..2cd2e1a059 100755 --- a/t/t5541-http-push-smart.sh +++ b/t/t5541-http-push-smart.sh @@ -351,7 +351,7 @@ test_expect_success 'push over smart http with auth' ' git push "$HTTPD_URL"/auth/smart/test_repo.git && git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \ log -1 --format=%s >actual && - expect_askpass both user@host && + expect_askpass both user%40host && test_cmp expect actual ' @@ -363,7 +363,7 @@ test_expect_success 'push to auth-only-for-push repo' ' git push "$HTTPD_URL"/auth-push/smart/test_repo.git && git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \ log -1 --format=%s >actual && - expect_askpass both user@host && + expect_askpass both user%40host && test_cmp expect actual ' @@ -393,7 +393,7 @@ test_expect_success 'push into half-auth-complete requires password' ' git push "$HTTPD_URL/half-auth-complete/smart/half-auth.git" && git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/half-auth.git" \ log -1 --format=%s >actual && - expect_askpass both user@host && + expect_askpass both user%40host && test_cmp expect actual ' diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 8f182a3cbf..5d0e394609 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -90,13 +90,13 @@ test_expect_success 'http auth can use user/pass in URL' ' test_expect_success 'http auth can use just user in URL' ' set_askpass wrong pass@host && git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-pass && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'http auth can request both user and pass' ' set_askpass user@host pass@host && git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-both && - expect_askpass both user@host + expect_askpass both user%40host ' test_expect_success 'http auth respects credential helper config' ' @@ -114,14 +114,14 @@ test_expect_success 'http auth can get username from config' ' test_config_global "credential.$HTTPD_URL.username" user@host && set_askpass wrong pass@host && git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-user && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'configured username does not override URL' ' test_config_global "credential.$HTTPD_URL.username" wrong && set_askpass wrong pass@host && git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-user2 && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'set up repo with http submodules' ' @@ -142,7 +142,7 @@ test_expect_success 'cmdline credential config passes to submodule via clone' ' set_askpass wrong pass@host && git -c "credential.$HTTPD_URL.username=user@host" \ clone --recursive super super-clone && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'cmdline credential config passes submodule via fetch' ' @@ -153,7 +153,7 @@ test_expect_success 'cmdline credential config passes submodule via fetch' ' git -C super-clone \ -c "credential.$HTTPD_URL.username=user@host" \ fetch --recurse-submodules && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'cmdline credential config passes submodule update' ' @@ -170,7 +170,7 @@ test_expect_success 'cmdline credential config passes submodule update' ' git -C super-clone \ -c "credential.$HTTPD_URL.username=user@host" \ submodule update && - expect_askpass pass user@host + expect_askpass pass user%40host ' test_expect_success 'fetch changes via http' ' diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index 0908534f25..8a27768dfb 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -181,7 +181,7 @@ test_expect_success 'clone from password-protected repository' ' echo two >expect && set_askpass user@host pass@host && git clone --bare "$HTTPD_URL/auth/smart/repo.git" smart-auth && - expect_askpass both user@host && + expect_askpass both user%40host && git --git-dir=smart-auth log -1 --format=%s >actual && test_cmp expect actual ' @@ -199,7 +199,7 @@ test_expect_success 'clone from auth-only-for-objects repository' ' echo two >expect && set_askpass user@host pass@host && git clone --bare "$HTTPD_URL/auth-fetch/smart/repo.git" half-auth && - expect_askpass both user@host && + expect_askpass both user%40host && git --git-dir=half-auth log -1 --format=%s >actual && test_cmp expect actual ' @@ -224,14 +224,14 @@ test_expect_success 'redirects send auth to new location' ' set_askpass user@host pass@host && git -c credential.useHttpPath=true \ clone $HTTPD_URL/smart-redir-auth/repo.git repo-redir-auth && - expect_askpass both user@host auth/smart/repo.git + expect_askpass both user%40host auth/smart/repo.git ' test_expect_success 'GIT_TRACE_CURL redacts auth details' ' rm -rf redact-auth trace && set_askpass user@host pass@host && GIT_TRACE_CURL="$(pwd)/trace" git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth && - expect_askpass both user@host && + expect_askpass both user%40host && # Ensure that there is no "Basic" followed by a base64 string, but that # the auth details are redacted @@ -243,7 +243,7 @@ test_expect_success 'GIT_CURL_VERBOSE redacts auth details' ' rm -rf redact-auth trace && set_askpass user@host pass@host && GIT_CURL_VERBOSE=1 git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth 2>trace && - expect_askpass both user@host && + expect_askpass both user%40host && # Ensure that there is no "Basic" followed by a base64 string, but that # the auth details are redacted @@ -256,7 +256,7 @@ test_expect_success 'GIT_TRACE_CURL does not redact auth details if GIT_TRACE_RE set_askpass user@host pass@host && GIT_TRACE_REDACT=0 GIT_TRACE_CURL="$(pwd)/trace" \ git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth && - expect_askpass both user@host && + expect_askpass both user%40host && grep -i "Authorization: Basic [0-9a-zA-Z+/]" trace ' @@ -568,7 +568,7 @@ test_expect_success 'http auth remembers successful credentials' ' # the first request prompts the user... set_askpass user@host pass@host && git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null && - expect_askpass both user@host && + expect_askpass both user%40host && # ...and the second one uses the stored value rather than # prompting the user. @@ -599,7 +599,7 @@ test_expect_success 'http auth forgets bogus credentials' ' # us to prompt the user again. set_askpass user@host pass@host && git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null && - expect_askpass both user@host + expect_askpass both user%40host ' test_expect_success 'client falls back from v2 to v0 to match server' '