From: Ivan Frade <ifrade@xxxxxxxxxx>
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@xxxxxxxxxx>
---
Documentation/git.txt | 5 +++--
fetch-pack.c | 3 +++
pkt-line.c | 40 ++++++++++++++++++++++++++++++++-
pkt-line.h | 1 +
t/t5702-protocol-v2.sh | 51 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 97 insertions(+), 3 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index d63c65e67d8..f64c8ce5183 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -832,8 +832,9 @@ for full details.
`GIT_TRACE_REDACT`::
By default, when tracing is activated, Git redacts the values of
- cookies, the "Authorization:" header, and the "Proxy-Authorization:"
- header. Set this variable to `0` to prevent this redaction.
+ cookies, the "Authorization:" header, the "Proxy-Authorization:"
+ header and packfile URLs. Set this variable to `0` to prevent this
+ redaction.
`GIT_LITERAL_PATHSPECS`::
Setting this variable to `1` will cause Git to treat all
diff --git a/fetch-pack.c b/fetch-pack.c
index a9604f35a3e..ad8ac49ca50 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1581,6 +1581,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
reader.me = "fetch-pack";
}
+ if (git_env_bool("GIT_TRACE_REDACT", 1))
+ reader.options |= PACKET_READ_REDACT_URL_PATH;
+
while (state != FETCH_DONE) {
switch (state) {
case FETCH_CHECK_LOCAL:
diff --git a/pkt-line.c b/pkt-line.c
index 2dc8ac274bd..ba0a2d65f0c 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -370,6 +370,31 @@ int packet_length(const char lenbuf_hex[4])
return (val < 0) ? val : (val << 8) | hex2chr(lenbuf_hex + 2);
}
+static char *find_url_path(const char* buffer, int *path_len)
+{
+ const char *URL_MARK = "://";
+ char *path = strstr(buffer, URL_MARK);
+ if (!path)
+ return NULL;
+
+ path += strlen(URL_MARK);
+ while (*path && *path != '/')
+ path++;
+
+ if (!*path || !*(path + 1))
+ return NULL;
+
+ // position after '/'
+ path++;
+
+ if (path_len) {
+ char *url_end = strchrnul(path, ' ');
+ *path_len = url_end - path;
+ }
+
+ return path;
+}
+
enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
size_t *src_len, char *buffer,
unsigned size, int *pktlen,
@@ -377,6 +402,8 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
{
int len;
char linelen[4];
+ char *url_path_start;
+ int url_path_len;
if (get_packet_data(fd, src_buffer, src_len, linelen, 4, options) < 0) {
*pktlen = -1;
@@ -427,7 +454,18 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
len--;
buffer[len] = 0;
- packet_trace(buffer, len, 0);
+ if (options & PACKET_READ_REDACT_URL_PATH &&
+ (url_path_start = find_url_path(buffer, &url_path_len))) {
+ const char *redacted = "<redacted>";
+ struct strbuf tracebuf = STRBUF_INIT;
+ strbuf_insert(&tracebuf, 0, buffer, len);
+ strbuf_splice(&tracebuf, url_path_start - buffer,
+ url_path_len, redacted, strlen(redacted));
+ packet_trace(tracebuf.buf, tracebuf.len, 0);
+ strbuf_release(&tracebuf);
+ } else {
+ packet_trace(buffer, len, 0);
+ }
if ((options & PACKET_READ_DIE_ON_ERR_PACKET) &&
starts_with(buffer, "ERR "))
diff --git a/pkt-line.h b/pkt-line.h
index 467ae013573..a610ecb88e8 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -87,6 +87,7 @@ void packet_fflush(FILE *f);
#define PACKET_READ_CHOMP_NEWLINE (1u<<1)
#define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
#define PACKET_READ_GENTLE_ON_READ_ERROR (1u<<3)
+#define PACKET_READ_REDACT_URL_PATH (1u<<4)
int packet_read(int fd, char *buffer, unsigned size, int options);
/*
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index d527cf6c49f..f01af2f2ed3 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -1107,6 +1107,57 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodul
test_i18ngrep "disallowed submodule name" err
'
+test_expect_success 'packfile-uri path redacted in trace' '
+ P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ rm -rf "$P" http_child log &&
+
+ git init "$P" &&
+ git -C "$P" config "uploadpack.allowsidebandall" "true" &&
+
+ echo my-blob >"$P/my-blob" &&
+ git -C "$P" add my-blob &&
+ git -C "$P" commit -m x &&
+
+ git -C "$P" hash-object my-blob >objh &&
+ git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
+ git -C "$P" config --add \
+ "uploadpack.blobpackfileuri" \
+ "$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
+
+ GIT_TRACE=1 GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \
+ git -c protocol.version=2 \
+ -c fetch.uriprotocols=http,https \
+ clone "$HTTPD_URL/smart/http_parent" http_child &&
+
+ grep -F "clone< \\1$(cat packh) $HTTPD_URL/<redacted>" log
+'
+
+test_expect_success 'packfile-uri path not redacted in trace when GIT_TRACE_REDACT=0' '
+ P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ rm -rf "$P" http_child log &&
+
+ git init "$P" &&
+ git -C "$P" config "uploadpack.allowsidebandall" "true" &&
+
+ echo my-blob >"$P/my-blob" &&
+ git -C "$P" add my-blob &&
+ git -C "$P" commit -m x &&
+
+ git -C "$P" hash-object my-blob >objh &&
+ git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
+ git -C "$P" config --add \
+ "uploadpack.blobpackfileuri" \
+ "$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
+
+ GIT_TRACE=1 GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \
+ GIT_TRACE_REDACT=0 \
+ git -c protocol.version=2 \
+ -c fetch.uriprotocols=http,https \
+ clone "$HTTPD_URL/smart/http_parent" http_child &&
+
+ grep -F "clone< \\1$(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" log
+'
+
test_expect_success 'http:// --negotiate-only' '
SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" &&
URI="$HTTPD_URL/smart/server" &&
--
gitgitgadget
