From: "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Glen Choo <chooglen@google.com>, Glen Choo <chooglen@google.com>
Subject: [PATCH] http: redact curl h2h3 headers in info
Date: Wed, 09 Nov 2022 00:52:31 +0000 [thread overview]
Message-ID: <pull.1377.git.git.1667955151994.gitgitgadget@gmail.com> (raw)
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
I initially sent this to the security list, but the general impression
is that this isn't sensitive enough for an embargoed fix, so this is
better discussed in the open instead.
Since this comes from curl's HTTP2.0/3.0 module, this can be mitigated
by setting http.version to 1.X, e.g. "git -c http.version=HTTP/1.1".
According to [1], the susceptible curl versions appear to be 7.86.0,
7.85.0, 7.84.0, 7.83.1, 7.83.0, 7.82.0, but I'm not sure which platforms
are vulnerable.
This patch fixes the issue on my machine running curl 7.85.0, so I think
it is okay to merge as-is. That said, I would strongly prefer to add
tests, but I haven't figured out how. In particular:
* Do we have a way of using HTTP/2.0 in our tests? A cursory glance at
our httpd config suggests that we only use HTTP/1.1.
* How could we set up end-to-end tests to ensure that we're testing
this against affected versions of curl? To avoid regressions, I'd
also prefer to test against future versions of curl too.
[1]
https://github.com/curl/curl/commit/f8c3724aa90472c0e617ddbbc420aa199971eb77
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1377%2Fchooglen%2Fhttp%2Fredact-h2h3-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1377/chooglen/http/redact-h2h3-v1
Pull-Request: https://github.com/git/git/pull/1377
http.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/http.c b/http.c
index 5d0502f51fd..cbcc7c3f5b6 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 0 if redactions been made, 1 otherwise. */
+static int redact_sensitive_header(struct strbuf *header)
{
+ int ret = 1;
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 = 0;
} else if (trace_curl_redact &&
skip_iprefix(header->buf, "Cookie:", &sensitive_header)) {
struct strbuf redacted_header = STRBUF_INIT;
@@ -612,6 +615,27 @@ static void redact_sensitive_header(struct strbuf *header)
strbuf_setlen(header, sensitive_header - header->buf);
strbuf_addbuf(header, &redacted_header);
+ ret = 0;
+ }
+ return ret;
+}
+
+/* Redact headers in info */
+static void redact_sensitive_info_header(struct strbuf *header)
+{
+ const char *sensitive_header;
+
+ 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, ']');
+ }
}
}
@@ -668,6 +692,18 @@ static void curl_dump_data(const char *text, unsigned char *ptr, size_t size)
strbuf_release(&out);
}
+static void curl_print_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 +711,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_print_info(data, size);
break;
case CURLINFO_HEADER_OUT:
text = "=> Send header";
base-commit: c03801e19cb8ab36e9c0d17ff3d5e0c3b0f24193
--
gitgitgadget
next reply other threads:[~2022-11-09 0:52 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 0:52 Glen Choo via GitGitGadget [this message]
2022-11-10 2:52 ` [PATCH] http: redact curl h2h3 headers in info Taylor Blau
2022-11-10 17:48 ` Glen Choo
2022-11-10 21:50 ` Jeff King
2022-11-10 22:53 ` Glen Choo
2022-11-11 2:29 ` Jeff King
2022-11-11 2:31 ` Taylor Blau
2022-11-11 14:49 ` [PATCH] t: run t5551 tests with both HTTP and HTTP/2 Jeff King
2022-11-11 15:06 ` Ævar Arnfjörð Bjarmason
2022-11-11 15:19 ` Jeff King
2022-11-11 15:20 ` Jeff King
2022-11-10 21:57 ` [PATCH] http: redact curl h2h3 headers in info Emily Shaffer
2022-11-10 22:14 ` Glen Choo
2022-11-11 2:35 ` Taylor Blau
2022-11-10 22:57 ` [PATCH v2] " Glen Choo via GitGitGadget
2022-11-11 2:36 ` Taylor Blau
2022-11-11 2:38 ` Jeff King
2022-11-11 2:39 ` Taylor Blau
2022-11-11 17:55 ` Glen Choo
2022-11-11 22:35 ` [PATCH v3 0/2] " Glen Choo via GitGitGadget
2022-11-11 22:35 ` [PATCH v3 1/2] t: run t5551 tests with both HTTP and HTTP/2 Jeff King via GitGitGadget
2022-11-11 22:35 ` [PATCH v3 2/2] http: redact curl h2h3 headers in info Glen Choo via GitGitGadget
2022-11-14 22:33 ` [PATCH v3 0/2] " Jeff King
2022-11-14 22:43 ` Taylor Blau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1377.git.git.1667955151994.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public 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).