@matheusbsbs / matheus-bagliano / commits / 827e7d4da4
Fork 0

http: redact all cookies, teach GIT_TRACE_REDACT=0

In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jun 5, 2020 at 14:21 UTC 827e7d4da470e8b9b222b2cf3b4a3b7f8c3c671f
3 files changed +39 -43
Documentation/git.txt
+4 -5
@@ -775,11 +775,10 @@ for full details.
775 See `GIT_TRACE2` for available trace output options and
776 link:technical/api-trace2.html[Trace2 documentation] for full details.
777
778 -`GIT_REDACT_COOKIES`::
779 - This can be set to a comma-separated list of strings. When a curl trace
780 - is enabled (see `GIT_TRACE_CURL` above), whenever a "Cookies:" header
781 - sent by the client is dumped, values of cookies whose key is in that
782 - list (case-sensitive) are redacted.
778 +`GIT_TRACE_REDACT`::
779 + By default, when tracing is activated, Git redacts the values of
780 + cookies, the "Authorization:" header, and the "Proxy-Authorization:"
781 + header. Set this variable to `0` to prevent this redaction.
782
783 `GIT_LITERAL_PATHSPECS`::
784 Setting this variable to `1` will cause Git to treat all
http.c
+11 -24
@@ -18,7 +18,7 @@
18
19 static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
20 static int trace_curl_data = 1;
21 -static struct string_list cookies_to_redact = STRING_LIST_INIT_DUP;
21 +static int trace_curl_redact = 1;
22 #if LIBCURL_VERSION_NUM >= 0x070a08
23 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
24 #else
@@ -642,8 +642,9 @@ static void redact_sensitive_header(struct strbuf *header)
642 {
643 const char *sensitive_header;
644
645 - if (skip_prefix(header->buf, "Authorization:", &sensitive_header) ||
646 - skip_prefix(header->buf, "Proxy-Authorization:", &sensitive_header)) {
645 + if (trace_curl_redact &&
646 + (skip_prefix(header->buf, "Authorization:", &sensitive_header) ||
647 + skip_prefix(header->buf, "Proxy-Authorization:", &sensitive_header))) {
648 /* The first token is the type, which is OK to log */
649 while (isspace(*sensitive_header))
650 sensitive_header++;
@@ -652,20 +653,15 @@ static void redact_sensitive_header(struct strbuf *header)
653 /* Everything else is opaque and possibly sensitive */
654 strbuf_setlen(header, sensitive_header - header->buf);
655 strbuf_addstr(header, " <redacted>");
655 - } else if (cookies_to_redact.nr &&
656 + } else if (trace_curl_redact &&
657 skip_prefix(header->buf, "Cookie:", &sensitive_header)) {
658 struct strbuf redacted_header = STRBUF_INIT;
658 - char *cookie;
659 + const char *cookie;
660
661 while (isspace(*sensitive_header))
662 sensitive_header++;
663
663 - /*
664 - * The contents of header starting from sensitive_header will
665 - * subsequently be overridden, so it is fine to mutate this
666 - * string (hence the assignment to "char *").
667 - */
668 - cookie = (char *) sensitive_header;
664 + cookie = sensitive_header;
665
666 while (cookie) {
667 char *equals;
@@ -678,14 +674,8 @@ static void redact_sensitive_header(struct strbuf *header)
674 strbuf_addstr(&redacted_header, cookie);
675 continue;
676 }
681 - *equals = 0; /* temporarily set to NUL for lookup */
682 - if (string_list_lookup(&cookies_to_redact, cookie)) {
683 - strbuf_addstr(&redacted_header, cookie);
684 - strbuf_addstr(&redacted_header, "=<redacted>");
685 - } else {
686 - *equals = '=';
687 - strbuf_addstr(&redacted_header, cookie);
688 - }
677 + strbuf_add(&redacted_header, cookie, equals - cookie);
678 + strbuf_addstr(&redacted_header, "=<redacted>");
679 if (semicolon) {
680 /*
681 * There are more cookies. (Or, for some
@@ -1003,11 +993,8 @@ static CURL *get_curl_handle(void)
993 setup_curl_trace(result);
994 if (getenv("GIT_TRACE_CURL_NO_DATA"))
995 trace_curl_data = 0;
1006 - if (getenv("GIT_REDACT_COOKIES")) {
1007 - string_list_split(&cookies_to_redact,
1008 - getenv("GIT_REDACT_COOKIES"), ',', -1);
1009 - string_list_sort(&cookies_to_redact);
1010 - }
996 + if (!git_env_bool("GIT_TRACE_REDACT", 1))
997 + trace_curl_redact = 0;
998
999 curl_easy_setopt(result, CURLOPT_USERAGENT,
1000 user_agent ? user_agent : git_user_agent());
t/t5551-http-fetch-smart.sh
+24 -14
@@ -209,6 +209,16 @@ test_expect_success 'GIT_CURL_VERBOSE redacts auth details' '
209 grep "Authorization: Basic <redacted>" trace
210 '
211
212 +test_expect_success 'GIT_TRACE_CURL does not redact auth details if GIT_TRACE_REDACT=0' '
213 + rm -rf redact-auth trace &&
214 + set_askpass user@host pass@host &&
215 + GIT_TRACE_REDACT=0 GIT_TRACE_CURL="$(pwd)/trace" \
216 + git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
217 + expect_askpass both user@host &&
218 +
219 + grep "Authorization: Basic [0-9a-zA-Z+/]" trace
220 +'
221 +
222 test_expect_success 'disable dumb http on server' '
223 git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
224 config http.getanyfile false
@@ -454,37 +464,37 @@ test_expect_success 'fetch by SHA-1 without tag following' '
464 --no-tags origin $(cat bar_hash)
465 '
466
457 -test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
467 +test_expect_success 'cookies are redacted by default' '
468 rm -rf clone &&
469 echo "Set-Cookie: Foo=1" >cookies &&
470 echo "Set-Cookie: Bar=2" >>cookies &&
461 - GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Bar,Baz \
471 + GIT_TRACE_CURL=true \
472 git -c "http.cookieFile=$(pwd)/cookies" clone \
473 $HTTPD_URL/smart/repo.git clone 2>err &&
464 - grep "Cookie:.*Foo=1" err &&
474 + grep "Cookie:.*Foo=<redacted>" err &&
475 grep "Cookie:.*Bar=<redacted>" err &&
476 + ! grep "Cookie:.*Foo=1" err &&
477 ! grep "Cookie:.*Bar=2" err
478 '
479
469 -test_expect_success 'GIT_REDACT_COOKIES redacts cookies when GIT_CURL_VERBOSE=1' '
480 +test_expect_success 'empty values of cookies are also redacted' '
481 rm -rf clone &&
471 - echo "Set-Cookie: Foo=1" >cookies &&
472 - echo "Set-Cookie: Bar=2" >>cookies &&
473 - GIT_CURL_VERBOSE=1 GIT_REDACT_COOKIES=Bar,Baz \
482 + echo "Set-Cookie: Foo=" >cookies &&
483 + GIT_TRACE_CURL=true \
484 git -c "http.cookieFile=$(pwd)/cookies" clone \
485 $HTTPD_URL/smart/repo.git clone 2>err &&
476 - grep "Cookie:.*Foo=1" err &&
477 - grep "Cookie:.*Bar=<redacted>" err &&
478 - ! grep "Cookie:.*Bar=2" err
486 + grep "Cookie:.*Foo=<redacted>" err
487 '
488
481 -test_expect_success 'GIT_REDACT_COOKIES handles empty values' '
489 +test_expect_success 'GIT_TRACE_REDACT=0 disables cookie redaction' '
490 rm -rf clone &&
483 - echo "Set-Cookie: Foo=" >cookies &&
484 - GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Foo \
491 + echo "Set-Cookie: Foo=1" >cookies &&
492 + echo "Set-Cookie: Bar=2" >>cookies &&
493 + GIT_TRACE_REDACT=0 GIT_TRACE_CURL=true \
494 git -c "http.cookieFile=$(pwd)/cookies" clone \
495 $HTTPD_URL/smart/repo.git clone 2>err &&
487 - grep "Cookie:.*Foo=<redacted>" err
496 + grep "Cookie:.*Foo=1" err &&
497 + grep "Cookie:.*Bar=2" err
498 '
499
500 test_expect_success 'GIT_TRACE_CURL_NO_DATA prevents data from being traced' '