Re: [PATCH 3/4] Add init-serve, the remote side of "git init --remote=host:path"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2009, 05:54 UTC
Message-ID: <7v63itbxe7.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20090301031609.GA30384@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 20 quoted lines

> On Sat, Feb 28, 2009 at 04:03:41PM -0800, Junio C Hamano wrote:
>
>> +/*
>> + * Notice any command line argument that we may not want to invoke
>> + * "git init" with when we are doing this remotely, and reject the
>> + * request.
>> + */
>> +static int forbidden_arg(const char *arg)
>> +{
>> +	if (!prefixcmp(arg, "--shared=") ||
>> +	    !strcmp(arg, "--shared") ||
>> +	    !strcmp(arg, "--bare"))
>> +		return 0;
>> +	return 1;
>> +}
>
> I started this mail to complain that this function was "disallow known
> bad" instead of "allow known good". But then after reading it carefully
> three times, I see that it is in fact "not allow known good". Can we
> make it "allowed_arg" to prevent double negation?

I originally had "arg_ok()", but the implementation checked that the argument is not --template, which is the only known to be bad one at the moment, so that we would allow things by default. Later I switched both name and the implementation ;-)

I think renaming it to affirmative form (and swapping the return value, of course) would be a good idea.

One issue I did not describe in the message was to what extent we would want to allow operations other than the creating of a new repository itself.

"Other than the creation" includes things like these:

 - "chgrp project-group" and "chmod g+s".

   It is typical for the system administrator to set up two classes of
   groups in /etc/groups file.  One group per user that is the primary
   group for a user, so that $HOME/ can be owned by that primary group and
   its permission bits set to 2775, and one group per project, so a user
   can belong to groups of the projects he works on.

   With the patch series:

    $ git init --remote=central.xz:/pub/projects/mine.git --shared --bare

   would take care of the "shared" (i.e. g+w) part, but the "mine.git"
   repository will either inherit the group ownership from /pub/projects
   (if /pub/projects is marked with g+s) or owned by the creating user's
   primary group, which would not be usable for use by a project the user
   belongs to.  IOW, --shared by itself is not very useful with its
   current form.

 - Writing to .git/description

   Primarily for use with gitweb; the need for this goes without saying.
   This shouldn't have security implications (even if the current
   implementation had one, it could easily be squashed).

 - Other administrative bits that have security implications:

   In a friendly environment without security concerns, e.g. company
   intranet setting, the user may want to do these things:

   - Installing custom hooks
   - Updating .git/config

   But these should never be allowed in a hosting-site setting.

My current thinking is that "init --remote" should not cater to the third kind. In a friendly environment the user would have a shell access, and if the system does not give a shell access, then the user should not allowed to muck freely with the repository.

I think the "chgrp/chmod g+s" part is necessary, and I envisioned that it would be done by a new option to "git init", but I haven't thought things through. The options to "init" will not be visible to git-shell because they are carried over pkt-line protocol as the payload, and programs like gitosis may have hard time implementing limited access to certain groups, even if they wanted to; I do not think gitosis would care, as it does not rely on the UNIX group permission model for its access control, but other implementations may care.