Jeff King <peff@xxxxxxxx> writes:
> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
