Skip to content

Commit

Permalink
http: redact curl h2h3 headers in info
Browse files Browse the repository at this point in the history
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] curl/curl@f8c3724

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
  • Loading branch information
chooglen authored and ttaylorr committed Nov 14, 2022
1 parent 73c49a4 commit b637a41
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 9 deletions.
47 changes: 41 additions & 6 deletions http.c
Expand Up @@ -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++;
Expand All @@ -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;

Expand Down Expand Up @@ -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, ']');
}
}
}

Expand All @@ -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));
Expand Down Expand Up @@ -668,14 +691,26 @@ 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;
enum { NO_FILTER = 0, DO_FILTER = 1 };

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";
Expand Down
6 changes: 3 additions & 3 deletions t/t5551-http-fetch-smart.sh
Expand Up @@ -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 &&
Expand All @@ -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 &&
Expand Down Expand Up @@ -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 &&
Expand Down

0 comments on commit b637a41

Please sign in to comment.