transport: add a protocol-whitelist environment variable
authorJeff King <peff@peff.net>
Wed, 16 Sep 2015 17:12:52 +0000 (13:12 -0400)
committerJunio C Hamano <gitster@pobox.com>
Wed, 23 Sep 2015 18:35:48 +0000 (11:35 -0700)
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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/git.txt
connect.c
t/lib-proto-disable.sh [new file with mode: 0644]
t/t5810-proto-disable-local.sh [new file with mode: 0755]
t/t5811-proto-disable-git.sh [new file with mode: 0755]
t/t5812-proto-disable-http.sh [new file with mode: 0755]
t/t5813-proto-disable-ssh.sh [new file with mode: 0755]
t/t5814-proto-disable-ext.sh [new file with mode: 0755]
transport-helper.c
transport.c
transport.h
index a62ed6f11abf973952cef795907c8f8488f12c69..b6a12b32ee5b7b01fc49ddda9f0ac280eaf0cac1 100644 (file)
@@ -1045,6 +1045,38 @@ GIT_ICASE_PATHSPECS::
        an operation has touched every ref (e.g., because you are
        cloning a repository to make a backup).
 
+`GIT_ALLOW_PROTOCOL`::
+       If set, provide a colon-separated list of protocols which are
+       allowed to be used with fetch/push/clone. This is useful to
+       restrict recursive submodule initialization from an untrusted
+       repository. Any protocol not mentioned will be disallowed (i.e.,
+       this is a whitelist, not a blacklist). If the variable is not
+       set at all, all protocols are enabled.  The protocol names
+       currently used by git are:
+
+         - `file`: any local file-based path (including `file://` URLs,
+           or local paths)
+
+         - `git`: the anonymous git protocol over a direct TCP
+           connection (or proxy, if configured)
+
+         - `ssh`: git over ssh (including `host:path` syntax,
+           `git+ssh://`, etc).
+
+         - `rsync`: git over rsync
+
+         - `http`: git over http, both "smart http" and "dumb http".
+           Note that this does _not_ include `https`; if you want both,
+           you should specify both as `http:https`.
+
+         - any external helpers are named by their protocol (e.g., use
+           `hg` to allow the `git-remote-hg` helper)
++
+Note that this controls only git's internal protocol selection.
+If libcurl is used (e.g., by the `http` transport), it may
+redirect to other protocols. There is not currently any way to
+restrict this.
+
 
 Discussion[[Discussion]]
 ------------------------
index 14c924b030cc17e083d4b8662faf5c6d48f68d85..bd4b50ea153c06089cbc8828b5369729b009eab4 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -9,6 +9,7 @@
 #include "url.h"
 #include "string-list.h"
 #include "sha1-array.h"
+#include "transport.h"
 
 static char *server_capabilities;
 static const char *parse_feature_value(const char *, const char *, int *);
