[v4,0/2] fetch-pack: redact packfile urls in traces

Message

Phillip Wood via GitGitGadget Oct. 26, 2021, 10:49 p.m. UTC

Changes since v3:

 * Enable redacting URLs for all sections
 * Redact only URL path (it was until the end of line)
 * Redact URL in die() with more friendly message
 * Update doc to mention that packfile URIs are also redacted.

Changes since v2:

 * Redact only the path of the URL
 * Test are now strict, validating the exact line expected in the log

Changes since v1:

 * Removed non-POSIX flags in tests
 * More accurate regex for the non-encrypted packfile line
 * Dropped documentation change
 * Dropped redacting the die message in http-fetch

Ivan Frade (2):
  fetch-pack: redact packfile urls in traces
  http-fetch: redact url on die() message

 Documentation/git.txt  |  5 +++--
 fetch-pack.c           |  3 +++
 http-fetch.c           | 15 +++++++++++--
 pkt-line.c             | 40 ++++++++++++++++++++++++++++++++-
 pkt-line.h             |  1 +
 t/t5702-protocol-v2.sh | 51 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 110 insertions(+), 5 deletions(-)


base-commit: e9e5ba39a78c8f5057262d49e261b42a8660d5b9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1052%2Fifradeo%2Fredact-packfile-uri-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1052/ifradeo/redact-packfile-uri-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1052

Range-diff vs v3:

 1:  9afe0093af4 ! 1:  973a250752c fetch-pack: redact packfile urls in traces
     @@ Commit message
      
          Signed-off-by: Ivan Frade <[e-mail]>
      
     - ## fetch-pack.c ##
     -@@ fetch-pack.c: static void receive_wanted_refs(struct packet_reader *reader,
     - static void receive_packfile_uris(struct packet_reader *reader,
     - 				  struct string_list *uris)
     - {
     -+	int saved_options;
     - 	process_section_header(reader, "packfile-uris", 0);
     -+	/*
     -+	 * In some setups, packfile-uris act as bearer tokens,
     -+	 * redact them by default.
     -+	 */
     -+	saved_options = reader->options;
     -+	if (git_env_bool("GIT_TRACE_REDACT", 1))
     -+		reader->options |= PACKET_READ_REDACT_URL_PATH;
     -+
     - 	while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
     - 		if (reader->pktlen < the_hash_algo->hexsz ||
     - 		    reader->line[the_hash_algo->hexsz] != ' ')
     -@@ fetch-pack.c: static void receive_packfile_uris(struct packet_reader *reader,
     + ## Documentation/git.txt ##
     +@@ Documentation/git.txt: for full details.
       
     - 		string_list_append(uris, reader->line);
     + `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
     +
     + ## fetch-pack.c ##
     +@@ fetch-pack.c: static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
     + 		reader.me = "fetch-pack";
       	}
     -+	reader->options = saved_options;
     + 
     ++	if (git_env_bool("GIT_TRACE_REDACT", 1))
     ++		reader.options |= PACKET_READ_REDACT_URL_PATH;
      +
     - 	if (reader->status != PACKET_READ_DELIM)
     - 		die("expected DELIM");
     - }
     + 	while (state != FETCH_DONE) {
     + 		switch (state) {
     + 		case FETCH_CHECK_LOCAL:
      
       ## pkt-line.c ##
      @@ pkt-line.c: int packet_length(const char lenbuf_hex[4])
       	return (val < 0) ? val : (val << 8) | hex2chr(lenbuf_hex + 2);
       }
       
     -+static int find_url_path_start(const char* buffer)
     ++static char *find_url_path(const char* buffer, int *path_len)
      +{
      +	const char *URL_MARK = "://";
     -+	char *p = strstr(buffer, URL_MARK);
     -+	if (!p) {
     -+		return -1;
     -+	}
     ++	char *path = strstr(buffer, URL_MARK);
     ++	if (!path)
     ++		return NULL;
      +
     -+	p += strlen(URL_MARK);
     -+	while (*p && *p != '/')
     -+		p++;
     ++	path += strlen(URL_MARK);
     ++	while (*path && *path != '/')
     ++		path++;
      +
     -+	// Position after '/'
     -+	if (*p && *(p + 1))
     -+		return (p + 1) - buffer;
     ++	if (!*path || !*(path + 1))
     ++		return NULL;
     ++
     ++	// position after '/'
     ++	path++;
     ++
     ++	if (path_len) {
     ++		char *url_end = strchrnul(path, ' ');
     ++		*path_len = url_end - path;
     ++	}
      +
     -+	return -1;
     ++	return path;
      +}
      +
       enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
     @@ pkt-line.c: enum packet_read_status packet_read_with_status(int fd, char **src_b
       {
       	int len;
       	char linelen[4];
     -+	int url_path_start;
     ++	char *url_path_start;
     ++	int url_path_len;
       
       	if (get_packet_data(fd, src_buffer, src_len, linelen, 4, options) < 0) {
       		*pktlen = -1;
     @@ pkt-line.c: enum packet_read_status packet_read_with_status(int fd, char **src_b
       	buffer[len] = 0;
      -	packet_trace(buffer, len, 0);
      +	if (options & PACKET_READ_REDACT_URL_PATH &&
     -+	    (url_path_start = find_url_path_start(buffer)) != -1) {
     ++	    (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,
     -+			      len - url_path_start, redacted, strlen(redacted));
     ++		strbuf_splice(&tracebuf, url_path_start - buffer,
     ++			      url_path_len, redacted, strlen(redacted));
      +		packet_trace(tracebuf.buf, tracebuf.len, 0);
      +		strbuf_release(&tracebuf);
      +	} else {
     @@ pkt-line.h: void packet_fflush(FILE *f);
       #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 **src_buffer, size_t *src_len, char
     - 		*buffer, unsigned size, int options);
     + int packet_read(int fd, char *buffer, unsigned size, int options);
       
     + /*
      
       ## t/t5702-protocol-v2.sh ##
      @@ t/t5702-protocol-v2.sh: test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodul
 -:  ----------- > 2:  c7f0977cabd http-fetch: redact url on die() message