Changes since v4:
* Use "uri" instead of "url"
* Look specifically for a line with packfile-uri format (instead of for a
URL in general)
* Limit the redacting to the packfile-uri section in do_fetch_pack_v2
* Use "%.*s" instead of duplicating parts of the string to print
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 | 4 ++++
http-fetch.c | 14 ++++++++++--
pkt-line.c | 39 +++++++++++++++++++++++++++++++-
pkt-line.h | 1 +
t/t5702-protocol-v2.sh | 51 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 109 insertions(+), 5 deletions(-)
base-commit: e9e5ba39a78c8f5057262d49e261b42a8660d5b9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1052%2Fifradeo%2Fredact-packfile-uri-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1052/ifradeo/redact-packfile-uri-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1052
Range-diff vs v4:
1: 973a250752c ! 1: c95b3cafcd6 fetch-pack: redact packfile urls in traces
@@ Documentation/git.txt: for full details.
- 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
++ header and packfile URIs. Set this variable to `0` to prevent this
+ redaction.
`GIT_LITERAL_PATHSPECS`::
@@ Documentation/git.txt: for full details.
## fetch-pack.c ##
@@ fetch-pack.c: static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
- reader.me = "fetch-pack";
- }
+ receive_wanted_refs(&reader, sought, nr_sought);
-+ 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:
+ /* 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);
++ reader.options &= ~PACKET_READ_REDACT_URI_PATH;
++
+ process_section_header(&reader, "packfile", 0);
+
+ /*
## 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 char *find_url_path(const char* buffer, int *path_len)
++static char *find_packfile_uri_path(const char *buffer)
+{
-+ const char *URL_MARK = "://";
-+ char *path = strstr(buffer, URL_MARK);
-+ if (!path)
-+ return NULL;
++ const char *URI_MARK = "://";
++ char *path;
++ int len;
+
-+ path += strlen(URL_MARK);
-+ while (*path && *path != '/')
-+ path++;
++ /* First char is sideband mark */
++ buffer += 1;
+
-+ if (!*path || !*(path + 1))
-+ return NULL;
++ len = strspn(buffer, "0123456789abcdefABCDEF");
++ if (!(len == 40 || len == 64) || buffer[len] != ' ')
++ return NULL; /* required "<hash>SP" not seen */
+
-+ // position after '/'
-+ path++;
++ path = strstr(buffer + len + 1, URI_MARK);
++ if (!path)
++ return NULL;
+
-+ if (path_len) {
-+ char *url_end = strchrnul(path, ' ');
-+ *path_len = url_end - path;
-+ }
++ path = strchr(path + strlen(URI_MARK), '/');
++ if (!path || !*(path + 1))
++ return NULL;
+
-+ return path;
++ /* position after '/' */
++ 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];
-+ char *url_path_start;
-+ int url_path_len;
++ char *uri_path_start;
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(buffer, &url_path_len))) {
++ 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, url_path_start - buffer,
-+ url_path_len, redacted, strlen(redacted));
++ 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 {
@@ pkt-line.h: 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)
++#define PACKET_READ_REDACT_URI_PATH (1u<<4)
int packet_read(int fd, char *buffer, unsigned size, int options);
/*
2: c7f0977cabd ! 2: 6912a690197 http-fetch: redact url on die() message
@@ Commit message
http-fetch: redact url on die() message
http-fetch prints the URL after failing to fetch it. This can be
- confusing to users (they cannot really do anything with it) but even
- worse, they can share by accident a sensitive URL (e.g. with
- credentials) while looking for help.
+ confusing to users (they cannot really do anything with it), and they
+ can share by accident a sensitive URL (e.g. with credentials) while
+ looking for help.
Redact the URL unless the GIT_TRACE_REDACT variable is set to false. This
mimics the redaction of other sensitive information in git, like the
Authorization header in HTTP.
+ Fix also capitalization of previous die() message (must start in
+ lowercase).
+
Signed-off-by: Ivan Frade <ifrade@google.com>
## http-fetch.c ##
@@ http-fetch.c: static void fetch_single_packfile(struct object_id *packfile_hash,
- curl_errorstr);
+ struct url_info url;
+ char *nurl = url_normalize(preq->url, &url);
-+ if (!git_env_bool("GIT_TRACE_REDACT", 1) || !nurl) {
-+ die("Unable to get pack file %s\n%s", preq->url,
++ if (!nurl || !git_env_bool("GIT_TRACE_REDACT", 1)) {
++ die("unable to get pack file '%s'\n%s", preq->url,
+ curl_errorstr);
+ } else {
-+ char *schema = xstrndup(url.url, url.scheme_len);
-+ char *host = xstrndup(&url.url[url.host_off], url.host_len);
-+ die("failed to get '%s' url from '%s' "
++ die("failed to get '%.*s' url from '%.*s' "
+ "(full URL redacted due to GIT_TRACE_REDACT setting)\n%s",
-+ schema, host, curl_errorstr);
++ (int)url.scheme_len, url.url,
++ (int)url.host_len, &url.url[url.host_off], curl_errorstr);
+ }
}
} else {Ivan Frade via GitGitGadget <gitgitgadget@gmail.com>