From 960deccb26a5bee6c6cd63d50e8272f540a27b19 Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Wed, 19 Oct 2005 14:27:01 -0700 Subject: git-daemon: timeout, eliminate double DWIM It turns out that not only did git-daemon do DWIM, but git-upload-pack does as well. This is bad; security checks have to be performed *after* canonicalization, not before. Additionally, the current git-daemon can be trivially DoSed by spewing SYNs at the target port. This patch adds a --strict option to git-upload-pack to disable all DWIM, a --timeout option to git-daemon and git-upload-pack, and an --init-timeout option to git-daemon (which is typically set to a much lower value, since the initial request should come immediately from the client.) Signed-off-by: H. Peter Anvin Signed-off-by: Junio C Hamano diff --git a/daemon.c b/daemon.c index 9ea6c31..c3381b3 100644 --- a/daemon.c +++ b/daemon.c @@ -13,7 +13,9 @@ static int log_syslog; static int verbose; -static const char daemon_usage[] = "git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all] [directory...]"; +static const char daemon_usage[] = +"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n" +" [--timeout=n] [--init-timeout=n] [directory...]"; /* List of acceptable pathname prefixes */ static char **ok_paths = NULL; @@ -21,6 +23,9 @@ static char **ok_paths = NULL; /* If this is set, git-daemon-export-ok is not required */ static int export_all_trees = 0; +/* Timeout, and initial timeout */ +static unsigned int timeout = 0; +static unsigned int init_timeout = 0; static void logreport(int priority, const char *err, va_list params) { @@ -170,6 +175,8 @@ static int upload(char *dir) /* Enough for the longest path above including final null */ int buflen = strlen(dir)+10; char *dirbuf = xmalloc(buflen); + /* Timeout as string */ + char timeout_buf[64]; loginfo("Request for '%s'", dir); @@ -190,8 +197,10 @@ static int upload(char *dir) */ signal(SIGTERM, SIG_IGN); + snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout); + /* git-upload-pack only ever reads stuff, so this is safe */ - execlp("git-upload-pack", "git-upload-pack", ".", NULL); + execlp("git-upload-pack", "git-upload-pack", "--strict", timeout_buf, ".", NULL); return -1; } @@ -200,7 +209,9 @@ static int execute(void) static char line[1000]; int len; + alarm(init_timeout ? init_timeout : timeout); len = packet_read_line(0, line, sizeof(line)); + alarm(0); if (len && line[len-1] == '\n') line[--len] = 0; @@ -598,6 +609,12 @@ int main(int argc, char **argv) export_all_trees = 1; continue; } + if (!strncmp(arg, "--timeout=", 10)) { + timeout = atoi(arg+10); + } + if (!strncmp(arg, "--init-timeout=", 10)) { + init_timeout = atoi(arg+15); + } if (!strcmp(arg, "--")) { ok_paths = &argv[i+1]; break; diff --git a/upload-pack.c b/upload-pack.c index 21b4b8b..8a41caf 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -4,13 +4,19 @@ #include "tag.h" #include "object.h" -static const char upload_pack_usage[] = "git-upload-pack "; +static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] "; #define MAX_HAS (16) #define MAX_NEEDS (256) static int nr_has = 0, nr_needs = 0; static unsigned char has_sha1[MAX_HAS][20]; static unsigned char needs_sha1[MAX_NEEDS][20]; +static unsigned int timeout = 0; + +static void reset_timeout(void) +{ + alarm(timeout); +} static int strip(char *line, int len) { @@ -100,6 +106,7 @@ static int get_common_commits(void) for(;;) { len = packet_read_line(0, line, sizeof(line)); + reset_timeout(); if (!len) { packet_write(1, "NAK\n"); @@ -122,6 +129,7 @@ static int get_common_commits(void) for (;;) { len = packet_read_line(0, line, sizeof(line)); + reset_timeout(); if (!len) continue; len = strip(line, len); @@ -145,6 +153,7 @@ static int receive_needs(void) for (;;) { unsigned char dummy[20], *sha1_buf; len = packet_read_line(0, line, sizeof(line)); + reset_timeout(); if (!len) return needs; @@ -179,6 +188,7 @@ static int send_ref(const char *refname, const unsigned char *sha1) static int upload_pack(void) { + reset_timeout(); head_ref(send_ref); for_each_ref(send_ref); packet_flush(1); @@ -193,18 +203,43 @@ static int upload_pack(void) int main(int argc, char **argv) { const char *dir; - if (argc != 2) + int i; + int strict = 0; + + for (i = 1; i < argc; i++) { + char *arg = argv[i]; + + if (arg[0] != '-') + break; + if (!strcmp(arg, "--strict")) { + strict = 1; + continue; + } + if (!strncmp(arg, "--timeout=", 10)) { + timeout = atoi(arg+10); + continue; + } + if (!strcmp(arg, "--")) { + i++; + break; + } + } + + if (i != argc-1) usage(upload_pack_usage); - dir = argv[1]; + dir = argv[i]; /* chdir to the directory. If that fails, try appending ".git" */ if (chdir(dir) < 0) { - if (chdir(mkpath("%s.git", dir)) < 0) + if (strict || chdir(mkpath("%s.git", dir)) < 0) die("git-upload-pack unable to chdir to %s", dir); } - chdir(".git"); + if (!strict) + chdir(".git"); + if (access("objects", X_OK) || access("refs", X_OK)) die("git-upload-pack: %s doesn't seem to be a git archive", dir); + putenv("GIT_DIR=."); upload_pack(); return 0; -- cgit v0.10.2-6-g49f6