[PATCH v2] http: redact curl h2h3 headers in info

["Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>]

From: Glen Choo <chooglen@google.com>

With GIT_TRACE_CURL=1 or GIT_CURL_VERBOSE=1, sensitive headers like
"Authorization" and "Cookie" get redacted. However, since [1], curl's
h2h3 module also prints headers in its "info", which don't get redacted.
For example,

  echo 'github.com	TRUE	/	FALSE	1698960413304	o	foo=bar' >cookiefile &&
  GIT_TRACE_CURL=1 GIT_TRACE_CURL_NO_DATA=1 git \
    -c 'http.cookiefile=cookiefile' \
    -c 'http.version=' \
    ls-remote https://github.com/git/git refs/heads/main 2>output &&
  grep 'cookie' output

produces output like:

  23:04:16.920495 http.c:678              == Info: h2h3 [cookie: o=foo=bar]
  23:04:16.920562 http.c:637              => Send header: cookie: o=<redacted>

Teach http.c to check for h2h3 headers in info and redact them using the
existing header redaction logic.

[1] https://github.com/curl/curl/commit/f8c3724aa90472c0e617ddbbc420aa199971eb77

Signed-off-by: Glen Choo <chooglen@google.com>
---
    http: redact curl h2h3 headers in info
    
    Thanks for the feedback, all :) For this patch, it sounds like it would
    be too onerous to do the kinds of testing I initially envisioned, so I
    think v2 is ready as-is.
    
    Changes in v2:
    
     * Describe the redacted string in comments.
     * Return 1 for "redactions have happened".
     * Fix a leak of the "inner" strbuf.
     * Rename function, fix typo.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1377%2Fchooglen%2Fhttp%2Fredact-h2h3-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1377/chooglen/http/redact-h2h3-v2
Pull-Request: https://github.com/git/git/pull/1377

Range-diff vs v1:

 1:  e9232396736 ! 1:  a8c35ff4ddf http: redact curl h2h3 headers in info
     @@ http.c: static void set_curl_keepalive(CURL *c)
       #endif
       
      -static void redact_sensitive_header(struct strbuf *header)
     -+/* Return 0 if redactions been made, 1 otherwise. */
     ++/* Return 1 if redactions have been made, 0 otherwise. */
      +static int redact_sensitive_header(struct strbuf *header)
       {
     -+	int ret = 1;
     ++	int ret = 0;
       	const char *sensitive_header;
       
       	if (trace_curl_redact &&
     @@ http.c: 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>");
     -+		ret = 0;
     ++		ret = 1;
       	} else if (trace_curl_redact &&
       		   skip_iprefix(header->buf, "Cookie:", &sensitive_header)) {
       		struct strbuf redacted_header = STRBUF_INIT;
     @@ http.c: static void redact_sensitive_header(struct strbuf *header)
       
       		strbuf_setlen(header, sensitive_header - header->buf);
       		strbuf_addbuf(header, &redacted_header);
     -+		ret = 0;
     ++		ret = 1;
      +	}
      +	return ret;
      +}
     @@ http.c: static void redact_sensitive_header(struct strbuf *header)
      +{
      +	const char *sensitive_header;
      +
     ++	/*
     ++	 * curl's h2h3 prints headers in info, e.g.:
     ++	 *   h2h3 [<header-name>: <header-val>]
     ++	 */
      +	if (trace_curl_redact &&
      +	    skip_iprefix(header->buf, "h2h3 [", &sensitive_header)) {
      +		struct strbuf inner = STRBUF_INIT;
      +
      +		/* Drop the trailing "]" */
      +		strbuf_add(&inner, sensitive_header, strlen(sensitive_header) - 1);
     -+		if (!redact_sensitive_header(&inner)) {
     ++		if (redact_sensitive_header(&inner)) {
      +			strbuf_setlen(header, strlen("h2h3 ["));
      +			strbuf_addbuf(header, &inner);
      +			strbuf_addch(header, ']');
      +		}
     ++
     ++		strbuf_release(&inner);
       	}
       }
       
     @@ http.c: static void curl_dump_data(const char *text, unsigned char *ptr, size_t
       	strbuf_release(&out);
       }
       
     -+static void curl_print_info(char *data, size_t size)
     ++static void curl_dump_info(char *data, size_t size)
      +{
      +	struct strbuf buf = STRBUF_INIT;
      +
     @@ http.c: static int curl_trace(CURL *handle, curl_infotype type, char *data, size
       	switch (type) {
       	case CURLINFO_TEXT:
      -		trace_printf_key(&trace_curl, "== Info: %s", data);
     -+		curl_print_info(data, size);
     ++		curl_dump_info(data, size);
       		break;
       	case CURLINFO_HEADER_OUT:
       		text = "=> Send header";


 http.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 5d0502f51fd..8135fac283e 100644
--- a/http.c
+++ b/http.c
@@ -560,8 +560,10 @@ static void set_curl_keepalive(CURL *c)
 }
 #endif
 
-static void redact_sensitive_header(struct strbuf *header)
+/* Return 1 if redactions have been made, 0 otherwise. */
+static int redact_sensitive_header(struct strbuf *header)
 {
+	int ret = 0;
 	const char *sensitive_header;
 
 	if (trace_curl_redact &&
@@ -575,6 +577,7 @@ 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>");
+		ret = 1;
 	} else if (trace_curl_redact &&
 		   skip_iprefix(header->buf, "Cookie:", &sensitive_header)) {
 		struct strbuf redacted_header = STRBUF_INIT;
@@ -612,6 +615,33 @@ static void redact_sensitive_header(struct strbuf *header)
 
 		strbuf_setlen(header, sensitive_header - header->buf);
 		strbuf_addbuf(header, &redacted_header);
+		ret = 1;
+	}
+	return ret;
+}
+
+/* Redact headers in info */
+static void redact_sensitive_info_header(struct strbuf *header)
+{
+	const char *sensitive_header;
+
+	/*
+	 * curl's h2h3 prints headers in info, e.g.:
+	 *   h2h3 [<header-name>: <header-val>]
+	 */
+	if (trace_curl_redact &&
+	    skip_iprefix(header->buf, "h2h3 [", &sensitive_header)) {
+		struct strbuf inner = STRBUF_INIT;
+
+		/* Drop the trailing "]" */
+		strbuf_add(&inner, sensitive_header, strlen(sensitive_header) - 1);
+		if (redact_sensitive_header(&inner)) {
+			strbuf_setlen(header, strlen("h2h3 ["));
+			strbuf_addbuf(header, &inner);
+			strbuf_addch(header, ']');
+		}
+
+		strbuf_release(&inner);
 	}
 }
 
@@ -668,6 +698,18 @@ static void curl_dump_data(const char *text, unsigned char *ptr, size_t size)
 	strbuf_release(&out);
 }
 
+static void curl_dump_info(char *data, size_t size)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	strbuf_add(&buf, data, size);
+
+	redact_sensitive_info_header(&buf);
+	trace_printf_key(&trace_curl, "== Info: %s", buf.buf);
+
+	strbuf_release(&buf);
+}
+
 static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp)
 {
 	const char *text;
@@ -675,7 +717,7 @@ static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size,
 
 	switch (type) {
 	case CURLINFO_TEXT:
-		trace_printf_key(&trace_curl, "== Info: %s", data);
+		curl_dump_info(data, size);
 		break;
 	case CURLINFO_HEADER_OUT:
 		text = "=> Send header";

base-commit: c03801e19cb8ab36e9c0d17ff3d5e0c3b0f24193
-- 
gitgitgadget