blob: b2aa1357fdefef21d6bf85323f98f906f81049e5 [file] [log] [blame]
From 1f34eea689413fa10a664f4c154b097be7796b0a Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sat, 18 May 2024 10:32:43 +0000
Subject: hook(clone protections): add escape hatch
commit 85811d32aca9f0ba324a04bd8709c315d472efbe upstream.
As defense-in-depth measures, v2.39.4 and friends leading up to v2.45.1
introduced code that detects when hooks have been installed during a
`git clone`, which is indicative of a common attack vector with critical
severity that allows Remote Code Execution.
There are legitimate use cases for such behavior, though, for example
when those hooks stem from Git's own templates, which system
administrators are at liberty to modify to enforce, say, commit message
conventions. The git clone protections specifically add exceptions to
allow for that.
Another legitimate use case that has been identified too late to be
handled in these security bug-fix versions is Git LFS: It behaves
somewhat similar to common attack vectors by writing a few hooks while
running the `smudge` filter during a regular clone, which means that Git
has no chance to know that the hooks are benign and e.g. the
`post-checkout` hook can be safely executed as part of the clone
operation.
To help Git LFS, and other tools behaving similarly (if there are any),
let's add a new, multi-valued `safe.hook.sha256` config setting. Like
the already-existing `safe.*` settings, it is ignored in
repository-local configs, and it is interpreted as a list of SHA-256
checksums of hooks' contents that are safe to execute during a clone
operation. Future Git LFS versions will need to write those entries at
the same time they install the `smudge`/`clean` filters.
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>
---
Documentation/config/safe.txt | 6 +++
hook.c | 69 ++++++++++++++++++++++++++++++++---
t/t1800-hook.sh | 15 ++++++++
3 files changed, 85 insertions(+), 5 deletions(-)
diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt
index 577df40223a..e2eb4992bef 100644
--- a/Documentation/config/safe.txt
+++ b/Documentation/config/safe.txt
@@ -59,3 +59,9 @@ which id the original user has.
If that is not what you would prefer and want git to only trust
repositories that are owned by root instead, then you can remove
the `SUDO_UID` variable from root's environment before invoking git.
+
+safe.hook.sha256::
+ The value is the SHA-256 of hooks that are considered to be safe
+ to run during a clone operation.
++
+Multiple values can be added via `git config --global --add`.
diff --git a/hook.c b/hook.c
index 8de469b134a..9eca6c0103a 100644
--- a/hook.c
+++ b/hook.c
@@ -10,6 +10,9 @@
#include "environment.h"
#include "setup.h"
#include "copy.h"
+#include "strmap.h"
+#include "hash-ll.h"
+#include "hex.h"
static int identical_to_template_hook(const char *name, const char *path)
{
@@ -37,11 +40,66 @@ static int identical_to_template_hook(const char *name, const char *path)
return ret;
}
+static struct strset safe_hook_sha256s = STRSET_INIT;
+static int safe_hook_sha256s_initialized;
+
+static int get_sha256_of_file_contents(const char *path, char *sha256)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int fd;
+ ssize_t res;
+
+ git_hash_ctx ctx;
+ const struct git_hash_algo *algo = &hash_algos[GIT_HASH_SHA256];
+ unsigned char hash[GIT_MAX_RAWSZ];
+
+ if ((fd = open(path, O_RDONLY)) < 0)
+ return -1;
+ res = strbuf_read(&sb, fd, 400);
+ close(fd);
+ if (res < 0)
+ return -1;
+
+ algo->init_fn(&ctx);
+ algo->update_fn(&ctx, sb.buf, sb.len);
+ strbuf_release(&sb);
+ algo->final_fn(hash, &ctx);
+
+ hash_to_hex_algop_r(sha256, hash, algo);
+
+ return 0;
+}
+
+static int safe_hook_cb(const char *key, const char *value,
+ const struct config_context *ctx UNUSED, void *d)
+{
+ struct strset *set = d;
+
+ if (value && !strcmp(key, "safe.hook.sha256"))
+ strset_add(set, value);
+
+ return 0;
+}
+
+static int is_hook_safe_during_clone(const char *name, const char *path, char *sha256)
+{
+ if (get_sha256_of_file_contents(path, sha256) < 0)
+ return 0;
+
+ if (!safe_hook_sha256s_initialized) {
+ safe_hook_sha256s_initialized = 1;
+ git_protected_config(safe_hook_cb, &safe_hook_sha256s);
+ }
+
+ return strset_contains(&safe_hook_sha256s, sha256);
+}
+
const char *find_hook(const char *name)
{
static struct strbuf path = STRBUF_INIT;
int found_hook;
+ char sha256[GIT_SHA256_HEXSZ + 1] = { '\0' };
strbuf_reset(&path);
strbuf_git_path(&path, "hooks/%s", name);
@@ -73,13 +131,14 @@ 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))
+ !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"
- "If this is intentional and the hook should actually "
- "be run, please\nrun the command again with "
- "`GIT_CLONE_PROTECTION_ACTIVE=false`"),
- name, path.buf);
+ "If this is intentional and the hook is safe to run, "
+ "please run the following command and try again:\n\n"
+ " git config --global --add safe.hook.sha256 %s"),
+ name, path.buf, sha256);
return path.buf;
}
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 8b0234cf2d5..cbdf60c451a 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -185,4 +185,19 @@ test_expect_success 'stdin to hooks' '
test_cmp expect actual
'
+test_expect_success '`safe.hook.sha256` and clone protections' '
+ git init safe-hook &&
+ write_script safe-hook/.git/hooks/pre-push <<-\EOF &&
+ echo "called hook" >safe-hook.log
+ EOF
+
+ test_must_fail env GIT_CLONE_PROTECTION_ACTIVE=true \
+ git -C safe-hook hook run pre-push 2>err &&
+ cmd="$(grep "git config --global --add safe.hook.sha256 [0-9a-f]" err)" &&
+ eval "$cmd" &&
+ GIT_CLONE_PROTECTION_ACTIVE=true \
+ git -C safe-hook hook run pre-push &&
+ test "called hook" = "$(cat safe-hook/safe-hook.log)"
+'
+
test_done