From a5adaced2e13c135d5d9cc65be9eb95aa3bacedf Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 16 Sep 2015 13:12:52 -0400 Subject: transport: add a protocol-whitelist environment variable If we are cloning an untrusted remote repository into a sandbox, we may also want to fetch remote submodules in order to get the complete view as intended by the other side. However, that opens us up to attacks where a malicious user gets us to clone something they would not otherwise have access to (this is not necessarily a problem by itself, but we may then act on the cloned contents in a way that exposes them to the attacker). Ideally such a setup would sandbox git entirely away from high-value items, but this is not always practical or easy to set up (e.g., OS network controls may block multiple protocols, and we would want to enable some but not others). We can help this case by providing a way to restrict particular protocols. We use a whitelist in the environment. This is more annoying to set up than a blacklist, but defaults to safety if the set of protocols git supports grows). If no whitelist is specified, we continue to default to allowing all protocols (this is an "unsafe" default, but since the minority of users will want this sandboxing effect, it is the only sensible one). A note on the tests: ideally these would all be in a single test file, but the git-daemon and httpd test infrastructure is an all-or-nothing proposition rather than a test-by-test prerequisite. By putting them all together, we would be unable to test the file-local code on machines without apache. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- transport.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) (limited to 'transport.c') diff --git a/transport.c b/transport.c index 88bde1d85e..94fe8658f2 100644 --- a/transport.c +++ b/transport.c @@ -909,6 +909,20 @@ static int external_specification_len(const char *url) return strchr(url, ':') - url; } +void transport_check_allowed(const char *type) +{ + struct string_list allowed = STRING_LIST_INIT_DUP; + const char *v = getenv("GIT_ALLOW_PROTOCOL"); + + if (!v) + return; + + string_list_split(&allowed, v, ':', -1); + if (!unsorted_string_list_has_string(&allowed, type)) + die("transport '%s' not allowed", type); + string_list_clear(&allowed, 0); +} + struct transport *transport_get(struct remote *remote, const char *url) { const char *helper; @@ -940,12 +954,14 @@ struct transport *transport_get(struct remote *remote, const char *url) if (helper) { transport_helper_init(ret, helper); } else if (starts_with(url, "rsync:")) { + transport_check_allowed("rsync"); ret->get_refs_list = get_refs_via_rsync; ret->fetch = fetch_objs_via_rsync; ret->push = rsync_transport_push; ret->smart_options = NULL; } else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) { struct bundle_transport_data *data = xcalloc(1, sizeof(*data)); + transport_check_allowed("file"); ret->data = data; ret->get_refs_list = get_refs_from_bundle; ret->fetch = fetch_refs_from_bundle; @@ -957,7 +973,10 @@ struct transport *transport_get(struct remote *remote, const char *url) || starts_with(url, "ssh://") || starts_with(url, "git+ssh://") || starts_with(url, "ssh+git://")) { - /* These are builtin smart transports. */ + /* + * These are builtin smart transports; "allowed" transports + * will be checked individually in git_connect. + */ struct git_transport_data *data = xcalloc(1, sizeof(*data)); ret->data = data; ret->set_option = NULL; -- cgit v1.2.3 From 5088d3b38775f8ac12d7f77636775b16059b67ef Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 22 Sep 2015 18:03:49 -0400 Subject: transport: refactor protocol whitelist code The current callers only want to die when their transport is prohibited. But future callers want to query the mechanism without dying. Let's break out a few query functions, and also save the results in a static list so we don't have to re-parse for each query. Based-on-a-patch-by: Blake Burkhart Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- transport.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) (limited to 'transport.c') diff --git a/transport.c b/transport.c index 94fe8658f2..647d2c2afa 100644 --- a/transport.c +++ b/transport.c @@ -909,18 +909,40 @@ static int external_specification_len(const char *url) return strchr(url, ':') - url; } -void transport_check_allowed(const char *type) +static const struct string_list *protocol_whitelist(void) { - struct string_list allowed = STRING_LIST_INIT_DUP; - const char *v = getenv("GIT_ALLOW_PROTOCOL"); + static int enabled = -1; + static struct string_list allowed = STRING_LIST_INIT_DUP; + + if (enabled < 0) { + const char *v = getenv("GIT_ALLOW_PROTOCOL"); + if (v) { + string_list_split(&allowed, v, ':', -1); + string_list_sort(&allowed); + enabled = 1; + } else { + enabled = 0; + } + } - if (!v) - return; + return enabled ? &allowed : NULL; +} + +int is_transport_allowed(const char *type) +{ + const struct string_list *allowed = protocol_whitelist(); + return !allowed || string_list_has_string(allowed, type); +} - string_list_split(&allowed, v, ':', -1); - if (!unsorted_string_list_has_string(&allowed, type)) +void transport_check_allowed(const char *type) +{ + if (!is_transport_allowed(type)) die("transport '%s' not allowed", type); - string_list_clear(&allowed, 0); +} + +int transport_restrict_protocols(void) +{ + return !!protocol_whitelist(); } struct transport *transport_get(struct remote *remote, const char *url) -- cgit v1.2.3