From e47363e5a8bdf5144059d664c45c0975243ef05b Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 13 Apr 2022 15:32:29 +0000 Subject: [PATCH 1/4] t0033: add tests for safe.directory It is difficult to change the ownership on a directory in our test suite, so insert a new GIT_TEST_ASSUME_DIFFERENT_OWNER environment variable to trick Git into thinking we are in a differently-owned directory. This allows us to test that the config is parsed correctly. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- setup.c | 3 ++- t/t0033-safe-directory.sh | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100755 t/t0033-safe-directory.sh diff --git a/setup.c b/setup.c index 95d5b00940..3c6ed17af9 100644 --- a/setup.c +++ b/setup.c @@ -1053,7 +1053,8 @@ static int ensure_valid_ownership(const char *path) { struct safe_directory_data data = { .path = path }; - if (is_path_owned_by_current_user(path)) + if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) && + is_path_owned_by_current_user(path)) return 1; read_very_early_config(safe_directory_cb, &data); diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh new file mode 100755 index 0000000000..9380ff3d01 --- /dev/null +++ b/t/t0033-safe-directory.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='verify safe.directory checks' + +. ./test-lib.sh + +GIT_TEST_ASSUME_DIFFERENT_OWNER=1 +export GIT_TEST_ASSUME_DIFFERENT_OWNER + +expect_rejected_dir () { + test_must_fail git status 2>err && + grep "safe.directory" err +} + +test_expect_success 'safe.directory is not set' ' + expect_rejected_dir +' + +test_expect_success 'safe.directory does not match' ' + git config --global safe.directory bogus && + expect_rejected_dir +' + +test_expect_success 'safe.directory matches' ' + git config --global --add safe.directory "$(pwd)" && + git status +' + +test_expect_success 'safe.directory matches, but is reset' ' + git config --global --add safe.directory "" && + expect_rejected_dir +' + +test_done From bb50ec3cc300eeff3aba7a2bea145aabdb477d31 Mon Sep 17 00:00:00 2001 From: Matheus Valadares Date: Wed, 13 Apr 2022 15:32:30 +0000 Subject: [PATCH 2/4] setup: fix safe.directory key not being checked It seems that nothing is ever checking to make sure the safe directories in the configs actually have the key safe.directory, so some unrelated config that has a value with a certain directory would also make it a safe directory. Signed-off-by: Matheus Valadares Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- setup.c | 3 +++ t/t0033-safe-directory.sh | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/setup.c b/setup.c index 3c6ed17af9..4b9f073617 100644 --- a/setup.c +++ b/setup.c @@ -1034,6 +1034,9 @@ static int safe_directory_cb(const char *key, const char *value, void *d) { struct safe_directory_data *data = d; + if (strcmp(key, "safe.directory")) + return 0; + if (!value || !*value) data->is_safe = 0; else { diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index 9380ff3d01..6f33c0dfef 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -21,6 +21,11 @@ test_expect_success 'safe.directory does not match' ' expect_rejected_dir ' +test_expect_success 'path exist as different key' ' + git config --global foo.bar "$(pwd)" && + expect_rejected_dir +' + test_expect_success 'safe.directory matches' ' git config --global --add safe.directory "$(pwd)" && git status From 0f85c4a30b072a26d74af8bbf63cc8f6a5dfc1b8 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 13 Apr 2022 15:32:31 +0000 Subject: [PATCH 3/4] setup: opt-out of check with safe.directory=* With the addition of the safe.directory in 8959555ce (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02) released in v2.35.2, we are receiving feedback from a variety of users about the feature. Some users have a very large list of shared repositories and find it cumbersome to add this config for every one of them. In a more difficult case, certain workflows involve running Git commands within containers. The container boundary prevents any global or system config from communicating `safe.directory` values from the host into the container. Further, the container almost always runs as a different user than the owner of the directory in the host. To simplify the reactions necessary for these users, extend the definition of the safe.directory config value to include a possible '*' value. This value implies that all directories are safe, providing a single setting to opt-out of this protection. Note that an empty assignment of safe.directory clears all previous values, and this is already the case with the "if (!value || !*value)" condition. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Documentation/config/safe.txt | 7 +++++++ setup.c | 6 ++++-- t/t0033-safe-directory.sh | 10 ++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt index 63597b2df8..6d764fe0cc 100644 --- a/Documentation/config/safe.txt +++ b/Documentation/config/safe.txt @@ -19,3 +19,10 @@ line option `-c safe.directory=`. The value of this setting is interpolated, i.e. `~/` expands to a path relative to the home directory and `%(prefix)/` expands to a path relative to Git's (runtime) prefix. ++ +To completely opt-out of this security check, set `safe.directory` to the +string `*`. This will allow all repositories to be treated as if their +directory was listed in the `safe.directory` list. If `safe.directory=*` +is set in system config and you want to re-enable this protection, then +initialize your list with an empty value before listing the repositories +that you deem safe. diff --git a/setup.c b/setup.c index 4b9f073617..aad9ace0af 100644 --- a/setup.c +++ b/setup.c @@ -1037,9 +1037,11 @@ static int safe_directory_cb(const char *key, const char *value, void *d) if (strcmp(key, "safe.directory")) return 0; - if (!value || !*value) + if (!value || !*value) { data->is_safe = 0; - else { + } else if (!strcmp(value, "*")) { + data->is_safe = 1; + } else { const char *interpolated = NULL; if (!git_config_pathname(&interpolated, key, value) && diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index 6f33c0dfef..239d93f4d2 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -36,4 +36,14 @@ test_expect_success 'safe.directory matches, but is reset' ' expect_rejected_dir ' +test_expect_success 'safe.directory=*' ' + git config --global --add safe.directory "*" && + git status +' + +test_expect_success 'safe.directory=*, but is reset' ' + git config --global --add safe.directory "" && + expect_rejected_dir +' + test_done From 17083c79ae842b51d82518e2efe5281346acea0e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 13 Apr 2022 13:31:29 -0700 Subject: [PATCH 4/4] Git 2.30.4 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.30.4.txt | 21 +++++++++++++++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.30.4.txt diff --git a/Documentation/RelNotes/2.30.4.txt b/Documentation/RelNotes/2.30.4.txt new file mode 100644 index 0000000000..4eedb74b16 --- /dev/null +++ b/Documentation/RelNotes/2.30.4.txt @@ -0,0 +1,21 @@ +Git v2.30.4 Release Notes +========================= + +This release contains minor fix-ups for the changes that went into +Git 2.30.3, which was made to address CVE-2022-24765. + + * The code that was meant to parse the new `safe.directory` + configuration variable was not checking what configuration + variable was being fed to it, which has been corrected. + + * '*' can be used as the value for the `safe.directory` variable to + signal that the user considers that any directory is safe. + + + +Derrick Stolee (2): + t0033: add tests for safe.directory + setup: opt-out of check with safe.directory=* + +Matheus Valadares (1): + setup: fix safe.directory key not being checked diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 7cf68be76f..a927c77478 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.30.3 +DEF_VER=v2.30.4 LF=' ' diff --git a/RelNotes b/RelNotes index 187351ccd9..4dcee84642 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.30.3.txt \ No newline at end of file +Documentation/RelNotes/2.30.4.txt \ No newline at end of file