git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [RFC PATCH 0/2] Cookie redaction during GIT_TRACE_CURL
@ 2018-01-18  0:34 Jonathan Tan
  2018-01-18  0:34 ` [RFC PATCH 1/2] http: support cookie redaction when tracing Jonathan Tan
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Jonathan Tan @ 2018-01-18  0:34 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan

Sometimes authentication information is sent over HTTP through cookies,
but when using GIT_TRACE_CURL, that information appears in logs. There
are some HTTP headers already redacted ("Authorization:" and
"Proxy-Authorization:") - the first patch extends such redaction to a
user-specified list.

I've also included another patch to allow omission of data transmission
information from being logged when using GIT_TRACE_CURL. This reduces
the information logged to that similar to GIT_CURL_VERBOSE.
(As for why not use GIT_CURL_VERBOSE instead - that is because
GIT_CURL_VERBOSE does not perform any redaction, merely using Curl's
default logging mechanism.)

The patches are ready for merging, but I marked this as "RFC" just in
case there is a better way to accomplish this.

Jonathan Tan (2):
  http: support cookie redaction when tracing
  http: support omitting data from traces

 http.c                      | 82 ++++++++++++++++++++++++++++++++++++++++-----
 t/t5551-http-fetch-smart.sh | 24 +++++++++++++
 2 files changed, 98 insertions(+), 8 deletions(-)

-- 
2.16.0.rc1.238.g530d649a79-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH 1/2] http: support cookie redaction when tracing
  2018-01-18  0:34 [RFC PATCH 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
@ 2018-01-18  0:34 ` Jonathan Tan
  2018-01-18 18:25   ` Eric Sunshine
  2018-01-18  0:34 ` [RFC PATCH 2/2] http: support omitting data from traces Jonathan Tan
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Jonathan Tan @ 2018-01-18  0:34 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan

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 <jonathantanmy@google.com>
---
 http.c                      | 55 +++++++++++++++++++++++++++++++++++++++++++++
 t/t5551-http-fetch-smart.sh | 12 ++++++++++
 2 files changed, 67 insertions(+)

diff --git a/http.c b/http.c
index 597771271..088cf70bf 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, " <redacted>");
+	} 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, "=<redacted>");
+			} 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 a51b7e20d..88bd9c094 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -364,5 +364,17 @@ test_expect_success 'custom http headers' '
 		submodule update sub
 '
 
+test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
+	rm -rf clone &&
+	echo "Set-Cookie: NotSecret=Foo" >cookies &&
+	echo "Set-Cookie: Secret=Bar" >>cookies &&
+	GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Secret,AnotherSecret \
+		git -c "http.cookieFile=$(pwd)/cookies" clone \
+		$HTTPD_URL/smart/repo.git clone 2>err &&
+	grep "Cookie:.*NotSecret=Foo" err &&
+	grep "Cookie:.*Secret=<redacted>" err &&
+	! grep "Cookie:.*Secret=Bar" err
+'
+
 stop_httpd
 test_done
-- 
2.16.0.rc1.238.g530d649a79-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH 2/2] http: support omitting data from traces
  2018-01-18  0:34 [RFC PATCH 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
  2018-01-18  0:34 ` [RFC PATCH 1/2] http: support cookie redaction when tracing Jonathan Tan
@ 2018-01-18  0:34 ` Jonathan Tan
  2018-01-18 18:25   ` Eric Sunshine
  2018-01-19  0:28 ` [PATCH v2 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Jonathan Tan @ 2018-01-18  0:34 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan

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 <jonathantanmy@google.com>
---
 http.c                      | 27 +++++++++++++++++++--------
 t/t5551-http-fetch-smart.sh | 12 ++++++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/http.c b/http.c
index 088cf70bf..32a823895 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 88bd9c094..0c62d00af 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -376,5 +376,17 @@ test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
 	! grep "Cookie:.*Secret=Bar" 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
-- 
2.16.0.rc1.238.g530d649a79-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 1/2] http: support cookie redaction when tracing
  2018-01-18  0:34 ` [RFC PATCH 1/2] http: support cookie redaction when tracing Jonathan Tan
@ 2018-01-18 18:25   ` Eric Sunshine
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Sunshine @ 2018-01-18 18:25 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: Git List

On Wed, Jan 17, 2018 at 7:34 PM, Jonathan Tan <jonathantanmy@google.com> wrote:
> 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 <jonathantanmy@google.com>
> ---
>  http.c                      | 55 +++++++++++++++++++++++++++++++++++++++++++++
>  t/t5551-http-fetch-smart.sh | 12 ++++++++++
>  2 files changed, 67 insertions(+)

Please document GIT_REDACT_COOKIES in Documentation/git.txt.

> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> @@ -364,5 +364,17 @@ test_expect_success 'custom http headers' '
> +test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
> +       rm -rf clone &&
> +       echo "Set-Cookie: NotSecret=Foo" >cookies &&
> +       echo "Set-Cookie: Secret=Bar" >>cookies &&
> +       GIT_TRACE_CURL=true GIT_REDACT_COOKIES=Secret,AnotherSecret \
> +               git -c "http.cookieFile=$(pwd)/cookies" clone \
> +               $HTTPD_URL/smart/repo.git clone 2>err &&
> +       grep "Cookie:.*NotSecret=Foo" err &&
> +       grep "Cookie:.*Secret=<redacted>" err &&
> +       ! grep "Cookie:.*Secret=Bar" err
> +'

The looseness of these grep expressions (/Cookie:.*Secret/ also
matches "Cookie: NotSecret", for instance) requires extra
concentration on the part of the reader to see that you do indeed
cover all cases. I wonder, therefore, if it would be better to tighten
them to instead match the exact string.

Also, after reading the implementation, I had expected to see testing
of the "Cookie: foo=bar; cow=moo" case, as well as the handled corner
cases, such as missing missing "=" and missing value after "=".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 2/2] http: support omitting data from traces
  2018-01-18  0:34 ` [RFC PATCH 2/2] http: support omitting data from traces Jonathan Tan
@ 2018-01-18 18:25   ` Eric Sunshine
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Sunshine @ 2018-01-18 18:25 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: Git List

