{"thread":{"id":"2152","subject":"[PATCH] git-daemon: timeout, eliminate double DWIM","startedAt":"2005-10-19T18:54:34Z","lastAt":"2005-10-20T01:26:15Z","messageCount":4,"participants":["H. Peter Anvin","Petr Baudis","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"10289","messageId":"4356966A.8010401@zytor.com","threadId":"2152","inReplyTo":null,"subject":"[PATCH] git-daemon: timeout, eliminate double DWIM","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-19T18:54:34Z","receivedAt":"2005-10-19T18:54:34Z","isPatch":true,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"It turns out that not only did git-daemon do DWIM, but git-upload-pack \ndoes as well. This is bad; security checks have to be performed *after* \ncanonicalization, not before.\n\nAdditionally, the current git-daemon can be trivially DoSed by spewing \nSYNs at the target port.\n\nThis patch adds a --strict option to git-upload-pack to disable all \nDWIM, a --timeout option to git-daemon and git-upload-pack, and an \n--init-timeout option to git-daemon (which is typically set to a much \nlower value, since the initial request should come immediately from the \nclient.)\n\nSigned-off-by: H. Peter Anvin \n\n\ndiff --git a/daemon.c b/daemon.c\n--- a/daemon.c\n+++ b/daemon.c\n@@ -13,7 +13,9 @@\n static int log_syslog;\n static int verbose;\n \n-static const char daemon_usage[] = \"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all] [directory...]\";\n+static const char daemon_usage[] =\n+\"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\\n\"\n+\" [--timeout=n] [--init-timeout=n] [directory...]\";\n \n /* List of acceptable pathname prefixes */\n static char **ok_paths = NULL;\n@@ -21,6 +23,9 @@ static char **ok_paths = NULL;\n /* If this is set, git-daemon-export-ok is not required */\n static int export_all_trees = 0;\n \n+/* Timeout, and initial timeout */\n+static unsigned int timeout = 0;\n+static unsigned int init_timeout = 0;\n \n static void logreport(int priority, const char *err, va_list params)\n {\n@@ -170,6 +175,8 @@ static int upload(char *dir)\n \t/* Enough for the longest path above including final null */\n \tint buflen = strlen(dir)+10;\n \tchar *dirbuf = xmalloc(buflen);\n+\t/* Timeout as string */\n+\tchar timeout_buf[64];\n \n \tloginfo(\"Request for '%s'\", dir);\n \n@@ -190,8 +197,10 @@ static int upload(char *dir)\n \t */\n \tsignal(SIGTERM, SIG_IGN);\n \n+\tsnprintf(timeout_buf, sizeof timeout_buf, \"--timeout=%u\", timeout);\n+\n \t/* git-upload-pack only ever reads stuff, so this is safe */\n-\texeclp(\"git-upload-pack\", \"git-upload-pack\", \".\", NULL);\n+\texeclp(\"git-upload-pack\", \"git-upload-pack\", \"--strict\", timeout_buf, \".\", NULL);\n \treturn -1;\n }\n \n@@ -200,7 +209,9 @@ static int execute(void)\n \tstatic char line[1000];\n \tint len;\n \n+\talarm(init_timeout ? init_timeout : timeout);\n \tlen = packet_read_line(0, line, sizeof(line));\n+\talarm(0);\n \n \tif (len && line[len-1] == '\\n')\n \t\tline[--len] = 0;\n@@ -598,6 +609,12 @@ int main(int argc, char **argv)\n \t\t\texport_all_trees = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"--timeout=\")) {\n+\t\t\ttimeout = atoi(arg+10);\n+\t\t}\n+\t\tif (!strncmp(arg, \"--init-timeout=\")) {\n+\t\t\tinit_timeout = atoi(arg+15);\n+\t\t}\n \t\tif (!strcmp(arg, \"--\")) {\n \t\t\tok_paths = &argv[i+1];\n \t\t\tbreak;\ndiff --git a/upload-pack.c b/upload-pack.c\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -4,13 +4,19 @@\n #include \"tag.h\"\n #include \"object.h\"\n \n-static const char upload_pack_usage[] = \"git-upload-pack \";\n+static const char upload_pack_usage[] = \"git-upload-pack [--strict] [--timeout=nn] \";\n \n-#define MAX_HAS (16)\n-#define MAX_NEEDS (256)\n+#define MAX_HAS 64\n+#define MAX_NEEDS 4096\n static int nr_has = 0, nr_needs = 0;\n static unsigned char has_sha1[MAX_HAS][20];\n static unsigned char needs_sha1[MAX_NEEDS][20];\n+static unsigned int timeout = 0;\n+\n+static void reset_timeout(void)\n+{\n+\talarm(timeout);\n+}\n \n static int strip(char *line, int len)\n {\n@@ -100,6 +106,7 @@ static int get_common_commits(void)\n \n \tfor(;;) {\n \t\tlen = packet_read_line(0, line, sizeof(line));\n+\t\treset_timeout();\n \n \t\tif (!len) {\n \t\t\tpacket_write(1, \"NAK\\n\");\n@@ -122,6 +129,7 @@ static int get_common_commits(void)\n \n \tfor (;;) {\n \t\tlen = packet_read_line(0, line, sizeof(line));\n+\t\treset_timeout();\n \t\tif (!len)\n \t\t\tcontinue;\n \t\tlen = strip(line, len);\n@@ -145,6 +153,7 @@ static int receive_needs(void)\n \tfor (;;) {\n \t\tunsigned char dummy[20], *sha1_buf;\n \t\tlen = packet_read_line(0, line, sizeof(line));\n+\t\treset_timeout();\n \t\tif (!len)\n \t\t\treturn needs;\n \n@@ -179,6 +188,7 @@ static int send_ref(const char *refname,\n \n static int upload_pack(void)\n {\n+\treset_timeout();\n \thead_ref(send_ref);\n \tfor_each_ref(send_ref);\n \tpacket_flush(1);\n@@ -193,18 +203,43 @@ static int upload_pack(void)\n int main(int argc, char **argv)\n {\n \tconst char *dir;\n-\tif (argc != 2)\n+\tint i;\n+\tint strict = 0;\n+\n+\tfor (i = 1; i < argc; i++) {\n+\t\tchar *arg = argv[i];\n+\n+\t\tif (arg[0] != '-')\n+\t\t\tbreak;\n+\t\tif (!strcmp(arg, \"--strict\")) {\n+\t\t\tstrict = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strncmp(arg, \"--timeout=\")) {\n+\t\t\ttimeout = atoi(arg+10);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(arg, \"--\")) {\n+\t\t\ti++;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\t\n+\tif (i != argc-1)\n \t\tusage(upload_pack_usage);\n-\tdir = argv[1];\n+\tdir = argv[i];\n \n \t/* chdir to the directory. If that fails, try appending \".git\" */\n \tif (chdir(dir) < 0) {\n-\t\tif (chdir(mkpath(\"%s.git\", dir)) < 0)\n+\t\tif (strict || chdir(mkpath(\"%s.git\", dir)) < 0)\n \t\t\tdie(\"git-upload-pack unable to chdir to %s\", dir);\n \t}\n-\tchdir(\".git\");\n+\tif (!strict)\n+\t\tchdir(\".git\");\n+\n \tif (access(\"objects\", X_OK) || access(\"refs\", X_OK))\n \t\tdie(\"git-upload-pack: %s doesn't seem to be a git archive\", dir);\n+\n \tputenv(\"GIT_DIR=.\");\n \tupload_pack();\n \treturn 0;\n"},{"id":"10321","messageId":"20051020002845.GT30889@pasky.or.cz","threadId":"2152","inReplyTo":"4356966A.8010401@zytor.com","subject":"Re: [PATCH] git-daemon: timeout, eliminate double DWIM","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-10-20T00:28:45Z","receivedAt":"2005-10-20T00:28:45Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Wed, Oct 19, 2005 at 08:54:34PM CEST, I got a letter\nwhere \"H. Peter Anvin\" told me that...\n> diff --git a/daemon.c b/daemon.c\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -13,7 +13,9 @@\n> static int log_syslog;\n> static int verbose;\n> \n> -static const char daemon_usage[] = \"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all] [directory...]\";\n> +static const char daemon_usage[] =\n> +\"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\\n\"\n> +\" [--timeout=n] [--init-timeout=n] [directory...]\";\n> \n> /* List of acceptable pathname prefixes */\n> static char **ok_paths = NULL;\n\nYou didn't update Documentation/git-daemon.txt.\n\n> diff --git a/upload-pack.c b/upload-pack.c\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -4,13 +4,19 @@\n> #include \"tag.h\"\n> #include \"object.h\"\n> \n> -static const char upload_pack_usage[] = \"git-upload-pack \";\n> +static const char upload_pack_usage[] = \"git-upload-pack [--strict] [--timeout=nn] \";\n\nDitto.\n\n\nAfter being confronted with incomplete documentation again just minutes\nago (will send patch soon), I think I'm going to start to be annoying\nand watch patches for this issue specifically. ;-)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"},{"id":"10325","messageId":"7vsluxvvdc.fsf@assigned-by-dhcp.cox.net","threadId":"2152","inReplyTo":"20051020002845.GT30889@pasky.or.cz","subject":"Re: [PATCH] git-daemon: timeout, eliminate double DWIM","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-20T01:18:39Z","receivedAt":"2005-10-20T01:18:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> You didn't update Documentation/git-daemon.txt.\n>...\n> Ditto.\n\nPatches welcome.\n"},{"id":"10326","messageId":"20051020012615.GU30889@pasky.or.cz","threadId":"2152","inReplyTo":"7vsluxvvdc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-daemon: timeout, eliminate double DWIM","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-10-20T01:26:15Z","receivedAt":"2005-10-20T01:26:15Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Thu, Oct 20, 2005 at 03:18:39AM CEST, I got a letter\nwhere Junio C Hamano told me that...\n> Petr Baudis writes:\n> \n> > You didn't update Documentation/git-daemon.txt.\n> >...\n> > Ditto.\n> \n> Patches welcome.\n\nAh, I didn't notice it was already merged, sorry.\n\n\nBTW, some of the commits in your tree have the same author and\ncommitter date while they shouldn't:\n\nauthor H. Peter Anvin Wed, 19 Oct 2005 14:27:01 -0700\ncommitter Junio C Hamano Wed, 19 Oct 2005 14:27:01 -0700\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"}]}