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>

diff --git a/Documentation/git.txt b/Documentation/git.txt
index d63c65e67d825d6aac6181de79689557bd76a019..c91aa2737f023d868f96a802a6ca9fd628b9ea7c 100644 (file)
--- 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 URIs. 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 a9604f35a3ea9055732d48e39b63a39f041f18f3..8b8c75f33aaf7331f0da761a2b84582c85422350 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1653,8 +1653,13 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
                                receive_wanted_refs(&reader, sought, nr_sought);
 
                        /* get the pack(s) */
+                       if (git_env_bool("GIT_TRACE_REDACT", 1))
+                               reader.options |= PACKET_READ_REDACT_URI_PATH;
                        if (process_section_header(&reader, "packfile-uris", 1))
                                receive_packfile_uris(&reader, &packfile_uris);
+                       /* We don't expect more URIs. Reset to avoid expensive URI check. */
+                       reader.options &= ~PACKET_READ_REDACT_URI_PATH;
+
                        process_section_header(&reader, "packfile", 0);
 
                        /*

diff --git a/pkt-line.c b/pkt-line.c
index 2dc8ac274bd0c03b5612e73fb587dde911865f0d..8e43c2def4ca4fa470b7a318ffa8a060b5c39838 100644 (file)
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -370,6 +370,32 @@ int packet_length(const char lenbuf_hex[4])
        return (val < 0) ? val : (val << 8) | hex2chr(lenbuf_hex + 2);
 }
 
+static char *find_packfile_uri_path(const char *buffer)
+{
+       const char *URI_MARK = "://";
+       char *path;
+       int len;
+
+       /* First char is sideband mark */
+       buffer += 1;
+
+       len = strspn(buffer, "0123456789abcdefABCDEF");
+       /* size of SHA1 and SHA256 hash */
+       if (!(len == 40 || len == 64) || buffer[len] != ' ')
+               return NULL; /* required "<hash>SP" not seen */
+
+       path = strstr(buffer + len + 1, URI_MARK);
+       if (!path)
+               return NULL;
+
+       path = strchr(path + strlen(URI_MARK), '/');
+       if (!path || !*(path + 1))
+               return NULL;
+
+       /* position after '/' */
+       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 +403,7 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
 {
        int len;
        char linelen[4];
+       char *uri_path_start;
 
        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_URI_PATH &&
+           (uri_path_start = find_packfile_uri_path(buffer))) {
+               const char *redacted = "<redacted>";
+               struct strbuf tracebuf = STRBUF_INIT;
+               strbuf_insert(&tracebuf, 0, buffer, len);
+               strbuf_splice(&tracebuf, uri_path_start - buffer,
+                             strlen(uri_path_start), 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 467ae01357301ee503c6f8a49430c7dd08a9ccdd..6d2a63db2387fbc37287988f93b4d927422008e5 100644 (file)
--- 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_URI_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 d527cf6c49f9d864ca7c296515e7509caf5a0c34..78f85b0714acbc9064199cec25f634370a58d36f 100755 (executable)
--- 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_PACKET="$(pwd)/log" \
+       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_PACKET="$(pwd)/log" \
+       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" &&