Re: Git clone reads safe.directory differently?
Junio C Hamano <gitster@pobox.com> writes:
> So we probably need to add another axis to the "strict" parameter
> enter_repo() takes to selectively disable the ownership checks only
> for upload-pack, or something like that.
So, here is a rough sketch for the above. Interested parties may build on top of it, perhaps by adding separate knobs to loosen or tighten the second parameter given to enter_repo() at different callsites, by writing tests to make sure they work as intended, and by documenting the security story around it none of which I do here ;-).
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 wordreplaces.
- 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, but you may want to add configuration knobs in protected configuration files to loosen it per callsites.
builtin/upload-pack.c | 5 ++++-
daemon.c | 6 ++++--
path.c | 10 ++++++----
path.h | 7 ++++++-
4 files changed, 20 insertions(+), 8 deletions(-)
diff --git c/builtin/upload-pack.c w/builtin/upload-pack.c
index 46d93278d9..fe50ce3eed 100644
--- c/builtin/upload-pack.c
+++ w/builtin/upload-pack.c
@@ -36,6 +36,7 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
N_("interrupt transfer after <n> seconds of inactivity")),
OPT_END()
};
+ unsigned enter_repo_flags = ENTER_REPO_ANY_OWNER_OK;
packet_trace_identity("upload-pack");
disable_replace_refs();
@@ -51,7 +52,9 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
dir = argv[0];
- if (!enter_repo(dir, strict))
+ if (strict)
+ enter_repo_flags |= ENTER_REPO_STRICT;
+ if (!enter_repo(dir, enter_repo_flags))
die("'%s' does not appear to be a git repository", dir);
switch (determine_protocol_version_server()) {
diff --git c/daemon.c w/daemon.c
index 17d331b2f3..fb37135521 100644
--- c/daemon.c
+++ w/daemon.c
@@ -149,6 +149,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
size_t rlen;
const char *path;
const char *dir;
+ unsigned enter_repo_flags;
dir = directory;
@@ -239,14 +240,15 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
dir = rpath;
}
- path = enter_repo(dir, strict_paths);
+ enter_repo_flags = strict_paths ? ENTER_REPO_STRICT : 0;
+ path = enter_repo(dir, enter_repo_flags);
if (!path && base_path && base_path_relaxed) {
/*
* if we fail and base_path_relaxed is enabled, try without
* prefixing the base path
*/
dir = directory;
- path = enter_repo(dir, strict_paths);
+ path = enter_repo(dir, enter_repo_flags);
}
if (!path) {
diff --git c/path.c w/path.c
index 19f7684f38..df5aefdb8f 100644
--- c/path.c
+++ w/path.c
@@ -727,7 +727,7 @@ char *interpolate_path(const char *path, int real_home)
* links. User relative paths are also returned as they are given,
* except DWIM suffixing.
*/
-const char *enter_repo(const char *path, int strict)
+const char *enter_repo(const char *path, unsigned flags)
{
static struct strbuf validated_path = STRBUF_INIT;
static struct strbuf used_path = STRBUF_INIT;
@@ -735,7 +735,7 @@ const char *enter_repo(const char *path, int strict)
if (!path)
return NULL;
- if (!strict) {
+ if (!(flags & ENTER_REPO_STRICT)) {
static const char *suffix[] = {
"/.git", "", ".git/.git", ".git", NULL,
};
@@ -779,7 +779,8 @@ const char *enter_repo(const char *path, int strict)
if (!suffix[i])
return NULL;
gitfile = read_gitfile(used_path.buf);
- die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
+ if (!(flags & ENTER_REPO_ANY_OWNER_OK))
+ die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
if (gitfile) {
strbuf_reset(&used_path);
strbuf_addstr(&used_path, gitfile);
@@ -790,7 +791,8 @@ const char *enter_repo(const char *path, int strict)
}
else {
const char *gitfile = read_gitfile(path);
- die_upon_dubious_ownership(gitfile, NULL, path);
+ if (!(flags & ENTER_REPO_ANY_OWNER_OK))
+ die_upon_dubious_ownership(gitfile, NULL, path);
if (gitfile)
path = gitfile;
if (chdir(path))
diff --git c/path.h w/path.h
index a6f0b70692..a0021a1425 100644
--- c/path.h
+++ w/path.h
@@ -187,7 +187,12 @@ int calc_shared_perm(int mode);
int adjust_shared_perm(const char *path);
char *interpolate_path(const char *path, int real_home);
-const char *enter_repo(const char *path, int strict);
+
+enum {
+ ENTER_REPO_STRICT = (1<<0),
+ ENTER_REPO_ANY_OWNER_OK = (1<<1),
+};
+const char *enter_repo(const char *path, unsigned flags);
const char *remove_leading_path(const char *in, const char *prefix);
const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);
