Re: [PATCH] fetch: Strip usernames from url's before storing them

[Andreas Ericsson <ae@op5.se>]

[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]

Michael J Gruber wrote:
> Andreas Ericsson venit, vidit, dixit 15.04.2009 14:16:
>> When pulling from a remote, the full URL including username
>> is by default added to the commit message. Since it adds
>> very little value but could be used by malicious people to
>> glean valid usernames (with matching hostnames), we're far
>> better off just stripping the username before storing the
>> remote URL locally.
> 
> Uhm, this is for non-fast-forwards when pull uses "merge" and creates a
> merge commit, right?
> Fetch does not create commit messages, and pull does not either if it
> rebases. So maybe the commit message could make it clearer for lesser
> git-educated people such as myself ;)
> 

Yes and no. This alters what gets written to .git/FETCH_HEAD, but since
what's written there only ever turns up in the history in the form of a
commit-message, you're essentially right.

The reason for this patch is that we published some repositories publicly
a week or two ago and one such malicious person started attacking all our
public servers with the usernames found in the commit messages. In our
case, this isn't such a big issue since all of the servers just fake an
SSH daemon when connected to from outside our internal network, but we
shouldn't take too lightly on casual information disclosure. It *could*
have been a problem and, if nothing else, this patch will probably save
some diskspace if it can prevent others being targeted by the same kind
of brute-force attack as we were.

Junio, this is based off of master, but applies cleanly to maint as well.
I'd actually prefer it to go on maint than master. The usernames in the
url's provide no real value but are potentially dangerous to disclose.

Attached is the micro-program I wrote to test the function itself.
Since I couldn't quite figure out how to set up a remote repository with
password protection and then fetch from it in a way that was generic
enough to go into the test-suite I didn't bother with that, but issuing
a git-pull shows that FETCH_HEAD and the commit message gets the correct
text.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

[-- Attachment #2: anon-url.c --]
[-- Type: text/x-csrc, Size: 2332 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define prefixcmp(haystack, needle) strncmp(haystack, needle, strlen(needle))
#define xcalloc(n, size) calloc(n, size)
#define xstrdup(str) strdup(str)

static char *anonymize_url(const char *url)
{
	char *anon_url;
	const char *at_sign = strchr(url, '@');
	size_t len, prefix_len = 0;

	if (!at_sign)
		return xstrdup(url);

	if (!prefixcmp(url, "ssh://"))
		prefix_len = strlen("ssh://");
	else if (!prefixcmp(url, "http://"))
		prefix_len = strlen("http://");
	else if (!prefixcmp(url, "https://"))
		prefix_len = strlen("https://");
	else if (!strchr(at_sign + 1, ':'))
		return xstrdup(url);

	len = prefix_len + strlen(at_sign + 1);
	anon_url = xcalloc(1, 1 + prefix_len + strlen(at_sign + 1));
	if (prefix_len)
		memcpy(anon_url, url, prefix_len);
	memcpy(anon_url + prefix_len, at_sign + 1, strlen(at_sign + 1));

	return anon_url;
}

int main(int argc, char **argv)
{
	int errors = 0;
	struct {
		char *raw;
		char *correct;
	}
	urls[] = {
		{ "rsync://host.xz/path/to/repo.git/", NULL, },
		{ "http://host.xz:port/path/to/repo.git/", NULL, },
		{ "https://host.xz:port/path/to/repo.git/", NULL,},
		{ "git://host.xz:port/path/to/repo.git/", NULL, },
		{ "git://host.xz:port/~user/path/to/repo.git/", NULL, },
		{
			"http://user@host.xz:port/path/to/repo.git/",
				"http://host.xz:port/path/to/repo.git/",
		},
		{
			"https://user@host.xz:port/path/to/repo.git/",
				"https://host.xz:port/path/to/repo.git/",
		},
		{
			"ssh://user@host.xz:port/path/to/repo.git/",
				"ssh://host.xz:port/path/to/repo.git/",
		},
		{
			"ssh://user@host.xz/path/to/repo.git/",
				"ssh://host.xz/path/to/repo.git/",
		},
		{
			"ssh://user@host.xz/~user/path/to/repo.git/",
				"ssh://host.xz/~user/path/to/repo.git/",
		},
		{
			"user@host.xz:/path/to/repo.git/",
				"host.xz:/path/to/repo.git/",
		},
		{
			"user@host.xz:~user/path/to/repo.git/",
				"host.xz:~user/path/to/repo.git/",
		},
		{ NULL, NULL },
	};
	int i;

	for (i = 0; urls[i].raw; i++) {
		char *anon_url = anonymize_url(urls[i].raw);
		if (!strcmp(anon_url, urls[i].correct ? urls[i].correct : urls[i].raw))
			continue;

		errors++;
		printf("raw    : %s\nanon   : %s\n", urls[i].raw, anon_url);
		printf("correct: %s\n", urls[i].correct);
	}

	printf("There were %d errors\n", errors);
	return 0;
}