From 0ffb5a6bf1b0fd9ce0c0b1fd9ce9fd30b89a2563 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Fri, 15 Nov 2024 00:54:04 +0000 Subject: 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 --- builtin/upload-pack.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'builtin/upload-pack.c') diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c index 272cddaafd..72af0094e4 100644 --- a/builtin/upload-pack.c +++ b/builtin/upload-pack.c @@ -34,6 +34,7 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix) N_("interrupt transfer after seconds of inactivity")), OPT_END() }; + unsigned enter_repo_flags = ENTER_REPO_ANY_OWNER_OK; packet_trace_identity("upload-pack"); disable_replace_refs(); @@ -49,7 +50,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()) { -- cgit v1.3-5-g9baa From 395b584b5751b009d657d8c3aed371f2a233d919 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 17 Dec 2024 07:43:51 +0100 Subject: serve: stop using `the_repository` Stop using `the_repository` in the "serve" subsystem by passing in a repository when advertising capabilities or serving requests. Adjust callers accordingly by using `the_repository`. While there may be some callers that have a repository available in their context, this trivial conversion allows for easier verification and bubbles up the use of `the_repository` by one level. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/upload-pack.c | 6 ++++-- serve.c | 36 +++++++++++++++++------------------- serve.h | 6 ++++-- t/helper/test-serve-v2.c | 7 +++++-- 4 files changed, 30 insertions(+), 25 deletions(-) (limited to 'builtin/upload-pack.c') diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c index dd63d6eadf..c2bbc035ab 100644 --- a/builtin/upload-pack.c +++ b/builtin/upload-pack.c @@ -1,3 +1,5 @@ +#define USE_THE_REPOSITORY_VARIABLE + #include "builtin.h" #include "exec-cmd.h" #include "gettext.h" @@ -63,9 +65,9 @@ int cmd_upload_pack(int argc, switch (determine_protocol_version_server()) { case protocol_v2: if (advertise_refs) - protocol_v2_advertise_capabilities(); + protocol_v2_advertise_capabilities(the_repository); else - protocol_v2_serve_loop(stateless_rpc); + protocol_v2_serve_loop(the_repository, stateless_rpc); break; case protocol_v1: /* diff --git a/serve.c b/serve.c index c8694e3751..f6dfe34a2b 100644 --- a/serve.c +++ b/serve.c @@ -1,5 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE - #include "git-compat-util.h" #include "repository.h" #include "config.h" @@ -159,7 +157,7 @@ static struct protocol_capability capabilities[] = { }, }; -void protocol_v2_advertise_capabilities(void) +void protocol_v2_advertise_capabilities(struct repository *r) { struct strbuf capability = STRBUF_INIT; struct strbuf value = STRBUF_INIT; @@ -170,7 +168,7 @@ void protocol_v2_advertise_capabilities(void) for (size_t i = 0; i < ARRAY_SIZE(capabilities); i++) { struct protocol_capability *c = &capabilities[i]; - if (c->advertise(the_repository, &value)) { + if (c->advertise(r, &value)) { strbuf_addstr(&capability, c->name); if (value.len) { @@ -214,20 +212,20 @@ static struct protocol_capability *get_capability(const char *key, const char ** return NULL; } -static int receive_client_capability(const char *key) +static int receive_client_capability(struct repository *r, const char *key) { const char *value; const struct protocol_capability *c = get_capability(key, &value); - if (!c || c->command || !c->advertise(the_repository, NULL)) + if (!c || c->command || !c->advertise(r, NULL)) return 0; if (c->receive) - c->receive(the_repository, value); + c->receive(r, value); return 1; } -static int parse_command(const char *key, struct protocol_capability **command) +static int parse_command(struct repository *r, const char *key, struct protocol_capability **command) { const char *out; @@ -238,7 +236,7 @@ static int parse_command(const char *key, struct protocol_capability **command) if (*command) die("command '%s' requested after already requesting command '%s'", out, (*command)->name); - if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command || value) + if (!cmd || !cmd->advertise(r, NULL) || !cmd->command || value) die("invalid command '%s'", out); *command = cmd; @@ -253,7 +251,7 @@ enum request_state { PROCESS_REQUEST_DONE, }; -static int process_request(void) +static int process_request(struct repository *r) { enum request_state state = PROCESS_REQUEST_KEYS; struct packet_reader reader; @@ -278,8 +276,8 @@ static int process_request(void) case PACKET_READ_EOF: BUG("Should have already died when seeing EOF"); case PACKET_READ_NORMAL: - if (parse_command(reader.line, &command) || - receive_client_capability(reader.line)) + if (parse_command(r, reader.line, &command) || + receive_client_capability(r, reader.line)) seen_capability_or_command = 1; else die("unknown capability '%s'", reader.line); @@ -319,30 +317,30 @@ static int process_request(void) if (!command) die("no command requested"); - if (client_hash_algo != hash_algo_by_ptr(the_repository->hash_algo)) + if (client_hash_algo != hash_algo_by_ptr(r->hash_algo)) die("mismatched object format: server %s; client %s", - the_repository->hash_algo->name, + r->hash_algo->name, hash_algos[client_hash_algo].name); - command->command(the_repository, &reader); + command->command(r, &reader); return 0; } -void protocol_v2_serve_loop(int stateless_rpc) +void protocol_v2_serve_loop(struct repository *r, int stateless_rpc) { if (!stateless_rpc) - protocol_v2_advertise_capabilities(); + protocol_v2_advertise_capabilities(r); /* * If stateless-rpc was requested then exit after * a single request/response exchange */ if (stateless_rpc) { - process_request(); + process_request(r); } else { for (;;) - if (process_request()) + if (process_request(r)) break; } } diff --git a/serve.h b/serve.h index f946cf904a..85bf73cfe5 100644 --- a/serve.h +++ b/serve.h @@ -1,7 +1,9 @@ #ifndef SERVE_H #define SERVE_H -void protocol_v2_advertise_capabilities(void); -void protocol_v2_serve_loop(int stateless_rpc); +struct repository; + +void protocol_v2_advertise_capabilities(struct repository *r); +void protocol_v2_serve_loop(struct repository *r, int stateless_rpc); #endif /* SERVE_H */ diff --git a/t/helper/test-serve-v2.c b/t/helper/test-serve-v2.c index 054cbcf5d8..63a200b8d4 100644 --- a/t/helper/test-serve-v2.c +++ b/t/helper/test-serve-v2.c @@ -1,6 +1,9 @@ +#define USE_THE_REPOSITORY_VARIABLE + #include "test-tool.h" #include "gettext.h" #include "parse-options.h" +#include "repository.h" #include "serve.h" #include "setup.h" @@ -28,9 +31,9 @@ int cmd__serve_v2(int argc, const char **argv) PARSE_OPT_KEEP_UNKNOWN_OPT); if (advertise_capabilities) - protocol_v2_advertise_capabilities(); + protocol_v2_advertise_capabilities(the_repository); else - protocol_v2_serve_loop(stateless_rpc); + protocol_v2_serve_loop(the_repository, stateless_rpc); return 0; } -- cgit v1.3-5-g9baa