From 2ab3af323e3c42a1dd554cdf459694aa90d36574 Mon Sep 17 00:00:00 2001
From: Arun Raghavan <ford_prefect@gentoo.org>
Date: Fri, 22 Jan 2010 03:01:48 +0530
Subject: [PATCH 2/2] upload-pack: Add a pre-upload-pack hook

This hook is run after want/have are communicated and before the actual
upload operation is begun. It is passed the set of want and have, as
well as the type of operation (fetch/clone). The intended use for this
hook is to reject large uploads (such as very large initial clones).
---
 Documentation/git-upload-pack.txt       |    9 +++-
 Documentation/githooks.txt              |   39 +++++++++++--
 t/Makefile                              |    1 +
 t/t5507-pre-upload-pack.sh              |   93 +++++++++++++++++++++++++++++++
 templates/hooks--pre-upload-pack.sample |   11 ++++
 upload-pack.c                           |   34 ++++++++---
 6 files changed, 169 insertions(+), 18 deletions(-)
 create mode 100644 t/t5507-pre-upload-pack.sh
 create mode 100644 templates/hooks--pre-upload-pack.sample

diff --git a/Documentation/git-upload-pack.txt b/Documentation/git-upload-pack.txt
index cf53937..60ab613 100644
--- a/Documentation/git-upload-pack.txt
+++ b/Documentation/git-upload-pack.txt
@@ -20,8 +20,11 @@ The UI for the protocol is on the 'git fetch-pack' side, and the
 program pair is meant to be used to pull updates from a remote
 repository.  For push operations, see 'git send-pack'.
 
+Before starting the upload operation, `pre-upload-pack` hook may be
+called (see linkgit:githooks[5]).
+
 After finishing the operation successfully, `post-upload-pack`
-hook is called (see linkgit:githooks[5]).
+hook may be called (see linkgit:githooks[5]).
 
 OPTIONS
 -------
@@ -32,6 +35,10 @@ OPTIONS
 --timeout=<n>::
 	Interrupt transfer after <n> seconds of inactivity.
 
+--allow-insecure-hooks:
+	Allows pre- and post-upload-pack hooks to be run. See the note
+	above on potential security concerns.
+
 <directory>::
 	The repository to sync from.
 
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 6b573a6..d70c42a 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -310,16 +310,20 @@ Both standard output and standard error output are forwarded to
 'git send-pack' on the other end, so you can simply `echo` messages
 for the user.
 
-post-upload-pack
-----------------
+pre-upload-pack
+---------------
 
-Note that this hook is POTENTIALLY INSECURE. It is run as the user who
+Note that this hook is POTENTIALLY INSECURE on shared systems where
+the owner of the repository is not trusted. It is run as the user who
 is pulling, so an attacker can make a victim run arbitrary code by
-convincing him to clone a repository. To enable this hook, git must be
-compiled with the ALLOW_INSECURE_HOOKS option.
+convincing him to clone a repository. To enable this hook, git must
+be compiled with the ALLOW_INSECURE_HOOKS option, and upload-pack must be
+called with --allow-insecure-hooks (this is usually done by setting a
+configuration option for git-daemon/gitosis/gitolite/etc.).
 
-After upload-pack successfully finishes its operation, this hook is called
-for logging purposes.
+Before the upload-pack is started (but after want/have have been
+communicated), this hook may be called. It can be used, for example,
+to deny very large uploads.
 
 The hook is passed various pieces of information, one per line, from its
 standard input.  Currently the following items can be fed to the hook, but
@@ -334,6 +338,27 @@ have SHA-1::
     the resulting pack, claiming to have them already.  Can occur zero
     or more times in the input.
 
