Allow cloning from repositories owned by another user
Historically, Git has allowed users to clone from an untrusted repository, and we have documented that this is safe to do so: `upload-pack` tries to avoid any dangerous configuration options or hooks from the repository it's serving, making it safe to clone an untrusted directory and run commands on the resulting clone. However, this was broken by f4aa8c8bb1 ("fetch/clone: detect dubious ownership of local repositories", 2024-04-10) in an attempt to make things more secure. That change resulted in a variety of problems when cloning locally and over SSH, but it did not change the stated security boundary. Because the security boundary has not changed, it is safe to adjust part of the code that patch introduced. To do that and restore the previous functionality, adjust enter_repo to take two flags instead of one. The two bits are - ENTER_REPO_STRICT: callers that require exact paths (as opposed to allowing known suffixes like ".git", ".git/.git" to be omitted) can set this bit. Corresponds to the "strict" parameter that the flags word replaces. - ENTER_REPO_ANY_OWNER_OK: callers that are willing to run without ownership check can set this bit. The former is --strict-paths option of "git daemon". The latter is set only by upload-pack, which honors the claimed security boundary. Note that local clones across ownership boundaries require --no-local so that upload-pack is used. Document this fact in the manual page and provide an example. This patch was based on one written by Junio C Hamano. Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson committed
Nov 15, 2024 at 00:54 UTC
0ffb5a6bf1b0fd9ce0c0b1fd9ce9fd30b89a2563
7 files changed
+49
-11
Documentation/git-clone.txt
+9
@@ -63,6 +63,9 @@ symbolic link, the clone will fail. This is a security measure to
63
prevent the unintentional copying of files by dereferencing the symbolic
64
links.
65
+
66
+This option does not work with repositories owned by other users for security
67
+reasons, and `--no-local` must be specified for the clone to succeed.
68
++
69
*NOTE*: this operation can race with concurrent modification to the
70
source repository, similar to running `cp -r src dst` while modifying
71
`src`.
@@ -381,6 +384,12 @@ $ cd my-linux
384
$ git clone --bare -l /home/proj/.git /pub/scm/proj.git
385
------------
386
387
+* Clone a local repository from a different user:
388
++
389
+------------
390
+$ git clone --no-local /home/otheruser/proj.git /pub/scm/proj.git
391
+------------
392
+
393
CONFIGURATION
394
-------------
395
builtin/upload-pack.c
+4
-1
@@ -34,6 +34,7 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
34
N_("interrupt transfer after <n> seconds of inactivity")),
35
OPT_END()
36
};
37
+ unsigned enter_repo_flags = ENTER_REPO_ANY_OWNER_OK;
38
39
packet_trace_identity("upload-pack");
40
disable_replace_refs();
@@ -49,7 +50,9 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
50
51
dir = argv[0];
52
52
- if (!enter_repo(dir, strict))
53
+ if (strict)
54
+ enter_repo_flags |= ENTER_REPO_STRICT;
55
+ if (!enter_repo(dir, enter_repo_flags))
56
die("'%s' does not appear to be a git repository", dir);
57
58
switch (determine_protocol_version_server()) {
daemon.c
+4
-2
@@ -149,6 +149,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
149
size_t rlen;
150
const char *path;
151
const char *dir;
152
+ unsigned enter_repo_flags;
153
154
dir = directory;
155
@@ -239,14 +240,15 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
240
dir = rpath;
241
}
242
242
- path = enter_repo(dir, strict_paths);
243
+ enter_repo_flags = strict_paths ? ENTER_REPO_STRICT : 0;
244
+ path = enter_repo(dir, enter_repo_flags);
245
if (!path && base_path && base_path_relaxed) {
246
/*
247
* if we fail and base_path_relaxed is enabled, try without
248
* prefixing the base path
249
*/
250
dir = directory;
249
- path = enter_repo(dir, strict_paths);
251
+ path = enter_repo(dir, enter_repo_flags);
252
}
253
254
if (!path) {
path.c
+6
-4
@@ -794,7 +794,7 @@ return_null:
794
* links. User relative paths are also returned as they are given,
795
* except DWIM suffixing.
796
*/
797
-const char *enter_repo(const char *path, int strict)
797
+const char *enter_repo(const char *path, unsigned flags)
798
{
799
static struct strbuf validated_path = STRBUF_INIT;
800
static struct strbuf used_path = STRBUF_INIT;
@@ -802,7 +802,7 @@ const char *enter_repo(const char *path, int strict)
802
if (!path)
803
return NULL;
804
805
- if (!strict) {
805
+ if (!(flags & ENTER_REPO_STRICT)) {
806
static const char *suffix[] = {
807
"/.git", "", ".git/.git", ".git", NULL,
808
};
@@ -846,7 +846,8 @@ const char *enter_repo(const char *path, int strict)
846
if (!suffix[i])
847
return NULL;
848
gitfile = read_gitfile(used_path.buf);
849
- die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
849
+ if (!(flags & ENTER_REPO_ANY_OWNER_OK))
850
+ die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
851
if (gitfile) {
852
strbuf_reset(&used_path);
853
strbuf_addstr(&used_path, gitfile);
@@ -857,7 +858,8 @@ const char *enter_repo(const char *path, int strict)
858
}
859
else {
860
const char *gitfile = read_gitfile(path);
860
- die_upon_dubious_ownership(gitfile, NULL, path);
861
+ if (!(flags & ENTER_REPO_ANY_OWNER_OK))
862
+ die_upon_dubious_ownership(gitfile, NULL, path);
863
if (gitfile)
864
path = gitfile;
865
if (chdir(path))
path.h
+16
-1
@@ -184,7 +184,22 @@ int validate_headref(const char *ref);
184
int adjust_shared_perm(const char *path);
185
186
char *interpolate_path(const char *path, int real_home);
187
-const char *enter_repo(const char *path, int strict);
187
+
188
+/* The bits are as follows:
189
+ *
190
+ * - ENTER_REPO_STRICT: callers that require exact paths (as opposed
191
+ * to allowing known suffixes like ".git", ".git/.git" to be
192
+ * omitted) can set this bit.
193
+ *
194
+ * - ENTER_REPO_ANY_OWNER_OK: callers that are willing to run without
195
+ * ownership check can set this bit.
196
+ */
197
+enum {
198
+ ENTER_REPO_STRICT = (1<<0),
199
+ ENTER_REPO_ANY_OWNER_OK = (1<<1),
200
+};
201
+
202
+const char *enter_repo(const char *path, unsigned flags);
203
const char *remove_leading_path(const char *in, const char *prefix);
204
const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
205
int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);
t/t0411-clone-from-partial.sh
-3
@@ -28,7 +28,6 @@ test_expect_success 'local clone must not fetch from promisor remote and execute
28
test_must_fail git clone \
29
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
30
evil clone1 2>err &&
31
- test_grep "detected dubious ownership" err &&
31
test_grep ! "fake-upload-pack running" err &&
32
test_path_is_missing script-executed
33
'
@@ -38,7 +37,6 @@ test_expect_success 'clone from file://... must not fetch from promisor remote a
37
test_must_fail git clone \
38
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
39
"file://$(pwd)/evil" clone2 2>err &&
41
- test_grep "detected dubious ownership" err &&
40
test_grep ! "fake-upload-pack running" err &&
41
test_path_is_missing script-executed
42
'
@@ -48,7 +46,6 @@ test_expect_success 'fetch from file://... must not fetch from promisor remote a
46
test_must_fail git fetch \
47
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
48
"file://$(pwd)/evil" 2>err &&
51
- test_grep "detected dubious ownership" err &&
49
test_grep ! "fake-upload-pack running" err &&
50
test_path_is_missing script-executed
51
'
t/t5605-clone-local.sh
+10
@@ -153,6 +153,16 @@ test_expect_success 'cloning a local path with --no-local does not hardlink' '
153
! repo_is_hardlinked force-nonlocal
154
'
155
156
+test_expect_success 'cloning a local path with --no-local from a different user succeeds' '
157
+ git clone --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
158
+ --no-local a nonlocal-otheruser 2>err &&
159
+ ! repo_is_hardlinked nonlocal-otheruser &&
160
+ # Verify that this is a git repository.
161
+ git -C nonlocal-otheruser rev-parse --show-toplevel &&
162
+ ! test_grep "detected dubious ownership" err
163
+
164
+'
165
+
166
test_expect_success 'cloning locally respects "-u" for fetching refs' '
167
test_must_fail git clone --bare -u false a should_not_work.git
168
'