fetch-pack: redact packfile urls in traces

In some setups, packfile uris act as bearer token. It is not recommended to expose them plainly in logs, although in special circunstances (e.g. debug) it makes sense to write them. Redact the packfile URL paths by default, unless the GIT_TRACE_REDACT variable is set to false. This mimics the redacting of the Authorization header in HTTP. Signed-off-by: Ivan Frade <ifrade@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ivan Frade committed Nov 10, 2021 at 23:51 UTC 88e9b1e3fcbd3a8edcf1d54528c49f8237906aba
5 files changed +99 -3
Documentation/git.txt
+3 -2
@@ -832,8 +832,9 @@ for full details.
832
833 `GIT_TRACE_REDACT`::
834 By default, when tracing is activated, Git redacts the values of
835 - cookies, the "Authorization:" header, and the "Proxy-Authorization:"
836 - header. Set this variable to `0` to prevent this redaction.
835 + cookies, the "Authorization:" header, the "Proxy-Authorization:"
836 + header and packfile URIs. Set this variable to `0` to prevent this
837 + redaction.
838
839 `GIT_LITERAL_PATHSPECS`::
840 Setting this variable to `1` will cause Git to treat all
fetch-pack.c
+5
@@ -1653,8 +1653,13 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
1653 receive_wanted_refs(&reader, sought, nr_sought);
1654
1655 /* get the pack(s) */
1656 + if (git_env_bool("GIT_TRACE_REDACT", 1))
1657 + reader.options |= PACKET_READ_REDACT_URI_PATH;
1658 if (process_section_header(&reader, "packfile-uris", 1))
1659 receive_packfile_uris(&reader, &packfile_uris);
1660 + /* We don't expect more URIs. Reset to avoid expensive URI check. */
1661 + reader.options &= ~PACKET_READ_REDACT_URI_PATH;
1662 +
1663 process_section_header(&reader, "packfile", 0);
1664
1665 /*
pkt-line.c
+39 -1
@@ -370,6 +370,32 @@ int packet_length(const char lenbuf_hex[4])
370 return (val < 0) ? val : (val << 8) | hex2chr(lenbuf_hex + 2);
371 }
372
373 +static char *find_packfile_uri_path(const char *buffer)
374 +{
375 + const char *URI_MARK = "://";
376 + char *path;
377 + int len;
378 +
379 + /* First char is sideband mark */
380 + buffer += 1;
381 +
382 + len = strspn(buffer, "0123456789abcdefABCDEF");
383 + /* size of SHA1 and SHA256 hash */
384 + if (!(len == 40 || len == 64) || buffer[len] != ' ')
385 + return NULL; /* required "<hash>SP" not seen */
386 +
387 + path = strstr(buffer + len + 1, URI_MARK);
388 + if (!path)
389 + return NULL;
390 +
391 + path = strchr(path + strlen(URI_MARK), '/');
392 + if (!path || !*(path + 1))
393 + return NULL;
394 +
395 + /* position after '/' */
396 + return ++path;
397 +}
398 +
399 enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
400 size_t *src_len, char *buffer,
401 unsigned size, int *pktlen,
@@ -377,6 +403,7 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
403 {
404 int len;
405 char linelen[4];
406 + char *uri_path_start;
407
408 if (get_packet_data(fd, src_buffer, src_len, linelen, 4, options) < 0) {
409 *pktlen = -1;
@@ -427,7 +454,18 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
454 len--;
455
456 buffer[len] = 0;
430 - packet_trace(buffer, len, 0);
457 + if (options & PACKET_READ_REDACT_URI_PATH &&
458 + (uri_path_start = find_packfile_uri_path(buffer))) {
459 + const char *redacted = "<redacted>";
460 + struct strbuf tracebuf = STRBUF_INIT;
461 + strbuf_insert(&tracebuf, 0, buffer, len);
462 + strbuf_splice(&tracebuf, uri_path_start - buffer,
463 + strlen(uri_path_start), redacted, strlen(redacted));
464 + packet_trace(tracebuf.buf, tracebuf.len, 0);
465 + strbuf_release(&tracebuf);
466 + } else {
467 + packet_trace(buffer, len, 0);
468 + }
469
470 if ((options & PACKET_READ_DIE_ON_ERR_PACKET) &&
471 starts_with(buffer, "ERR "))
pkt-line.h
+1
@@ -87,6 +87,7 @@ void packet_fflush(FILE *f);
87 #define PACKET_READ_CHOMP_NEWLINE (1u<<1)
88 #define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
89 #define PACKET_READ_GENTLE_ON_READ_ERROR (1u<<3)
90 +#define PACKET_READ_REDACT_URI_PATH (1u<<4)
91 int packet_read(int fd, char *buffer, unsigned size, int options);
92
93 /*
t/t5702-protocol-v2.sh
+51
@@ -1107,6 +1107,57 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodul
1107 test_i18ngrep "disallowed submodule name" err
1108 '
1109
1110 +test_expect_success 'packfile-uri path redacted in trace' '
1111 + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
1112 + rm -rf "$P" http_child log &&
1113 +
1114 + git init "$P" &&
1115 + git -C "$P" config "uploadpack.allowsidebandall" "true" &&
1116 +
1117 + echo my-blob >"$P/my-blob" &&
1118 + git -C "$P" add my-blob &&
1119 + git -C "$P" commit -m x &&
1120 +
1121 + git -C "$P" hash-object my-blob >objh &&
1122 + git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
1123 + git -C "$P" config --add \
1124 + "uploadpack.blobpackfileuri" \
1125 + "$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
1126 +
1127 + GIT_TRACE_PACKET="$(pwd)/log" \
1128 + git -c protocol.version=2 \
1129 + -c fetch.uriprotocols=http,https \
1130 + clone "$HTTPD_URL/smart/http_parent" http_child &&
1131 +
1132 + grep -F "clone< \\1$(cat packh) $HTTPD_URL/<redacted>" log
1133 +'
1134 +
1135 +test_expect_success 'packfile-uri path not redacted in trace when GIT_TRACE_REDACT=0' '
1136 + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
1137 + rm -rf "$P" http_child log &&
1138 +
1139 + git init "$P" &&
1140 + git -C "$P" config "uploadpack.allowsidebandall" "true" &&
1141 +
1142 + echo my-blob >"$P/my-blob" &&
1143 + git -C "$P" add my-blob &&
1144 + git -C "$P" commit -m x &&
1145 +
1146 + git -C "$P" hash-object my-blob >objh &&
1147 + git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
1148 + git -C "$P" config --add \
1149 + "uploadpack.blobpackfileuri" \
1150 + "$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
1151 +
1152 + GIT_TRACE_PACKET="$(pwd)/log" \
1153 + GIT_TRACE_REDACT=0 \
1154 + git -c protocol.version=2 \
1155 + -c fetch.uriprotocols=http,https \
1156 + clone "$HTTPD_URL/smart/http_parent" http_child &&
1157 +
1158 + grep -F "clone< \\1$(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" log
1159 +'
1160 +
1161 test_expect_success 'http:// --negotiate-only' '
1162 SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" &&
1163 URI="$HTTPD_URL/smart/server" &&