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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx
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.
#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@xxxxxxx:port/path/to/repo.git/";,
				"http://host.xz:port/path/to/repo.git/";,
		},
		{
			"https://user@xxxxxxx:port/path/to/repo.git/";,
				"https://host.xz:port/path/to/repo.git/";,
		},
		{
			"ssh://user@xxxxxxx:port/path/to/repo.git/",
				"ssh://host.xz:port/path/to/repo.git/",
		},
		{
			"ssh://user@xxxxxxx/path/to/repo.git/",
				"ssh://host.xz/path/to/repo.git/",
		},
		{
			"ssh://user@xxxxxxx/~user/path/to/repo.git/",
				"ssh://host.xz/~user/path/to/repo.git/",
		},
		{
			"user@xxxxxxx:/path/to/repo.git/",
				"host.xz:/path/to/repo.git/",
		},
		{
			"user@xxxxxxx:~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;
}

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]