@@ -694,6 +695,8 @@ struct child_process *git_connect(int fd[2], const char *url,
                else
                        target_host = xstrdup(hostandport);
 
+               transport_check_allowed("git");
+
                /* These underlying connection commands die() if they
                 * cannot connect.
                 */
@@ -727,6 +730,7 @@ struct child_process *git_connect(int fd[2], const char *url,
                        int putty;
                        char *ssh_host = hostandport;
                        const char *port = NULL;
+                       transport_check_allowed("ssh");
                        get_host_and_port(&ssh_host, &port);
 
                        if (!port)
@@ -768,6 +772,7 @@ struct child_process *git_connect(int fd[2], const char *url,
                        /* remove repo-local variables from the environment */
                        conn->env = local_repo_env;
                        conn->use_shell = 1;
+                       transport_check_allowed("file");
                }
                argv_array_push(&conn->args, cmd.buf);
 
diff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh
new file mode 100644 (file)
index 0000000..b0917d9
--- /dev/null
@@ -0,0 +1,96 @@
+# Test routines for checking protocol disabling.
+
+# test cloning a particular protocol
+#   $1 - description of the protocol
+#   $2 - machine-readable name of the protocol
+#   $3 - the URL to try cloning
+test_proto () {
+       desc=$1
+       proto=$2
+       url=$3
+
+       test_expect_success "clone $1 (enabled)" '
+               rm -rf tmp.git &&
+               (
+                       GIT_ALLOW_PROTOCOL=$proto &&
+                       export GIT_ALLOW_PROTOCOL &&
+                       git clone --bare "$url" tmp.git
+               )
+       '
+
+       test_expect_success "fetch $1 (enabled)" '
+               (
+                       cd tmp.git &&
+                       GIT_ALLOW_PROTOCOL=$proto &&
+                       export GIT_ALLOW_PROTOCOL &&
+                       git fetch
+               )
+       '
+
+       test_expect_success "push $1 (enabled)" '
+               (
+                       cd tmp.git &&
+                       GIT_ALLOW_PROTOCOL=$proto &&
+                       export GIT_ALLOW_PROTOCOL &&
+                       git push origin HEAD:pushed
+               )
+       '
+
+       test_expect_success "push $1 (disabled)" '
+               (
+                       cd tmp.git &&
+                       GIT_ALLOW_PROTOCOL=none &&
+                       export GIT_ALLOW_PROTOCOL &&
+                       test_must_fail git push origin HEAD:pushed
+               )
+       '
+
+       test_expect_success "fetch $1 (disabled)" '
+               (
+                       cd tmp.git &&
+                       GIT_ALLOW_PROTOCOL=none &&
+                       export GIT_ALLOW_PROTOCOL &&
+                       test_must_fail git fetch
+               )
+       '
+
+       test_expect_success "clone $1 (disabled)" '
+               rm -rf tmp.git &&
+               (
+                       GIT_ALLOW_PROTOCOL=none &&
+                       export GIT_ALLOW_PROTOCOL &&
+                       test_must_fail git clone --bare "$url" tmp.git
+               )
+       '
+}
+
+# set up an ssh wrapper that will access $host/$repo in the
+# trash directory, and enable it for subsequent tests.
+setup_ssh_wrapper () {
+       test_expect_success 'setup ssh wrapper' '
+               write_script ssh-wrapper <<-\EOF &&
+               echo >&2 "ssh: $*"
+               host=$1; shift
+               cd "$TRASH_DIRECTORY/$host" &&
+               eval "$*"
+               EOF
+               GIT_SSH="$PWD/ssh-wrapper" &&
+               export GIT_SSH &&
+               export TRASH_DIRECTORY
+       '
+}
+
+# set up a wrapper that can be used with remote-ext to
+# access repositories in the "remote" directory of trash-dir,
+# like "ext::fake-remote %S repo.git"
+setup_ext_wrapper () {
+       test_expect_success 'setup ext wrapper' '
+               write_script fake-remote <<-\EOF &&
+               echo >&2 "fake-remote: $*"
+               cd "$TRASH_DIRECTORY/remote" &&
+               eval "$*"
+               EOF
+               PATH=$TRASH_DIRECTORY:$PATH &&
+               export TRASH_DIRECTORY
+       '
+}
diff --git a/t/t5810-proto-disable-local.sh b/t/t5810-proto-disable-local.sh
new file mode 100755 (executable)
index 0000000..563592d
--- /dev/null
@@ -0,0 +1,14 @@
+#!/bin/sh
+
+test_description='test disabling of local paths in clone/fetch'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-proto-disable.sh"
+
+test_expect_success 'setup repository to clone' '
+       test_commit one
+'
+
+test_proto "file://" file "file://$PWD"
+test_proto "path" file .
+
+test_done
diff --git a/t/t5811-proto-disable-git.sh b/t/t5811-proto-disable-git.sh
new file mode 100755 (executable)
index 0000000..8ac6b2a
--- /dev/null
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='test disabling of git-over-tcp in clone/fetch'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-proto-disable.sh"
+. "$TEST_DIRECTORY/lib-git-daemon.sh"
+start_git_daemon
+
+test_expect_success 'create git-accessible repo' '
+       bare="$GIT_DAEMON_DOCUMENT_ROOT_PATH/repo.git" &&
+       test_commit one &&
+       git --bare init "$bare" &&
+       git push "$bare" HEAD &&
+       >"$bare/git-daemon-export-ok" &&
+       git -C "$bare" config daemon.receivepack true
+'
+
+test_proto "git://" git "$GIT_DAEMON_URL/repo.git"
+
+test_done
diff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh
new file mode 100755 (executable)
index 0000000..dd5001c
--- /dev/null
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='test disabling of git-over-http in clone/fetch'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-proto-disable.sh"
+. "$TEST_DIRECTORY/lib-httpd.sh"
+start_httpd
+
+test_expect_success 'create git-accessible repo' '
+       bare="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+       test_commit one &&
+       git --bare init "$bare" &&
+       git push "$bare" HEAD &&
+       git -C "$bare" config http.receivepack true
+'
+
+test_proto "smart http" http "$HTTPD_URL/smart/repo.git"
+
+stop_httpd
+test_done
diff --git a/t/t5813-proto-disable-ssh.sh b/t/t5813-proto-disable-ssh.sh
new file mode 100755 (executable)
index 0000000..ad877d7
--- /dev/null
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='test disabling of git-over-ssh in clone/fetch'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-proto-disable.sh"
+
+setup_ssh_wrapper
+
+test_expect_success 'setup repository to clone' '
+       test_commit one &&
+       mkdir remote &&
+       git init --bare remote/repo.git &&
+       git push remote/repo.git HEAD
+'
+
+test_proto "host:path" ssh "remote:repo.git"
+test_proto "ssh://" ssh "ssh://remote/$PWD/remote/repo.git"
+test_proto "git+ssh://" ssh "git+ssh://remote/$PWD/remote/repo.git"
+
+test_done
diff --git a/t/t5814-proto-disable-ext.sh b/t/t5814-proto-disable-ext.sh
new file mode 100755 (executable)
index 0000000..9d6f7df
--- /dev/null
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+test_description='test disabling of remote-helper paths in clone/fetch'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-proto-disable.sh"
+
+setup_ext_wrapper
+
+test_expect_success 'setup repository to clone' '
+       test_commit one &&
+       mkdir remote &&
+       git init --bare remote/repo.git &&
+       git push remote/repo.git HEAD
+'
+
+test_proto "remote-helper" ext "ext::fake-remote %S repo.git"
+
+test_done
index 7dc4a443aee0c6220dc0581d4bcfdd1d65da1d8a..0b5362c268c38a1e4110313e198194fab9f76c83 100644 (file)
@@ -1038,6 +1038,8 @@ int transport_helper_init(struct transport *transport, const char *name)
        struct helper_data *data = xcalloc(1, sizeof(*data));
        data->name = name;
 
+       transport_check_allowed(name);
+
        if (getenv("GIT_TRANSPORT_HELPER_DEBUG"))
                debug = 1;
 
index 88bde1d85e48e7866881806d1ec3f1e3d420ab67..94fe8658f2bfa7775e667f7c28264c3dbe7f63b7 100644 (file)
@@ -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;
index 3e0091eaabe406759005bc24ce2ff39144caa72d..f7df6ec1d2a1f17e9932e531110cc3fe4bdbb81c 100644 (file)
@@ -132,6 +132,13 @@ struct transport {
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
 
+/*
+ * Check whether a transport is allowed by the environment,
+ * and die otherwise. type should generally be the URL scheme,
+ * as described in Documentation/git.txt
+ */
+void transport_check_allowed(const char *type);
+
 /* Transport options which apply to git:// and scp-style URLs */
 
 /* The program to use on the remote side to send a pack */