Re: [PATCH 3/3] daemon: Support a --user-path option.
- From
Junio C Hamano <junkio@cox.net>
- Date
- Feb 4, 2006, 22:02 UTC
- Message-ID
- <7vacd6wznz.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <slrndu9v7h.2i8.mdw@metalzone.distorted.org.uk>
Mark Wooding <mdw@distorted.org.uk> writes:
> OK, then: how about putting the user_path logic into enter_repo? Like > this, perhaps:
I'd like to *REALLY* leave enter_repo alone unless necessary. The last round to finalize enter_repo() was painful enough to think its security implications through. And this particular "public_html" insertion change can be done without touching it.
I think the attached patch would essentially do the same thing. Only lightly tested, but tested nevertheless ;-). This comes on top of what I have in 'pu'.
One thing I have not done yet but I consider bould be a good change is to perhaps change the whitelist logic in path_ok() after enter_repo() says what canonicalized path it decided to use, so that it allows any user-relative paths without checking the whitelist when user_path is in effect (and perhaps not empty, so giving --user-path="public_git" would allow everybody while giving --user-path alone or --user-path="" would not). Otherwise, as the code currently stands (regardless of any of the patches in the discussion in this thread) I think you need to list all users on the whitelist line. I think --user-path is about giving all users ability to export their stuff. What do you think?
-- >8 --
diff --git a/daemon.c b/daemon.c index dab8c2c..a1ccda3 100644 --- a/daemon.c +++ b/daemon.c @@ -18,7 +18,8 @@ static int reuseaddr; static const char daemon_usage[] = "git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n" " [--timeout=n] [--init-timeout=n] [--strict-paths]\n" -" [--base-path=path] [--reuseaddr] [directory...]"; +" [--base-path=path] [--user-path | --user-path=path]\n" +" [--reuseaddr] [directory...]"; /* List of acceptable pathname prefixes */ static char **ok_paths = NULL; @@ -30,6 +31,12 @@ static int export_all_trees = 0; /* Take all paths relative to this one if non-NULL */ static char *base_path = NULL; +/* If defined, ~user notation is allowed and the string is inserted + * after ~user/. E.g. a request to git://host/~alice/frotz would + * go to /home/alice/pub_git/frotz with --user-path=pub_git. + */ +static char *user_path = NULL; + /* Timeout, and initial timeout */ static unsigned int timeout = 0; static unsigned int init_timeout = 0; @@ -137,6 +144,7 @@ static int avoid_alias(char *p) static char *path_ok(char *dir) { + static char rpath[PATH_MAX]; char *path; if (avoid_alias(dir)) { @@ -144,12 +152,31 @@ static char *path_ok(char *dir) return NULL; } - if (base_path) { - static char rpath[PATH_MAX]; - if (!strict_paths && *dir == '~') - ; /* allow user relative paths */ - else if (*dir != '/') { - /* otherwise allow only absolute */ + if (*dir == '~') { + if (!user_path) { + logerror("'%s': User-path not allowed", dir); + return NULL; + } + if (*user_path) { + /* Got either "~alice" or "~alice/foo"; + * rewrite them to "~alice/%s" or + * "~alice/%s/foo". + */ + int namlen, restlen = strlen(dir); + char *slash = strchr(dir, '/'); + if (!slash) + slash = dir + restlen; + namlen = slash - dir; + restlen -= namlen; + loginfo("userpath <%s>, request <%s>, namlen %d, restlen %d, slash <%s>", user_path, dir, namlen, restlen, slash); + snprintf(rpath, PATH_MAX, "%.*s/%s%.*s", + namlen, dir, user_path, restlen, slash); + dir = rpath; + } + } + else if (base_path) { + if (*dir != '/') { + /* Allow only absolute */ logerror("'%s': Non-absolute path denied (base-path active)", dir); return NULL; } @@ -688,6 +715,14 @@ int main(int argc, char **argv) reuseaddr = 1; continue; } + if (!strcmp(arg, "--user-path")) { + user_path = ""; + continue; + } + if (!strncmp(arg, "--user-path=", 12)) { + user_path = arg + 12; + continue; + } if (!strcmp(arg, "--")) { ok_paths = &argv[i+1]; break;