+kind string:
+    Either "clone" (when the client did not give us any "have", and asked
+    for all our refs with "want"), or "fetch" (otherwise).
+
+post-upload-pack
+----------------
+
+The same SECURITY CONCERNS as pre-upload-pack apply here.
+
+After upload-pack successfully finishes its operation, this hook is called
+(for example, for logging).
+
+want SHA-1::
+    40-byte hexadecimal object name the client asked to include in the
+    resulting pack.  Can occur one or more times in the input.
+
+have SHA-1::
+    40-byte hexadecimal object name the client asked to exclude from
+    the resulting pack, claiming to have them already.  Can occur zero
+    or more times in the input.
+
 time float::
     Number of seconds spent for creating the packfile.
 
diff --git a/t/Makefile b/t/Makefile
index 598816a..39f87de 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -20,6 +20,7 @@ TSVN = $(wildcard t91[0-9][0-9]-*.sh)
 
 ifndef ALLOW_INSECURE_HOOKS
 	T := $(filter-out t5501-post-upload-pack.sh,$(T))
+	T := $(filter-out t5507-pre-upload-pack.sh,$(T))
 endif
 
 all: pre-clean
diff --git a/t/t5507-pre-upload-pack.sh b/t/t5507-pre-upload-pack.sh
new file mode 100644
index 0000000..d3a7ba7
--- /dev/null
+++ b/t/t5507-pre-upload-pack.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+
+test_description='pre upload-hook'
+
+. ./test-lib.sh
+
+LOGFILE=".git/pre-upload-pack-log"
+
+test_expect_success setup '
+	test_commit A &&
+	test_commit B &&
+	git reset --hard A &&
+	test_commit C &&
+	git branch prev B &&
+	mkdir -p .git/hooks &&
+	{
+		echo "#!$SHELL_PATH" &&
+		echo "cat >pre-upload-pack-log"
+	} >".git/hooks/pre-upload-pack" &&
+	chmod +x .git/hooks/pre-upload-pack
+'
+
+test_expect_success initial '
+	rm -fr sub &&
+	git init sub &&
+	(
+		cd sub &&
+		git fetch --no-tags .. prev
+	) &&
+	want=$(sed -n "s/^want //p" "$LOGFILE") &&
+	test "$want" = "$(git rev-parse --verify B)" &&
+	! grep "^have " "$LOGFILE" &&
+	kind=$(sed -n "s/^kind //p" "$LOGFILE") &&
+	test "$kind" = fetch
+'
+
+test_expect_success second '
+	rm -fr sub &&
+	git init sub &&
+	(
+		cd sub &&
+		git fetch --no-tags .. prev:refs/remotes/prev &&
+		git fetch --no-tags .. master
+	) &&
+	want=$(sed -n "s/^want //p" "$LOGFILE") &&
+	test "$want" = "$(git rev-parse --verify C)" &&
+	have=$(sed -n "s/^have //p" "$LOGFILE") &&
+	test "$have" = "$(git rev-parse --verify B)" &&
+	kind=$(sed -n "s/^kind //p" "$LOGFILE") &&
+	test "$kind" = fetch
+'
+
+test_expect_success all '
+	rm -fr sub &&
+	HERE=$(pwd) &&
+	git init sub &&
+	(
+		cd sub &&
+		git clone "file://$HERE/.git" new
+	) &&
+	sed -n "s/^want //p" "$LOGFILE" | sort >actual &&
+	git rev-parse A B C | sort >expect &&
+	test_cmp expect actual &&
+	! grep "^have " "$LOGFILE" &&
+	kind=$(sed -n "s/^kind //p" "$LOGFILE") &&
+	test "$kind" = clone
+'
+
+cat > pre-upload-pack <<EOF
+#!$SHELL_PATH
+kind=\$(awk '/^kind /{print \$2; exit}' -)
+if test "\$kind" = "clone"; then
+  echo "Sorry, no cloning!"
+exit 1; fi
+EOF
+
+test_expect_success 'with failing hook' '
+	rm -fr .git
+	test_create_repo src &&
+	(
+		cd src &&
+		mkdir .git/hooks &&
+		mv ../pre-upload-pack ".git/hooks/pre-upload-pack" &&
+		chmod +x .git/hooks/pre-upload-pack &&
+		echo foo > file &&
+		git add file &&
+		git commit -m initial
+	) &&
+	test_must_fail git clone -n "file://$(pwd)/src" dst
+
+'
+
+test_done
diff --git a/templates/hooks--pre-upload-pack.sample b/templates/hooks--pre-upload-pack.sample
new file mode 100644
index 0000000..7342d23
--- /dev/null
+++ b/templates/hooks--pre-upload-pack.sample
@@ -0,0 +1,11 @@
+#!/bin/sh
+
+# This sample shows how one may reject an upload-pack where the client is
+# trying to perform an initial clone clone
+
+kind=$(awk '/^kind /{print $2; exit}' -)
+
+if test "$kind" = "clone"; then
+  echo "Sorry, the clone operation is not allowed"
+  exit 1
+fi
diff --git a/upload-pack.c b/upload-pack.c
index 9aa35ea..645965d 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -11,7 +11,11 @@
 #include "list-objects.h"
 #include "run-command.h"
 
