git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 2/2] http: redact curl h2h3 headers in info

From
Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
Date
Nov 11, 2022, 22:35 UTC
Message-ID
<bb5df1a48b98cb71826d53acb6c8f3b25d140b83.1668206106.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1377.v3.git.git.1668206106.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 (invoked when using HTTP/2) 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. This fixes the broken redaction logic that we noted in the previous commit, so mark the redaction tests as passing under HTTP2.

[1] https://github.com/curl/curl/commit/f8c3724aa90472c0e617ddbbc420aa199971eb77
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Glen Choo <chooglen@google.com>
---
 http.c                      | 47 ++++++++++++++++++++++++++++++++-----
 t/t5551-http-fetch-smart.sh |  6 ++---
 2 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/http.c b/http.c
index 5d0502f51fd..8a5ba3f4776 100644
--- a/http.c
+++ b/http.c
@@ -560,13 +560,15 @@ 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, size_t offset)
 {
+	int ret = 0;
 	const char *sensitive_header;
 
 	if (trace_curl_redact &&
-	    (skip_iprefix(header->buf, "Authorization:", &sensitive_header) ||
-	     skip_iprefix(header->buf, "Proxy-Authorization:", &sensitive_header))) {
+	    (skip_iprefix(header->buf + offset, "Authorization:", &sensitive_header) ||
+	     skip_iprefix(header->buf + offset, "Proxy-Authorization:", &sensitive_header))) {
 		/* The first token is the type, which is OK to log */
 		while (isspace(*sensitive_header))
 			sensitive_header++;
@@ -575,8 +577,9 @@ 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)) {
+		   skip_iprefix(header->buf + offset, "Cookie:", &sensitive_header)) {
 		struct strbuf redacted_header = STRBUF_INIT;
 		const char *cookie;
 
@@ -612,6 +615,26 @@ 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)) {
+		if (redact_sensitive_header(header, sensitive_header - header->buf)) {
+			/* redaction ate our closing bracket */
+			strbuf_addch(header, ']');
+		}
 	}
 }
 
@@ -629,7 +652,7 @@ static void curl_dump_header(const char *text, unsigned char *ptr, size_t size,
 
 	for (header = headers; *header; header++) {
 		if (hide_sensitive_header)
-			redact_sensitive_header(*header);
+			redact_sensitive_header(*header, 0);
 		strbuf_insertstr((*header), 0, text);
 		strbuf_insertstr((*header), strlen(text), ": ");
 		strbuf_rtrim((*header));
@@ -668,6 +691,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 +710,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";
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index ad0a0639e6b..a2ebce1787b 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -200,7 +200,7 @@ test_expect_success 'redirects send auth to new location' '
 	expect_askpass both user@host auth/smart/repo.git
 '
 
-test_expect_success !HTTP2 'GIT_TRACE_CURL redacts auth details' '
+test_expect_success 'GIT_TRACE_CURL redacts auth details' '
 	rm -rf redact-auth trace &&
 	set_askpass user@host pass@host &&
 	GIT_TRACE_CURL="$(pwd)/trace" git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
@@ -212,7 +212,7 @@ test_expect_success !HTTP2 'GIT_TRACE_CURL redacts auth details' '
 	grep -i "Authorization: Basic <redacted>" trace
 '
 
-test_expect_success !HTTP2 'GIT_CURL_VERBOSE redacts auth details' '
+test_expect_success 'GIT_CURL_VERBOSE redacts auth details' '
 	rm -rf redact-auth trace &&
 	set_askpass user@host pass@host &&
 	GIT_CURL_VERBOSE=1 git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth 2>trace &&
@@ -482,7 +482,7 @@ test_expect_success 'fetch by SHA-1 without tag following' '
 		--no-tags origin $(cat bar_hash)
 '
 
-test_expect_success !HTTP2 'cookies are redacted by default' '
+test_expect_success 'cookies are redacted by default' '
 	rm -rf clone &&
 	echo "Set-Cookie: Foo=1" >cookies &&
 	echo "Set-Cookie: Bar=2" >>cookies &&

-- 
gitgitgadget
Previous: Jeff King via GitGitGadgetNext: Jeff King
Message 22 of 24 in “http: redact curl h2h3 headers in info”
  1. http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 9, 2022
  2. Taylor BlauNov 10, 2022
  3. Glen ChooNov 10, 2022
  4. Jeff KingNov 10, 2022
  5. Glen ChooNov 10, 2022
  6. Jeff KingNov 11, 2022
  7. Taylor BlauNov 11, 2022
  8. t: run t5551 tests with both HTTP and HTTP/2Jeff King, Nov 11, 2022
  9. Ævar Arnfjörð BjarmasonNov 11, 2022
  10. Jeff KingNov 11, 2022
  11. Jeff KingNov 11, 2022
  12. Emily ShafferNov 10, 2022
  13. Glen ChooNov 10, 2022
  14. Taylor BlauNov 11, 2022
  15. http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 10, 2022
  16. Taylor BlauNov 11, 2022
  17. Jeff KingNov 11, 2022
  18. Taylor BlauNov 11, 2022
  19. Glen ChooNov 11, 2022
  20. 0/2 http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 11, 2022
  21. 1/2 t: run t5551 tests with both HTTP and HTTP/2Jeff King via GitGitGadget, Nov 11, 2022
  22. 2/2 http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 11, 2022
  23. Jeff KingNov 14, 2022
  24. Taylor BlauNov 14, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.