git-daemon: timeout, eliminate double DWIM
PATCH 1/1 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.)
daemon.c +19 -2
@@ -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=" )) { + timeout =atoi (arg+10 ); + } +if (!strncmp (arg,"--init-timeout=" )) { + init_timeout =atoi (arg+15 ); + }if (!strcmp (arg,"--" )) { ok_paths = &argv[i+1 ];break ;
upload-pack.c +42 -7
97 lines not on this page · raw message
Dear diary, on Wed, Oct 19, 2005 at 08:54:34PM CEST, I got a letter where "H. Peter Anvin" <[email protected]> told me that...
Quoted 4 lines
daemon.c line 21
Quoted 10 lines
daemon.c +3
-1@@ -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 ;
You didn't update Documentation/git-daemon.txt.
upload-pack.c line 7
Quoted 9 lines
upload-pack.c +1
-1@@ -4,13 +4,19 @@#include "tag.h" #include "object.h" -static const char upload_pack_usage[] ="git-upload-pack <dir>" ; +static const char upload_pack_usage[] ="git-upload-pack [--strict] [--timeout=nn] <dir>" ;
Ditto.
After being confronted with incomplete documentation again just minutes ago (will send patch soon), I think I'm going to start to be annoying and watch patches for this issue specifically. ;-)
Signature · 4 lines
Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ VI has two modes: the one in which it beeps and the one in which it doesn't.
Quoted 3 lines · Petr Baudis
You didn't update
Documentation/git-daemon.txt.
...
Ditto.
Patches welcome.
Dear diary, on Thu, Oct 20, 2005 at 03:18:39AM CEST, I got a letter where Junio C Hamano <[email protected]> told me that...
Quoted 7 lines
Quoted 3 lines · Petr Baudis
You didn't update
Documentation/git-daemon.txt.
...
Ditto.Patches welcome.
Ah, I didn't notice it was already merged, sorry.
BTW, some of the commits in your tree have the same author and committer date while they shouldn't:
author H. Peter Anvin <[email protected]> Wed, 19 Oct 2005 14:27:01 -0700 committer Junio C Hamano <[email protected]> Wed, 19 Oct 2005 14:27:01 -0700
Signature · 4 lines
Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ VI has two modes: the one in which it beeps and the one in which it doesn't.