+#if ALLOW_INSECURE_HOOKS
+static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<n>] [--allow-secure-hooks] <dir>";
+#else
 static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<n>] <dir>";
+#endif
 
 /* bits #0..7 in revision.h, #8..10 in commit.c */
 #define THEY_HAVE	(1u << 11)
@@ -41,11 +45,7 @@ static int use_sideband;
 static int debug_fd;
 static int advertise_refs;
 static int stateless_rpc;
-#ifdef ALLOW_INSECURE_HOOKS
-static int allow_insecure_hooks = 1;
-#else
 static int allow_insecure_hooks = 0;
-#endif
 
 static void reset_timeout(void)
 {
@@ -166,14 +166,19 @@ static int feed_obj_to_hook(const char *label, struct object_array *oa, int i, i
 				sha1_to_hex(oa->objects[i].item->sha1));
 }
 
-static int run_post_upload_pack_hook(size_t total, struct timeval *tv)
+static int run_upload_pack_hook(int post, size_t total, struct timeval *tv)
 {
 	const char *argv[2];
 	struct child_process proc;
 	int err, i;
 
-	argv[0] = "hooks/post-upload-pack";
-	argv[1] = NULL;
+	if (!post) {
+		argv[0] = "hooks/pre-upload-pack";
+		argv[1] = NULL;
+	} else {
+		argv[0] = "hooks/post-upload-pack";
+		argv[1] = NULL;
+	}
 
 	if (access(argv[0], X_OK) < 0)
 		return 0;
@@ -192,10 +197,10 @@ static int run_post_upload_pack_hook(size_t total, struct timeval *tv)
 		err |= feed_obj_to_hook("want", &want_obj, i, proc.in);
 	for (i = 0; !err && i < have_obj.nr; i++)
 		err |= feed_obj_to_hook("have", &have_obj, i, proc.in);
-	if (!err)
+	if (!err && post)
 		err |= feed_msg_to_hook(proc.in, "time %ld.%06ld\n",
 					(long)tv->tv_sec, (long)tv->tv_usec);
-	if (!err)
+	if (!err && post)
 		err |= feed_msg_to_hook(proc.in, "size %ld\n", (long)total);
 	if (!err)
 		err |= feed_msg_to_hook(proc.in, "kind %s\n",
@@ -755,7 +760,10 @@ static void upload_pack(void)
 	receive_needs();
 	if (want_obj.nr) {
 		get_common_commits();
-		create_pack_file();
+		if (run_upload_pack_hook(0, 0, NULL))
+			error("pre-upload hook aborted");
+		else
+			create_pack_file();
 	}
 }
 
@@ -794,6 +802,12 @@ int main(int argc, char **argv)
 			i++;
 			break;
 		}
+#ifdef ALLOW_INSECURE_HOOKS
+		if (!strcmp(arg, "--allow-insecure-hooks")) {
+			allow_insecure_hooks = 1;
+			continue;
+		}
+#endif
 	}
 
 	if (i != argc-1)
-- 
1.7.3.1

