Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
> 
>>Junio C Hamano wrote:
>>
>>>Andreas Ericsson <ae@op5.se> writes:
>>>...
>>>At one point, Linus posted an outline of "restricted login shell
>>>for use with git over ssh".  I think you could start from there,
>>>perhaps extend it so that it checks the binaries *and* pathnames
>>>the user can specify (e.g. only under your own $HOME is allowed,
>>>and no /../ in them, or something silly like that).
>>>
>>
>>I found this in the archives:
>>http://article.gmane.org/gmane.comp.version-control.git/5784/match=restricted+login
>>
>>Is that what you're referring to?
> 
> 
> No, it is this one:
> 
>     http://marc.theaimsgroup.com/?l=git&m=112681457828137&w=2
> 
> But it is orthogonal to what you are doing in this patch.
> 
> 
>>Let me know if you want things done differently.
> 
> 
> I think srvside_chdir() should not do the userdir expansion
> under --strict (otherwise you would need a matching change in
> daemon.c as well, but I would rather not).
> 

True. I'll rework it.

> The --strict flag in upload-pack is to make sure git-daemon can
> see what is being accessed and make its policy decision even
> before it calls upload-pack.  In a pathological case, somebody
> can create a directory "/~foo/bar/.git", where the "/~foo"
> directory is different from "/home/foo", and have git-daemon
> check that the former is OK and call your upload-pack.  Your
> upload-pack uses srvside_chdir() and exposes /home/foo/bar/.git;


It shouldn't, because srvside_chdir() will only user-expand paths that 
start with a tilde.


> this circumvents git-daemon's policy decision, doesn't it?
> 
> I also agree with everything Pasky already said.
> 
>  * In a URL, a colon after hostname means "port number
>    follows".  So it was a good intention to make these
>    consistent:
> 
>         git fetch ssh://kernel.org:git
>         git fetch kernel.org:git
> 
>    it should not be done.  IOW, if I wanted to use the former
>    form (which I do not think I'd use myself), I should say either one
>    of:
> 
>         git fetch ssh://kernel.org:~/git
>         git fetch ssh://kernel.org:~junio/git
> 
>    Oh, I just noticed you do not handle the former, because you
>    did not have to, but now you need to.
> 
>  * Use of "extern const char *__progname" is questionable.  I
>    could be easily talked into:
> 
>     - have "extern const char *git_program_name" in cache.h or
>       somewhere;
> 
>     - convert programs (gradually) to set that at the beginning
>       of main();
> 

See the attached patch, which adds git_progname as a global variable to 
daemon.c with a minimum of fuzz. The one-liner below will add it to the 
rest of the programs. GNU sed >= 4.0.9 required.

grep -l "int main" *.c | xargs -- sed -i '/^#include/i#include "main.h"'

>     - update die() and error() to use that variable when
>       reporting (both callers and implementation) -- this is
>       optional.
> 
> 

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231