On Wed, Jan 17, 2018 at 7:34 PM, Jonathan Tan <jonathantanmy@google.com> wrote:
> 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 <jonathantanmy@google.com>
> ---
>  http.c                      | 27 +++++++++++++++++++--------
>  t/t5551-http-fetch-smart.sh | 12 ++++++++++++
>  2 files changed, 31 insertions(+), 8 deletions(-)

Please document GIT_TRACE_CURL_NO_DATA in Documentation/git.txt.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 0/2] Cookie redaction during GIT_TRACE_CURL
  2018-01-18  0:34 [RFC PATCH 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
  2018-01-18  0:34 ` [RFC PATCH 1/2] http: support cookie redaction when tracing Jonathan Tan
  2018-01-18  0:34 ` [RFC PATCH 2/2] http: support omitting data from traces Jonathan Tan
@ 2018-01-19  0:28 ` Jonathan Tan
  2018-01-19  0:28 ` [PATCH v2 1/2] http: support cookie redaction when tracing Jonathan Tan
  2018-01-19  0:28 ` [PATCH v2 2/2] http: support omitting data from traces Jonathan Tan
  4 siblings, 0 replies; 8+ messages in thread
From: Jonathan Tan @ 2018-01-19  0:28 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, sunshine

Thanks, Eric. Changes in v2:
 - documented all environment variables introduced
 - made test more clear by ensuring that no cookie keys are suffixes or
   prefixes of others
 - tested empty value

As far as I can tell, it does not seem possible that Git generates a
cookie with no equals sign (like "Secure" or "HttpOnly" described in RFC
6265). When I try to craft a cookie file containing that (using
"Set-Cookie: Foo=; HttpOnly", for example), the no-equals-sign cookie
just disappears.

Jonathan Tan (2):
  http: support cookie redaction when tracing
  http: support omitting data from traces

 Documentation/git.txt       | 10 ++++++
 http.c                      | 82 ++++++++++++++++++++++++++++++++++++++++-----
 t/t5551-http-fetch-smart.sh | 33 ++++++++++++++++++
 3 files changed, 117 insertions(+), 8 deletions(-)

-- 
2.16.0.rc2.37.ge0d575025.dirty


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] http: support cookie redaction when tracing
  2018-01-18  0:34 [RFC PATCH 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
                   ` (2 preceding siblings ...)
  2018-01-19  0:28 ` [PATCH v2 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
@ 2018-01-19  0:28 ` Jonathan Tan
  2018-01-19  0:28 ` [PATCH v2 2/2] http: support omitting data from traces Jonathan Tan
  4 siblings, 0 replies; 8+ messages in thread
From: Jonathan Tan @ 2018-01-19  0:28 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, sunshine

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 <jonathantanmy@google.com>
---
 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 3f4161a79..5446d2143 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 597771271..088cf70bf 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, " <redacted>");
+	} 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, "=<redacted>");
+			} 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 a51b7e20d..21a5ce860 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=<redacted>" 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=<redacted>" err
+'
+
 stop_httpd
 test_done
-- 
2.16.0.rc2.37.ge0d575025.dirty


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] http: support omitting data from traces
  2018-01-18  0:34 [RFC PATCH 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
                   ` (3 preceding siblings ...)
  2018-01-19  0:28 ` [PATCH v2 1/2] http: support cookie redaction when tracing Jonathan Tan
@ 2018-01-19  0:28 ` Jonathan Tan
  4 siblings, 0 replies; 8+ messages in thread
From: Jonathan Tan @ 2018-01-19  0:28 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, sunshine

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 <jonathantanmy@google.com>
---
 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 5446d2143..8163b5796 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 088cf70bf..32a823895 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 21a5ce860..f5721b4a5 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=<redacted>" 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
-- 
2.16.0.rc2.37.ge0d575025.dirty


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-01-19  0:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18  0:34 [RFC PATCH 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
2018-01-18  0:34 ` [RFC PATCH 1/2] http: support cookie redaction when tracing Jonathan Tan
2018-01-18 18:25   ` Eric Sunshine
2018-01-18  0:34 ` [RFC PATCH 2/2] http: support omitting data from traces Jonathan Tan
2018-01-18 18:25   ` Eric Sunshine
2018-01-19  0:28 ` [PATCH v2 0/2] Cookie redaction during GIT_TRACE_CURL Jonathan Tan
2018-01-19  0:28 ` [PATCH v2 1/2] http: support cookie redaction when tracing Jonathan Tan
2018-01-19  0:28 ` [PATCH v2 2/2] http: support omitting data from traces Jonathan Tan

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).