debian/patches/0006-hooks-clone-protections-simplify-templates-hooks-vali.diff - jrn/git - Git at Google

 From 8813bb5f4109991b88c98584a4abbb2d06cfbc28 Mon Sep 17 00:00:00 2001
 From: Johannes Schindelin <johannes.schindelin@gmx.de>
 Date: Sat, 18 May 2024 10:32:45 +0000
 Subject: hooks(clone protections): simplify templates hooks validation

 commit eff37e9b1dec25a3e1297eb89a36d8e68fe01b40 upstream.

 When an active hook is encountered during a clone operation, to protect
 against Remote Code Execution attack vectors, Git checks whether the
 hook was copied over from the templates directory.

 When that logic was introduced, there was no other way to check this
 than to add a function to compare files.

 In the meantime, we've added code to compute the SHA-256 checksum of a
 given hook and compare that checksum against a list of known-safe ones.

 Let's simplify the logic by adding to said list when copying the
 templates' hooks.

 We need to be careful to support multi-process operations such as
 recursive submodule clones: In such a scenario, the list of SHA-256
 checksums that is kept in memory is not enough, we also have to pass the
 information down to child processes via `GIT_CONFIG_PARAMETERS`.

 Extend the regression test in t5601 to ensure that recursive clones are
 handled as expected.

 Note: Technically there is no way that the checksums computed while
 initializing the submodules' gitdirs can be passed to the process that
 performs the checkout: For historical reasons, these operations are
 performed in processes spawned in separate loops from the
 super-project's `git clone` process. But since the templates from which
 the submodules are initialized are the very same as the ones from which
 the super-project is initialized, we can get away with using the list of
 SHA-256 checksums that is computed when initializing the super-project
 and passing that down to the `submodule--helper` processes that perform
 the recursive checkout.

 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
 Signed-off-by: Junio C Hamano <gitster@pobox.com>
 Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
 ---
  hook.c           | 43 ++++++++++++++++---------------------------
  hook.h           | 10 ++++++++++
  setup.c          |  7 +++++++
  t/t5601-clone.sh | 19 +++++++++++++++++++
  4 files changed, 52 insertions(+), 27 deletions(-)

 diff --git a/hook.c b/hook.c
 index fc0548edb66..8ac51c9912b 100644
 --- a/hook.c
 +++ b/hook.c
 @@ -14,32 +14,6 @@
  #include "hash-ll.h"
  #include "hex.h"

 -static int identical_to_template_hook(const char *name, const char *path)
 -{
 -	const char *env = getenv("GIT_CLONE_TEMPLATE_DIR");
 -	const char *template_dir = get_template_dir(env && *env ? env : NULL);
 -	struct strbuf template_path = STRBUF_INIT;
 -	int found_template_hook, ret;
 -
 -	strbuf_addf(&template_path, "%s/hooks/%s", template_dir, name);
 -	found_template_hook = access(template_path.buf, X_OK) >= 0;
 -#ifdef STRIP_EXTENSION
 -	if (!found_template_hook) {
 -		strbuf_addstr(&template_path, STRIP_EXTENSION);
 -		found_template_hook = access(template_path.buf, X_OK) >= 0;
 -	}
 -#endif
 -	if (!found_template_hook) {
 -		strbuf_release(&template_path);
 -		return 0;
 -	}
 -
 -	ret = do_files_match(template_path.buf, path);
 -
 -	strbuf_release(&template_path);
 -	return ret;
 -}
 -
  static struct strset safe_hook_sha256s = STRSET_INIT;
  static int safe_hook_sha256s_initialized;

 @@ -70,6 +44,22 @@ static int get_sha256_of_file_contents(const char *path, char *sha256)
  	return 0;
  }

 +void add_safe_hook(const char *path)
 +{
 +	char sha256[GIT_SHA256_HEXSZ + 1] = { '\0' };
 +
 +	if (!get_sha256_of_file_contents(path, sha256)) {
 +		char *p;
 +
 +		strset_add(&safe_hook_sha256s, sha256);
 +
 +		/* support multi-process operations e.g. recursive clones */
 +		p = xstrfmt("safe.hook.sha256=%s", sha256);
 +		git_config_push_parameter(p);
 +		free(p);
 +	}
 +}
 +
  static int safe_hook_cb(const char *key, const char *value,
  			const struct config_context *ctx UNUSED, void *d)
  {
 @@ -142,7 +132,6 @@ const char *find_hook(const char *name)
  		return NULL;
  	}
  	if (!git_hooks_path && git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0) &&
 -	    !identical_to_template_hook(name, path.buf) &&
  	    !is_hook_safe_during_clone(name, path.buf, sha256))
  		die(_("active `%s` hook found during `git clone`:\n\t%s\n"
  		      "For security reasons, this is disallowed by default.\n"
 diff --git a/hook.h b/hook.h
 index 19ab9a5806e..b4770d9bd88 100644
 --- a/hook.h
 +++ b/hook.h
 @@ -87,4 +87,14 @@ int run_hooks(const char *hook_name);
   * hook. This function behaves like the old run_hook_le() API.
   */
  int run_hooks_l(const char *hook_name, ...);
 +
 +/**
 + * Mark the contents of the provided path as safe to run during a clone
 + * operation.
 + *
 + * This function is mainly used when copying templates to mark the
 + * just-copied hooks as benign.
 + */
 +void add_safe_hook(const char *path);
 +
  #endif
 diff --git a/setup.c b/setup.c
 index 30f243fc32d..25828a85ec3 100644
 --- a/setup.c
 +++ b/setup.c
 @@ -17,6 +17,8 @@
  #include "trace2.h"
  #include "worktree.h"
  #include "exec-cmd.h"
 +#include "run-command.h"
 +#include "hook.h"

  static int inside_git_dir = -1;
  static int inside_work_tree = -1;
 @@ -1868,6 +1870,7 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
  	size_t path_baselen = path->len;
  	size_t template_baselen = template_path->len;
  	struct dirent *de;
 +	int is_hooks_dir = ends_with(template_path->buf, "/hooks/");

  	/* Note: if ".git/hooks" file exists in the repository being
  	 * re-initialized, /etc/core-git/templates/hooks/update would
 @@ -1920,6 +1923,10 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
  			strbuf_release(&lnk);
  		}
  		else if (S_ISREG(st_template.st_mode)) {
 +			if (is_hooks_dir &&
 +			    is_executable(template_path->buf))
 +				add_safe_hook(template_path->buf);
 +
  			if (copy_file(path->buf, template_path->buf, st_template.st_mode))
  				die_errno(_("cannot copy '%s' to '%s'"),
  					  template_path->buf, path->buf);
 diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
 index deb1c282c71..ca3a8d1ebed 100755
 --- a/t/t5601-clone.sh
 +++ b/t/t5601-clone.sh
 @@ -836,6 +836,25 @@ test_expect_success 'clone with init.templatedir runs hooks' '
  		git config --unset init.templateDir &&
  		test_grep ! "active .* hook found" err &&
  		test_path_is_missing hook-run-local-config/hook.run
 +	) &&
 +
 +	test_config_global protocol.file.allow always &&
 +	git -C tmpl/hooks submodule add "$(pwd)/tmpl/hooks" sub &&
 +	test_tick &&
 +	git -C tmpl/hooks add .gitmodules sub &&
 +	git -C tmpl/hooks commit -m submodule &&
 +
 +	(
 +		sane_unset GIT_TEMPLATE_DIR &&
 +		NO_SET_GIT_TEMPLATE_DIR=t &&
 +		export NO_SET_GIT_TEMPLATE_DIR &&
 +
 +		git -c init.templateDir="$(pwd)/tmpl" \
 +			clone --recurse-submodules \
 +			tmpl/hooks hook-run-submodule 2>err &&
 +		test_grep ! "active .* hook found" err &&
 +		test_path_is_file hook-run-submodule/hook.run &&
 +		test_path_is_file hook-run-submodule/sub/hook.run
  	)
  '
	From 8813bb5f4109991b88c98584a4abbb2d06cfbc28 Mon Sep 17 00:00:00 2001
	From: Johannes Schindelin <johannes.schindelin@gmx.de>
	Date: Sat, 18 May 2024 10:32:45 +0000
	Subject: hooks(clone protections): simplify templates hooks validation

	commit eff37e9b1dec25a3e1297eb89a36d8e68fe01b40 upstream.

	When an active hook is encountered during a clone operation, to protect
	against Remote Code Execution attack vectors, Git checks whether the
	hook was copied over from the templates directory.

	When that logic was introduced, there was no other way to check this
	than to add a function to compare files.

	In the meantime, we've added code to compute the SHA-256 checksum of a
	given hook and compare that checksum against a list of known-safe ones.

	Let's simplify the logic by adding to said list when copying the
	templates' hooks.

	We need to be careful to support multi-process operations such as
	recursive submodule clones: In such a scenario, the list of SHA-256
	checksums that is kept in memory is not enough, we also have to pass the
	information down to child processes via `GIT_CONFIG_PARAMETERS`.

	Extend the regression test in t5601 to ensure that recursive clones are
	handled as expected.

	Note: Technically there is no way that the checksums computed while
	initializing the submodules' gitdirs can be passed to the process that
	performs the checkout: For historical reasons, these operations are
	performed in processes spawned in separate loops from the
	super-project's `git clone` process. But since the templates from which
	the submodules are initialized are the very same as the ones from which
	the super-project is initialized, we can get away with using the list of
	SHA-256 checksums that is computed when initializing the super-project
	and passing that down to the `submodule--helper` processes that perform
	the recursive checkout.

	Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
	Signed-off-by: Junio C Hamano <gitster@pobox.com>
	Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
	---
	hook.c \| 43 ++++++++++++++++---------------------------
	hook.h \| 10 ++++++++++
	setup.c \| 7 +++++++
	t/t5601-clone.sh \| 19 +++++++++++++++++++
	4 files changed, 52 insertions(+), 27 deletions(-)

	diff --git a/hook.c b/hook.c
	index fc0548edb66..8ac51c9912b 100644
	--- a/hook.c
	+++ b/hook.c
	@@ -14,32 +14,6 @@
	#include "hash-ll.h"
	#include "hex.h"

	-static int identical_to_template_hook(const char name, const char path)
	-{
	- const char *env = getenv("GIT_CLONE_TEMPLATE_DIR");
	- const char template_dir = get_template_dir(env && env ? env : NULL);
	- struct strbuf template_path = STRBUF_INIT;
	- int found_template_hook, ret;
	-
	- strbuf_addf(&template_path, "%s/hooks/%s", template_dir, name);
	- found_template_hook = access(template_path.buf, X_OK) >= 0;
	-#ifdef STRIP_EXTENSION
	- if (!found_template_hook) {
	- strbuf_addstr(&template_path, STRIP_EXTENSION);
	- found_template_hook = access(template_path.buf, X_OK) >= 0;
	- }
	-#endif
	- if (!found_template_hook) {
	- strbuf_release(&template_path);
	- return 0;
	- }
	-
	- ret = do_files_match(template_path.buf, path);
	-
	- strbuf_release(&template_path);
	- return ret;
	-}
	-
	static struct strset safe_hook_sha256s = STRSET_INIT;
	static int safe_hook_sha256s_initialized;

	@@ -70,6 +44,22 @@ static int get_sha256_of_file_contents(const char path, char sha256)
	return 0;
	}

	+void add_safe_hook(const char *path)
	+{
	+ char sha256[GIT_SHA256_HEXSZ + 1] = { '\0' };
	+
	+ if (!get_sha256_of_file_contents(path, sha256)) {
	+ char *p;
	+
	+ strset_add(&safe_hook_sha256s, sha256);
	+
	+ /* support multi-process operations e.g. recursive clones */
	+ p = xstrfmt("safe.hook.sha256=%s", sha256);
	+ git_config_push_parameter(p);
	+ free(p);
	+ }
	+}
	+
	static int safe_hook_cb(const char key, const char value,
	const struct config_context ctx UNUSED, void d)
	{
	@@ -142,7 +132,6 @@ const char find_hook(const char name)
	return NULL;
	}
	if (!git_hooks_path && git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0) &&
	- !identical_to_template_hook(name, path.buf) &&
	!is_hook_safe_during_clone(name, path.buf, sha256))
	die(_("active `%s` hook found during `git clone`:\n\t%s\n"
	"For security reasons, this is disallowed by default.\n"
	diff --git a/hook.h b/hook.h
	index 19ab9a5806e..b4770d9bd88 100644
	--- a/hook.h
	+++ b/hook.h
	@@ -87,4 +87,14 @@ int run_hooks(const char *hook_name);
	* hook. This function behaves like the old run_hook_le() API.
	*/
	int run_hooks_l(const char *hook_name, ...);
	+
	+/**
	+ * Mark the contents of the provided path as safe to run during a clone
	+ * operation.
	+ *
	+ * This function is mainly used when copying templates to mark the
	+ * just-copied hooks as benign.
	+ */
	+void add_safe_hook(const char *path);
	+
	#endif
	diff --git a/setup.c b/setup.c
	index 30f243fc32d..25828a85ec3 100644
	--- a/setup.c
	+++ b/setup.c
	@@ -17,6 +17,8 @@
	#include "trace2.h"
	#include "worktree.h"
	#include "exec-cmd.h"
	+#include "run-command.h"
	+#include "hook.h"

	static int inside_git_dir = -1;
	static int inside_work_tree = -1;
	@@ -1868,6 +1870,7 @@ static void copy_templates_1(struct strbuf path, struct strbuf template_path,
	size_t path_baselen = path->len;
	size_t template_baselen = template_path->len;
	struct dirent *de;
	+ int is_hooks_dir = ends_with(template_path->buf, "/hooks/");

	/* Note: if ".git/hooks" file exists in the repository being
	* re-initialized, /etc/core-git/templates/hooks/update would
	@@ -1920,6 +1923,10 @@ static void copy_templates_1(struct strbuf path, struct strbuf template_path,
	strbuf_release(&lnk);
	}
	else if (S_ISREG(st_template.st_mode)) {
	+ if (is_hooks_dir &&
	+ is_executable(template_path->buf))
	+ add_safe_hook(template_path->buf);
	+
	if (copy_file(path->buf, template_path->buf, st_template.st_mode))
	die_errno(_("cannot copy '%s' to '%s'"),
	template_path->buf, path->buf);
	diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
	index deb1c282c71..ca3a8d1ebed 100755
	--- a/t/t5601-clone.sh
	+++ b/t/t5601-clone.sh
	@@ -836,6 +836,25 @@ test_expect_success 'clone with init.templatedir runs hooks' '
	git config --unset init.templateDir &&
	test_grep ! "active .* hook found" err &&
	test_path_is_missing hook-run-local-config/hook.run
	+ ) &&
	+
	+ test_config_global protocol.file.allow always &&
	+ git -C tmpl/hooks submodule add "$(pwd)/tmpl/hooks" sub &&
	+ test_tick &&
	+ git -C tmpl/hooks add .gitmodules sub &&
	+ git -C tmpl/hooks commit -m submodule &&
	+
	+ (
	+ sane_unset GIT_TEMPLATE_DIR &&
	+ NO_SET_GIT_TEMPLATE_DIR=t &&
	+ export NO_SET_GIT_TEMPLATE_DIR &&
	+
	+ git -c init.templateDir="$(pwd)/tmpl" \
	+ clone --recurse-submodules \
	+ tmpl/hooks hook-run-submodule 2>err &&
	+ test_grep ! "active .* hook found" err &&
	+ test_path_is_file hook-run-submodule/hook.run &&
	+ test_path_is_file hook-run-submodule/sub/hook.run
	)
	'