{"thread":{"id":"2298","subject":"[PATCH 4/4] git-daemon support for user-relative paths.","startedAt":"2005-11-01T22:59:21Z","lastAt":"2005-11-02T09:39:15Z","messageCount":6,"participants":["Andreas Ericsson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"10954","messageId":"20051101225921.3E7455BF74@nox.op5.se","threadId":"2298","inReplyTo":null,"subject":"[PATCH 4/4] git-daemon support for user-relative paths.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-01T22:59:21Z","receivedAt":"2005-11-01T22:59:21Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Dropped a fair amount of redundant functionality in favour of the library\ncode in path.c. git-upload-pack is now called with canonicalized paths.\n\nAdded option --strict-paths to retain old behaviour (documented).\n\nSigned-off-by: Andreas Ericsson <ae@op5.se>\n\n---\n\n Documentation/git-daemon.txt |   17 +++++-\n daemon.c                     |  128 ++++++++++++++----------------------------\n 2 files changed, 58 insertions(+), 87 deletions(-)\n\napplies-to: c40ce0cb704af8d7d1d655bcf709e7621db60e0e\ne35eb905a06718894a6c79f73d763122fff2a61a\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex 67c5f22..25f03df 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -8,7 +8,7 @@ git-daemon - A really simple server for \n SYNOPSIS\n --------\n 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n-             [--timeout=n] [--init-timeout=n] [directory...]\n+             [--timeout=n] [--init-timeout=n] [--strict-paths] [directory...]\n \n DESCRIPTION\n -----------\n@@ -29,10 +29,16 @@ This is ideally suited for read-only upd\n \n OPTIONS\n -------\n+--strict-paths::\n+\tMatch paths exactly (i.e. don't allow \"/foo/repo\" when the real path is\n+\t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n+\tgit-daemon will refuse to start when this option is enabled and no\n+\twhitelist is specified.\n+\n --export-all::\n \tAllow pulling from all directories that look like GIT repositories\n-\t(have the 'objects' subdirectory and a 'HEAD' file), even if they\n-\tdo not have the 'git-daemon-export-ok' file.\n+\t(have the 'objects' and 'refs' subdirectories as well as a 'HEAD' file),\n+\teven if they do not have the 'git-daemon-export-ok' file.\n \n --inetd::\n \tHave the server run as an inetd service.\n@@ -57,6 +63,11 @@ OPTIONS\n --verbose::\n \tLog details about the incoming connections and requested files.\n \n+<directory>::\n+\tA directory to add to the whitelist of allowed directory. Unless\n+\t--strict-paths is specified this will also include subdirectories\n+\tof each named directory.\n+\t\n Author\n ------\n Written by Linus Torvalds <torvalds@osdl.org> and YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>\ndiff --git a/daemon.c b/daemon.c\nindex c3f8641..7b36363 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -10,16 +10,18 @@\n #include \"pkt-line.h\"\n #include \"cache.h\"\n \n-static int log_syslog;\n+static int log_syslog = 0;\n static int verbose;\n \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+\"           [--timeout=n] [--init-timeout=n] [--strict-paths] [directory...]\";\n \n /* List of acceptable pathname prefixes */\n static char **ok_paths = NULL;\n \n+static int strict_paths = 0;\n+\n /* If this is set, git-daemon-export-ok is not required */\n static int export_all_trees = 0;\n \n@@ -81,69 +83,49 @@ static void loginfo(const char *err, ...\n \tva_end(params);\n }\n \n-static int path_ok(const char *dir)\n+static const char *path_ok(const char *dir)\n {\n-\tconst char *p = dir;\n-\tchar **pp;\n-\tint sl, ndot;\n-\n-\t/* The pathname here should be an absolute path. */\n-\tif ( *p++ != '/' )\n-\t\treturn 0;\n-\n-\tsl = 1;  ndot = 0;\n-\n-\tfor (;;) {\n-\t\tif ( *p == '.' ) {\n-\t\t\tndot++;\n-\t\t} else if ( *p == '\\0' ) {\n-\t\t\t/* Reject \".\" and \"..\" at the end of the path */\n-\t\t\tif ( sl && ndot > 0 && ndot < 3 )\n-\t\t\t\treturn 0;\n+\tconst char *path = is_git_repo(dir, strict_paths);\n \n-\t\t\t/* Otherwise OK */\n-\t\t\tbreak;\n-\t\t} else if ( *p == '/' ) {\n-\t\t\t/* Refuse \"\", \".\" or \"..\" */\n-\t\t\tif ( sl && ndot < 3 )\n-\t\t\t\treturn 0;\n-\t\t\tsl = 1;\n-\t\t\tndot = 0;\n-\t\t} else {\n-\t\t\tsl = ndot = 0;\n-\t\t}\n-\t\tp++;\n+\t/* No such directory or not a git archive */\n+\tif(!path) {\n+\t\tlogerror(\"'%s': unable to chdir or not a git archive\", dir);\n+\t\treturn NULL;\n \t}\n \n \tif ( ok_paths && *ok_paths ) {\n-\t\tint ok = 0;\n-\t\tint dirlen = strlen(dir);\n+\t\tchar **pp = NULL;\n+\t\tint dirlen = strlen(path);\n \n \t\tfor ( pp = ok_paths ; *pp ; pp++ ) {\n \t\t\tint len = strlen(*pp);\n-\t\t\tif ( len <= dirlen &&\n-\t\t\t     !strncmp(*pp, dir, len) &&\n-\t\t\t     (dir[len] == '/' || dir[len] == '\\0') ) {\n-\t\t\t\tok = 1;\n-\t\t\t\tbreak;\n+\t\t\tif ( len <= dirlen && !strncmp(*pp, path, len) ) {\n+\t\t\t\tif( path[len] == '\\0' || (!strict_paths && path[len] == '/') )\n+\t\t\t\t\treturn path;\n \t\t\t}\n \t\t}\n-\n-\t\tif ( !ok )\n-\t\t\treturn 0; /* Path not in whitelist */\n+\t}\n+\telse {\n+\t\t/* Be backwards compatible */\n+\t\tif(!strict_paths)\n+\t\t\treturn path;\n \t}\n \n-\treturn 1;\t\t/* Path acceptable */\n+\tlogerror(\"'%s': not in whitelist\", path);\n+\treturn NULL;\t\t/* Fallthrough. Deny by default */\n }\n \n-static int set_dir(const char *dir)\n+static int upload(char *dir)\n {\n-\tif (!path_ok(dir)) {\n-\t\terrno = EACCES;\n-\t\treturn -1;\n-\t}\n+\t/* Timeout as string */\n+\tchar timeout_buf[64];\n+\tconst char *path;\n+\n+\tloginfo(\"Request for '%s'\", dir);\n \n-\tif ( chdir(dir) )\n+\t/* path_ok() canonicalizes the path, chdir()'s\n+\t * and does whitelist checking */\n+\tif(!(path = path_ok(dir)))\n \t\treturn -1;\n \n \t/*\n@@ -152,45 +134,16 @@ static int set_dir(const char *dir)\n \t * We want a readable HEAD, usable \"objects\" directory, and\n \t * a \"git-daemon-export-ok\" flag that says that the other side\n \t * is ok with us doing this.\n+\t *\n+\t * path_ok() uses is_git_repo() though, so we only need to\n+\t * check if the path is exported.\n \t */\n \tif (!export_all_trees && access(\"git-daemon-export-ok\", F_OK)) {\n+\t\tlogerror(\"'%s': not exported\", path);\n \t\terrno = EACCES;\n \t\treturn -1;\n \t}\n \n-\tif (access(\"objects/\", X_OK) || access(\"HEAD\", R_OK)) {\n-\t\terrno = EINVAL;\n-\t\treturn -1;\n-\t}\n-\n-\t/* If all this passed, we're OK */\n-\treturn 0;\n-}\n-\n-static int upload(char *dir)\n-{\n-\t/* Try paths in this order */\n-\tstatic const char *paths[] = { \"%s\", \"%s/.git\", \"%s.git\", \"%s.git/.git\", NULL };\n-\tconst char **pp;\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-\tfor ( pp = paths ; *pp ; pp++ ) {\n-\t\tsnprintf(dirbuf, buflen, *pp, dir);\n-\t\tif ( !set_dir(dirbuf) )\n-\t\t\tbreak;\n-\t}\n-\n-\tif ( !*pp ) {\n-\t\tlogerror(\"Cannot set directory '%s': %s\", dir, strerror(errno));\n-\t\treturn -1;\n-\t}\n-\n \t/*\n \t * We'll ignore SIGTERM from now on, we have a\n \t * good client.\n@@ -200,7 +153,7 @@ static int upload(char *dir)\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\", \"--strict\", timeout_buf, \".\", NULL);\n+\texeclp(\"git-upload-pack\", \"git-upload-pack\", \"--strict\", timeout_buf, path, NULL);\n \treturn -1;\n }\n \n@@ -216,7 +169,7 @@ static int execute(void)\n \tif (len && line[len-1] == '\\n')\n \t\tline[--len] = 0;\n \n-\tif (!strncmp(\"git-upload-pack /\", line, 17))\n+\tif (!strncmp(\"git-upload-pack \", line, 16))\n \t\treturn upload(line+16);\n \n \tlogerror(\"Protocol error: '%s'\", line);\n@@ -615,6 +568,10 @@ int main(int argc, char **argv)\n \t\tif (!strncmp(arg, \"--init-timeout=\", 15)) {\n \t\t\tinit_timeout = atoi(arg+15);\n \t\t}\n+\t\tif (!strcmp(arg, \"--strict-paths\")) {\n+\t\t\tstrict_paths = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--\")) {\n \t\t\tok_paths = &argv[i+1];\n \t\t\tbreak;\n@@ -626,6 +583,9 @@ int main(int argc, char **argv)\n \t\tusage(daemon_usage);\n \t}\n \n+\tif(strict_paths && (!ok_paths || !*ok_paths))\n+\t\tdie(\"git-daemon: option --strict-paths requires a whitelist\");\n+\n \tif (inetd_mode) {\n \t\tfclose(stderr); //FIXME: workaround\n \t\treturn execute();\n---\n0.99.9.GIT\n"},{"id":"10971","messageId":"7vvezb6h4c.fsf@assigned-by-dhcp.cox.net","threadId":"2298","inReplyTo":"20051101225921.3E7455BF74@nox.op5.se","subject":"Re: [PATCH 4/4] git-daemon support for user-relative paths.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-02T00:14:11Z","receivedAt":"2005-11-02T00:14:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Ericsson <ae@op5.se> writes:\n\n> +             [--timeout=n] [--init-timeout=n] [--strict-paths] [directory...]\n\nWhy not just --strict?\n\n> @@ -10,16 +10,18 @@\n>  #include \"pkt-line.h\"\n>  #include \"cache.h\"\n>  \n> -static int log_syslog;\n> +static int log_syslog = 0;\n\nI'd drop this.\n\n>  /* If this is set, git-daemon-export-ok is not required */\n>  static int export_all_trees = 0;\n>  \n> @@ -81,69 +83,49 @@ static void loginfo(const char *err, ...\n>  \tva_end(params);\n>  }\n>  \n> -static int path_ok(const char *dir)\n> +static const char *path_ok(const char *dir)\n>  {\n> +\tconst char *path = is_git_repo(dir, strict_paths);\n> +\t/* No such directory or not a git archive */\n> +\tif(!path) {\n> +\t\tlogerror(\"'%s': unable to chdir or not a git archive\", dir);\n> +\t\treturn NULL;\n>  \t}\n>  \n>  \tif ( ok_paths && *ok_paths ) {\n> +\t\tchar **pp = NULL;\n> +\t\tint dirlen = strlen(path);\n>  \n>  \t\tfor ( pp = ok_paths ; *pp ; pp++ ) {\n>  \t\t\tint len = strlen(*pp);\n> +\t\t\tif ( len <= dirlen && !strncmp(*pp, path, len) ) {\n> +\t\t\t\tif( path[len] == '\\0' || (!strict_paths && path[len] == '/') )\n> +\t\t\t\t\treturn path;\n>  \t\t\t}\n>  \t\t}\n\nSorry, but I am a bit confused.  Does this mean that you need to\nlist all directories under --strict-paths, instead of saying\n\"/pub/scm and everything under it is OK\"?\n\nI like the general direction this set is taking, but let's let\nit simmer for a while.\n"},{"id":"10996","messageId":"4368760E.6030208@op5.se","threadId":"2298","inReplyTo":"7vvezb6h4c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/4] git-daemon support for user-relative paths.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-02T08:17:18Z","receivedAt":"2005-11-02T08:17:18Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andreas Ericsson <ae@op5.se> writes:\n> \n> \n>>+             [--timeout=n] [--init-timeout=n] [--strict-paths] [directory...]\n> \n> \n> Why not just --strict?\n> \n> \n>>@@ -10,16 +10,18 @@\n>> #include \"pkt-line.h\"\n>> #include \"cache.h\"\n>> \n>>-static int log_syslog;\n>>+static int log_syslog = 0;\n> \n> \n> I'd drop this.\n> \n\nNo can do. It has to be set either here or down in main. It's nice to \nhave the default in the declaration.\n\n> \n>> /* If this is set, git-daemon-export-ok is not required */\n>> static int export_all_trees = 0;\n>> \n>>@@ -81,69 +83,49 @@ static void loginfo(const char *err, ...\n>> \tva_end(params);\n>> }\n>> \n>>-static int path_ok(const char *dir)\n>>+static const char *path_ok(const char *dir)\n>> {\n>>+\tconst char *path = is_git_repo(dir, strict_paths);\n>>+\t/* No such directory or not a git archive */\n>>+\tif(!path) {\n>>+\t\tlogerror(\"'%s': unable to chdir or not a git archive\", dir);\n>>+\t\treturn NULL;\n>> \t}\n>> \n>> \tif ( ok_paths && *ok_paths ) {\n>>+\t\tchar **pp = NULL;\n>>+\t\tint dirlen = strlen(path);\n>> \n>> \t\tfor ( pp = ok_paths ; *pp ; pp++ ) {\n>> \t\t\tint len = strlen(*pp);\n>>+\t\t\tif ( len <= dirlen && !strncmp(*pp, path, len) ) {\n>>+\t\t\t\tif( path[len] == '\\0' || (!strict_paths && path[len] == '/') )\n>>+\t\t\t\t\treturn path;\n>> \t\t\t}\n>> \t\t}\n> \n> \n> Sorry, but I am a bit confused.  Does this mean that you need to\n> list all directories under --strict-paths, instead of saying\n> \"/pub/scm and everything under it is OK\"?\n> \n\nOnly if --strict-paths is set, otherwise it works as usual. It's \ndocumented in the man-page, but perhaps it's a bit too paranoid.\n\n> I like the general direction this set is taking, but let's let\n> it simmer for a while.\n> \n\nOk. I'll take that to mean \"hold off on the --server-root and --userdir \npatch for a while\" then.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"10998","messageId":"43687717.6080404@op5.se","threadId":"2298","inReplyTo":"7vvezb6h4c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/4] git-daemon support for user-relative paths.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-02T08:21:43Z","receivedAt":"2005-11-02T08:21:43Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andreas Ericsson <ae@op5.se> writes:\n> \n> \n>>+             [--timeout=n] [--init-timeout=n] [--strict-paths] [directory...]\n> \n> \n> Why not just --strict?\n> \n\nIt's a bit clearer what it is that's strict, and allows for future \n--strict-* options to be added without breaking anyones init-scripts.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"11008","messageId":"7v1x1zv1ln.fsf@assigned-by-dhcp.cox.net","threadId":"2298","inReplyTo":"4368760E.6030208@op5.se","subject":"Re: [PATCH 4/4] git-daemon support for user-relative paths.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-02T09:30:12Z","receivedAt":"2005-11-02T09:30:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Ericsson <ae@op5.se> writes:\n\n> Junio C Hamano wrote:\n>>> -static int log_syslog;\n>>>+static int log_syslog = 0;\n>> I'd drop this.\n>\n> No can do. It has to be set either here or down in main. It's nice to \n> have the default in the declaration.\n\nIsn't \"static int log_syslog\" in BSS to be initialized to zero anyway?\n\n>> I like the general direction this set is taking, but let's let\n>> it simmer for a while.\n>\n> Ok. I'll take that to mean \"hold off on the --server-root and --userdir \n> patch for a while\" then.\n\nI do not mind keeping it in the proposed updates branch for\npeople to see and experiment with, but not in the master branch,\nat least for now.\n"},{"id":"11009","messageId":"43688943.4010002@op5.se","threadId":"2298","inReplyTo":"7v1x1zv1ln.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/4] git-daemon support for user-relative paths.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-02T09:39:15Z","receivedAt":"2005-11-02T09:39:15Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andreas Ericsson <ae@op5.se> writes:\n> \n> \n>>Junio C Hamano wrote:\n>>\n>>>>-static int log_syslog;\n>>>>+static int log_syslog = 0;\n>>>\n>>>I'd drop this.\n>>\n>>No can do. It has to be set either here or down in main. It's nice to \n>>have the default in the declaration.\n> \n> \n> Isn't \"static int log_syslog\" in BSS to be initialized to zero anyway?\n> \n\nI would have thought so, but it wasn't when I tested it. This could be a \ncompiler-bug in my end, I suppose, but spelling it out enhances \nreadability and leaves no room for doubt which is always nice.\n\n>>\n>>Ok. I'll take that to mean \"hold off on the --server-root and --userdir \n>>patch for a while\" then.\n> \n> \n> I do not mind keeping it in the proposed updates branch for\n> people to see and experiment with, but not in the master branch,\n> at least for now.\n> \n\nI'll cook it up then.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"}]}