From: Aaron Schrab <aaron@schrab.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Christopher Dale <chrelad@gmail.com>, git@vger.kernel.org
Subject: Re: Adding hooks.directory config option; wiring into run_hook
Date: Fri, 16 Dec 2011 22:03:27 -0500 [thread overview]
Message-ID: <20111217030327.GD30928@pug.qqx.org> (raw)
In-Reply-To: <7vmxasie6k.fsf@alter.siamese.dyndns.org>
At 10:06 -0800 16 Dec 2011, Junio C Hamano <gitster@pobox.com> wrote:
>Christopher Dale <chrelad@gmail.com> writes:
>
>> ...
>> trusted path execution policies. These systems require that any file
>> that can be executed exhibit at least the following characteristics:
>>
>> * The executable, it's directory, and each directory above it are
>> not writable.
>>
>> Since the hooks directory is within a directory that by it's very nature
>> requires write permissions,...
>
>Sorry, but I am not interested at all. If you can write into $GIT_DIR/config
>then you can point at any directory with hooks.directory and that would mean
>it would defeat your "trusted path execution policies".
How does that defeat the policy? It would certainly allow somebody who
can write to $GIT_DIR to disable hooks, use hooks that were meant for a
different repository, or perhaps even use executables that weren't
intended to be hooks. If the proposed configuration option were
modified by an attacker to point to some directory that doesn't meet the
requirements, then the underlying system would still prevent execution;
the same result that would happen if they'd try to install hooks in the
traditional location.
I see other problems with that policy, at least with the short
description that was provided. Unless there are also restrictions on
the allowed owners of the executable and its containing directories, an
attacker would still be able to install new executables and then remove
write permissions before trying to use them. But, that flaw (if it
exists) wouldn't be affected by the proposed change to git.
Requiring that all executables on a secured system be installed by an
admin doesn't seem to be a completely unreasonable requirement. The
supplied patch looks to be fairly small and easy to understand, so I
wouldn't think that including it would present much of a problem for
maintaining git.
The option might also be useful to allow the same hooks directory to be
used for multiple repositories, although symlinks would likely be a
better way to do that.
next prev parent reply other threads:[~2011-12-17 3:03 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-16 18:00 [PATCH] Adding hooks.directory config option; wiring into run_hook Christopher Dale
2011-12-16 18:06 ` Junio C Hamano
2011-12-16 18:28 ` Christopher Dale
2011-12-16 19:23 ` Junio C Hamano
2011-12-17 3:03 ` Aaron Schrab [this message]
2011-12-17 1:58 ` Aaron Schrab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111217030327.GD30928@pug.qqx.org \
--to=aaron@schrab.com \
--cc=chrelad@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).