credential: sanitize the user prompt

When asking the user interactively for credentials, we want to avoid misleading them e.g. via control sequences that pretend that the URL targets a trusted host when it does not. While Git learned, over the course of the preceding commits, to disallow URLs containing URL-encoded control characters by default, credential helpers are still allowed to specify values very freely (apart from Line Feed and NUL characters, anything is allowed), and this would allow, say, a username containing control characters to be specified that would then be displayed in the interactive terminal prompt asking the user for the password, potentially sending those control characters directly to the terminal. This is undesirable because control characters can be used to mislead users to divulge secret information to untrusted sites. To prevent such an attack vector, let's add a `git_prompt()` that forces the displayed text to be sanitized, i.e. displaying question marks instead of control characters. Note: While this commit's diff changes a lot of `user@host` strings to `user%40host`, which may look suspicious on the surface, there is a good reason for that: this string specifies a user name, not a <username>@<hostname> combination! In the context of t5541, the actual combination looks like this: `user%40@127.0.0.1:5541`. Therefore, these string replacements document a net improvement introduced by this commit, as `user@host@127.0.0.1` could have left readers wondering where the user name ends and where the host name begins. Hinted-at-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Johannes Schindelin committed Oct 30, 2024 at 13:26 UTC 7725b8100ffbbff2750ee4d61a0fcc1f53a086e8
7 files changed +53 -20
Documentation/config/credential.txt
+6
@@ -14,6 +14,12 @@ credential.useHttpPath::
14 or https URL to be important. Defaults to false. See
15 linkgit:gitcredentials[7] for more information.
16
17 +credential.sanitizePrompt::
18 + By default, user names and hosts that are shown as part of the
19 + password prompt are not allowed to contain control characters (they
20 + will be URL-encoded by default). Configure this setting to `false` to
21 + override that behavior.
22 +
23 credential.username::
24 If no username is set for a network authentication, use this username
25 by default. See credential.<context>.* below, and
credential.c
+6 -1
@@ -67,6 +67,8 @@ static int credential_config_callback(const char *var, const char *value,
67 }
68 else if (!strcmp(key, "usehttppath"))
69 c->use_http_path = git_config_bool(var, value);
70 + else if (!strcmp(key, "sanitizeprompt"))
71 + c->sanitize_prompt = git_config_bool(var, value);
72
73 return 0;
74 }
@@ -179,7 +181,10 @@ static char *credential_ask_one(const char *what, struct credential *c,
181 struct strbuf prompt = STRBUF_INIT;
182 char *r;
183
182 - credential_describe(c, &desc);
184 + if (c->sanitize_prompt)
185 + credential_format(c, &desc);
186 + else
187 + credential_describe(c, &desc);
188 if (desc.len)
189 strbuf_addf(&prompt, "%s for '%s': ", what, desc.buf);
190 else
credential.h
+3 -1
@@ -119,7 +119,8 @@ struct credential {
119 configured:1,
120 quit:1,
121 use_http_path:1,
122 - username_from_proto:1;
122 + username_from_proto:1,
123 + sanitize_prompt:1;
124
125 char *username;
126 char *password;
@@ -132,6 +133,7 @@ struct credential {
133 #define CREDENTIAL_INIT { \
134 .helpers = STRING_LIST_INIT_DUP, \
135 .password_expiry_utc = TIME_MAX, \
136 + .sanitize_prompt = 1, \
137 }
138
139 /* Initialize a credential structure, setting all fields to empty. */
t/t0300-credentials.sh
+20
@@ -45,6 +45,10 @@ test_expect_success 'setup helper scripts' '
45 test -z "$pexpiry" || echo password_expiry_utc=$pexpiry
46 EOF
47
48 + write_script git-credential-cntrl-in-username <<-\EOF &&
49 + printf "username=\\007latrix Lestrange\\n"
50 + EOF
51 +
52 PATH="$PWD:$PATH"
53 '
54
@@ -825,4 +829,20 @@ test_expect_success 'credential config with partial URLs' '
829 test_i18ngrep "skipping credential lookup for key" stderr
830 '
831
832 +BEL="$(printf '\007')"
833 +
834 +test_expect_success 'interactive prompt is sanitized' '
835 + check fill cntrl-in-username <<-EOF
836 + protocol=https
837 + host=example.org
838 + --
839 + protocol=https
840 + host=example.org
841 + username=${BEL}latrix Lestrange
842 + password=askpass-password
843 + --
844 + askpass: Password for ${SQ}https://%07latrix%20Lestrange@example.org${SQ}:
845 + EOF
846 +'
847 +
848 test_done
t/t5541-http-push-smart.sh
+3 -3
@@ -351,7 +351,7 @@ test_expect_success 'push over smart http with auth' '
351 git push "$HTTPD_URL"/auth/smart/test_repo.git &&
352 git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
353 log -1 --format=%s >actual &&
354 - expect_askpass both user@host &&
354 + expect_askpass both user%40host &&
355 test_cmp expect actual
356 '
357
@@ -363,7 +363,7 @@ test_expect_success 'push to auth-only-for-push repo' '
363 git push "$HTTPD_URL"/auth-push/smart/test_repo.git &&
364 git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
365 log -1 --format=%s >actual &&
366 - expect_askpass both user@host &&
366 + expect_askpass both user%40host &&
367 test_cmp expect actual
368 '
369
@@ -393,7 +393,7 @@ test_expect_success 'push into half-auth-complete requires password' '
393 git push "$HTTPD_URL/half-auth-complete/smart/half-auth.git" &&
394 git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/half-auth.git" \
395 log -1 --format=%s >actual &&
396 - expect_askpass both user@host &&
396 + expect_askpass both user%40host &&
397 test_cmp expect actual
398 '
399
t/t5550-http-fetch-dumb.sh
+7 -7
@@ -90,13 +90,13 @@ test_expect_success 'http auth can use user/pass in URL' '
90 test_expect_success 'http auth can use just user in URL' '
91 set_askpass wrong pass@host &&
92 git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-pass &&
93 - expect_askpass pass user@host
93 + expect_askpass pass user%40host
94 '
95
96 test_expect_success 'http auth can request both user and pass' '
97 set_askpass user@host pass@host &&
98 git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-both &&
99 - expect_askpass both user@host
99 + expect_askpass both user%40host
100 '
101
102 test_expect_success 'http auth respects credential helper config' '
@@ -114,14 +114,14 @@ test_expect_success 'http auth can get username from config' '
114 test_config_global "credential.$HTTPD_URL.username" user@host &&
115 set_askpass wrong pass@host &&
116 git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-user &&
117 - expect_askpass pass user@host
117 + expect_askpass pass user%40host
118 '
119
120 test_expect_success 'configured username does not override URL' '
121 test_config_global "credential.$HTTPD_URL.username" wrong &&
122 set_askpass wrong pass@host &&
123 git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-user2 &&
124 - expect_askpass pass user@host
124 + expect_askpass pass user%40host
125 '
126
127 test_expect_success 'set up repo with http submodules' '
@@ -142,7 +142,7 @@ test_expect_success 'cmdline credential config passes to submodule via clone' '
142 set_askpass wrong pass@host &&
143 git -c "credential.$HTTPD_URL.username=user@host" \
144 clone --recursive super super-clone &&
145 - expect_askpass pass user@host
145 + expect_askpass pass user%40host
146 '
147
148 test_expect_success 'cmdline credential config passes submodule via fetch' '
@@ -153,7 +153,7 @@ test_expect_success 'cmdline credential config passes submodule via fetch' '
153 git -C super-clone \
154 -c "credential.$HTTPD_URL.username=user@host" \
155 fetch --recurse-submodules &&
156 - expect_askpass pass user@host
156 + expect_askpass pass user%40host
157 '
158
159 test_expect_success 'cmdline credential config passes submodule update' '
@@ -170,7 +170,7 @@ test_expect_success 'cmdline credential config passes submodule update' '
170 git -C super-clone \
171 -c "credential.$HTTPD_URL.username=user@host" \
172 submodule update &&
173 - expect_askpass pass user@host
173 + expect_askpass pass user%40host
174 '
175
176 test_expect_success 'fetch changes via http' '
t/t5551-http-fetch-smart.sh
+8 -8
@@ -181,7 +181,7 @@ test_expect_success 'clone from password-protected repository' '
181 echo two >expect &&
182 set_askpass user@host pass@host &&
183 git clone --bare "$HTTPD_URL/auth/smart/repo.git" smart-auth &&
184 - expect_askpass both user@host &&
184 + expect_askpass both user%40host &&
185 git --git-dir=smart-auth log -1 --format=%s >actual &&
186 test_cmp expect actual
187 '
@@ -199,7 +199,7 @@ test_expect_success 'clone from auth-only-for-objects repository' '
199 echo two >expect &&
200 set_askpass user@host pass@host &&
201 git clone --bare "$HTTPD_URL/auth-fetch/smart/repo.git" half-auth &&
202 - expect_askpass both user@host &&
202 + expect_askpass both user%40host &&
203 git --git-dir=half-auth log -1 --format=%s >actual &&
204 test_cmp expect actual
205 '
@@ -224,14 +224,14 @@ test_expect_success 'redirects send auth to new location' '
224 set_askpass user@host pass@host &&
225 git -c credential.useHttpPath=true \
226 clone $HTTPD_URL/smart-redir-auth/repo.git repo-redir-auth &&
227 - expect_askpass both user@host auth/smart/repo.git
227 + expect_askpass both user%40host auth/smart/repo.git
228 '
229
230 test_expect_success 'GIT_TRACE_CURL redacts auth details' '
231 rm -rf redact-auth trace &&
232 set_askpass user@host pass@host &&
233 GIT_TRACE_CURL="$(pwd)/trace" git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
234 - expect_askpass both user@host &&
234 + expect_askpass both user%40host &&
235
236 # Ensure that there is no "Basic" followed by a base64 string, but that
237 # the auth details are redacted
@@ -243,7 +243,7 @@ test_expect_success 'GIT_CURL_VERBOSE redacts auth details' '
243 rm -rf redact-auth trace &&
244 set_askpass user@host pass@host &&
245 GIT_CURL_VERBOSE=1 git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth 2>trace &&
246 - expect_askpass both user@host &&
246 + expect_askpass both user%40host &&
247
248 # Ensure that there is no "Basic" followed by a base64 string, but that
249 # the auth details are redacted
@@ -256,7 +256,7 @@ test_expect_success 'GIT_TRACE_CURL does not redact auth details if GIT_TRACE_RE
256 set_askpass user@host pass@host &&
257 GIT_TRACE_REDACT=0 GIT_TRACE_CURL="$(pwd)/trace" \
258 git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
259 - expect_askpass both user@host &&
259 + expect_askpass both user%40host &&
260
261 grep -i "Authorization: Basic [0-9a-zA-Z+/]" trace
262 '
@@ -568,7 +568,7 @@ test_expect_success 'http auth remembers successful credentials' '
568 # the first request prompts the user...
569 set_askpass user@host pass@host &&
570 git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
571 - expect_askpass both user@host &&
571 + expect_askpass both user%40host &&
572
573 # ...and the second one uses the stored value rather than
574 # prompting the user.
@@ -599,7 +599,7 @@ test_expect_success 'http auth forgets bogus credentials' '
599 # us to prompt the user again.
600 set_askpass user@host pass@host &&
601 git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
602 - expect_askpass both user@host
602 + expect_askpass both user%40host
603 '
604
605 test_expect_success 'client falls back from v2 to v0 to match server' '