Re: User-relative paths

From: Junio C Hamano <junkio@cox.net>
Date: Oct 23, 2005, 19:56 UTC
Message-ID: <7v1x2cyplw.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <435B5AE0.1060400@op5.se>

Andreas Ericsson <ae@op5.se> writes:

Show 14 quoted lines

> 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).

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; 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();

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