From 83411783c3ebd0ffb55a05e8ad63b13b8530747f Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 18 Jan 2018 16:28:01 -0800 Subject: [PATCH 1/2] http: support cookie redaction when tracing When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and "Proxy-Authorization:" HTTP headers. Extend this redaction to a user-specified list of cookies, specified through the "GIT_REDACT_COOKIES" environment variable. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- Documentation/git.txt | 6 ++++ http.c | 55 +++++++++++++++++++++++++++++++++++++ t/t5551-http-fetch-smart.sh | 21 ++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 3f4161a799..5446d21438 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -646,6 +646,12 @@ of clones and fetches. variable. See `GIT_TRACE` for available trace output options. +`GIT_REDACT_COOKIES`:: + This can be set to a comma-separated list of strings. When a curl trace + is enabled (see `GIT_TRACE_CURL` above), whenever a "Cookies:" header + sent by the client is dumped, values of cookies whose key is in that + list (case-sensitive) are redacted. + `GIT_LITERAL_PATHSPECS`:: Setting this variable to `1` will cause Git to treat all pathspecs literally, rather than as glob patterns. For example, diff --git a/http.c b/http.c index 5977712712..088cf70bf0 100644 --- a/http.c +++ b/http.c @@ -13,8 +13,10 @@ #include "transport.h" #include "packfile.h" #include "protocol.h" +#include "string-list.h" static struct trace_key trace_curl = TRACE_KEY_INIT(CURL); +static struct string_list cookies_to_redact = STRING_LIST_INIT_DUP; #if LIBCURL_VERSION_NUM >= 0x070a08 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER; #else @@ -575,6 +577,54 @@ static void redact_sensitive_header(struct strbuf *header) /* Everything else is opaque and possibly sensitive */ strbuf_setlen(header, sensitive_header - header->buf); strbuf_addstr(header, " "); + } else if (cookies_to_redact.nr && + skip_prefix(header->buf, "Cookie:", &sensitive_header)) { + struct strbuf redacted_header = STRBUF_INIT; + char *cookie; + + while (isspace(*sensitive_header)) + sensitive_header++; + + /* + * The contents of header starting from sensitive_header will + * subsequently be overridden, so it is fine to mutate this + * string (hence the assignment to "char *"). + */ + cookie = (char *) sensitive_header; + + while (cookie) { + char *equals; + char *semicolon = strstr(cookie, "; "); + if (semicolon) + *semicolon = 0; + equals = strchrnul(cookie, '='); + if (!equals) { + /* invalid cookie, just append and continue */ + strbuf_addstr(&redacted_header, cookie); + continue; + } + *equals = 0; /* temporarily set to NUL for lookup */ + if (string_list_lookup(&cookies_to_redact, cookie)) { + strbuf_addstr(&redacted_header, cookie); + strbuf_addstr(&redacted_header, "="); + } else { + *equals = '='; + strbuf_addstr(&redacted_header, cookie); + } + if (semicolon) { + /* + * There are more cookies. (Or, for some + * reason, the input string ends in "; ".) + */ + strbuf_addstr(&redacted_header, "; "); + cookie = semicolon + strlen("; "); + } else { + cookie = NULL; + } + } + + strbuf_setlen(header, sensitive_header - header->buf); + strbuf_addbuf(header, &redacted_header); } } @@ -807,6 +857,11 @@ static CURL *get_curl_handle(void) if (getenv("GIT_CURL_VERBOSE")) curl_easy_setopt(result, CURLOPT_VERBOSE, 1L); setup_curl_trace(result); + if (getenv("GIT_REDACT_COOKIES")) { + string_list_split(&cookies_to_redact, + getenv("GIT_REDACT_COOKIES"), ',', -1); + string_list_sort(&cookies_to_redact); + } curl_easy_setopt(result, CURLOPT_USERAGENT, user_agent ? user_agent : git_user_agent()); diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index a51b7e20d3..21a5ce8601 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -364,5 +364,26 @@ test_expect_success 'custom http headers' ' submodule update sub ' +test_expect_success 'GIT_REDACT_COOKIES redacts cookies' ' + rm -rf clone && + echo "Set-Cookie: Foo=1" >cookies && + echo "Set-Cookie: Bar=2" >>cookies && + GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Bar,Baz \ + git -c "http.cookieFile=$(pwd)/cookies" clone \ + $HTTPD_URL/smart/repo.git clone 2>err && + grep "Cookie:.*Foo=1" err && + grep "Cookie:.*Bar=" err && + ! grep "Cookie:.*Bar=2" err +' + +test_expect_success 'GIT_REDACT_COOKIES handles empty values' ' + rm -rf clone && + echo "Set-Cookie: Foo=" >cookies && + GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Foo \ + git -c "http.cookieFile=$(pwd)/cookies" clone \ + $HTTPD_URL/smart/repo.git clone 2>err && + grep "Cookie:.*Foo=" err +' + stop_httpd test_done From 8ba18e6fa402666fe94a285cd27addd9b0df6462 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 18 Jan 2018 16:28:02 -0800 Subject: [PATCH 2/2] http: support omitting data from traces GIT_TRACE_CURL provides a way to debug what is being sent and received over HTTP, with automatic redaction of sensitive information. But it also logs data transmissions, which significantly increases the log file size, sometimes unnecessarily. Add an option "GIT_TRACE_CURL_NO_DATA" to allow the user to omit such data transmissions. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- Documentation/git.txt | 4 ++++ http.c | 27 +++++++++++++++++++-------- t/t5551-http-fetch-smart.sh | 12 ++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 5446d21438..8163b5796b 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -646,6 +646,10 @@ of clones and fetches. variable. See `GIT_TRACE` for available trace output options. +`GIT_TRACE_CURL_NO_DATA`:: + When a curl trace is enabled (see `GIT_TRACE_CURL` above), do not dump + data (that is, only dump info lines and headers). + `GIT_REDACT_COOKIES`:: This can be set to a comma-separated list of strings. When a curl trace is enabled (see `GIT_TRACE_CURL` above), whenever a "Cookies:" header diff --git a/http.c b/http.c index 088cf70bf0..32a8238955 100644 --- a/http.c +++ b/http.c @@ -16,6 +16,7 @@ #include "string-list.h" static struct trace_key trace_curl = TRACE_KEY_INIT(CURL); +static int trace_curl_data = 1; static struct string_list cookies_to_redact = STRING_LIST_INIT_DUP; #if LIBCURL_VERSION_NUM >= 0x070a08 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER; @@ -695,24 +696,32 @@ static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, curl_dump_header(text, (unsigned char *)data, size, DO_FILTER); break; case CURLINFO_DATA_OUT: - text = "=> Send data"; - curl_dump_data(text, (unsigned char *)data, size); + if (trace_curl_data) { + text = "=> Send data"; + curl_dump_data(text, (unsigned char *)data, size); + } break; case CURLINFO_SSL_DATA_OUT: - text = "=> Send SSL data"; - curl_dump_data(text, (unsigned char *)data, size); + if (trace_curl_data) { + text = "=> Send SSL data"; + curl_dump_data(text, (unsigned char *)data, size); + } break; case CURLINFO_HEADER_IN: text = "<= Recv header"; curl_dump_header(text, (unsigned char *)data, size, NO_FILTER); break; case CURLINFO_DATA_IN: - text = "<= Recv data"; - curl_dump_data(text, (unsigned char *)data, size); + if (trace_curl_data) { + text = "<= Recv data"; + curl_dump_data(text, (unsigned char *)data, size); + } break; case CURLINFO_SSL_DATA_IN: - text = "<= Recv SSL data"; - curl_dump_data(text, (unsigned char *)data, size); + if (trace_curl_data) { + text = "<= Recv SSL data"; + curl_dump_data(text, (unsigned char *)data, size); + } break; default: /* we ignore unknown types by default */ @@ -857,6 +866,8 @@ static CURL *get_curl_handle(void) if (getenv("GIT_CURL_VERBOSE")) curl_easy_setopt(result, CURLOPT_VERBOSE, 1L); setup_curl_trace(result); + if (getenv("GIT_TRACE_CURL_NO_DATA")) + trace_curl_data = 0; if (getenv("GIT_REDACT_COOKIES")) { string_list_split(&cookies_to_redact, getenv("GIT_REDACT_COOKIES"), ',', -1); diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index 21a5ce8601..f5721b4a59 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -385,5 +385,17 @@ test_expect_success 'GIT_REDACT_COOKIES handles empty values' ' grep "Cookie:.*Foo=" err ' +test_expect_success 'GIT_TRACE_CURL_NO_DATA prevents data from being traced' ' + rm -rf clone && + GIT_TRACE_CURL=true \ + git clone $HTTPD_URL/smart/repo.git clone 2>err && + grep "=> Send data" err && + + rm -rf clone && + GIT_TRACE_CURL=true GIT_TRACE_CURL_NO_DATA=1 \ + git clone $HTTPD_URL/smart/repo.git clone 2>err && + ! grep "=> Send data" err +' + stop_httpd test_done