Skip to content

Commit a5adace

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ecad27c commit a5adace

11 files changed

+254
-1
lines changed

Documentation/git.txt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,38 @@ GIT_ICASE_PATHSPECS::
10451045
an operation has touched every ref (e.g., because you are
10461046
cloning a repository to make a backup).
10471047

1048+
`GIT_ALLOW_PROTOCOL`::
1049+
If set, provide a colon-separated list of protocols which are
1050+
allowed to be used with fetch/push/clone. This is useful to
1051+
restrict recursive submodule initialization from an untrusted
1052+
repository. Any protocol not mentioned will be disallowed (i.e.,
1053+
this is a whitelist, not a blacklist). If the variable is not
1054+
set at all, all protocols are enabled. The protocol names
1055+
currently used by git are:
1056+
1057+
- `file`: any local file-based path (including `file://` URLs,
1058+
or local paths)
1059+
1060+
- `git`: the anonymous git protocol over a direct TCP
1061+
connection (or proxy, if configured)
1062+
1063+
- `ssh`: git over ssh (including `host:path` syntax,
1064+
`git+ssh://`, etc).
1065+
1066+
- `rsync`: git over rsync
1067+
1068+
- `http`: git over http, both "smart http" and "dumb http".
1069+
Note that this does _not_ include `https`; if you want both,
1070+
you should specify both as `http:https`.
1071+
1072+
- any external helpers are named by their protocol (e.g., use
1073+
`hg` to allow the `git-remote-hg` helper)
1074+
+
1075+
Note that this controls only git's internal protocol selection.
1076+
If libcurl is used (e.g., by the `http` transport), it may
1077+
redirect to other protocols. There is not currently any way to
1078+
restrict this.
1079+
10481080

10491081
Discussion[[Discussion]]
10501082
------------------------

connect.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "url.h"
1010
#include "string-list.h"
1111
#include "sha1-array.h"
12+
#include "transport.h"
1213

1314
static char *server_capabilities;
1415
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,
694695
else
695696
target_host = xstrdup(hostandport);
696697

698+
transport_check_allowed("git");
699+
697700
/* These underlying connection commands die() if they
698701
* cannot connect.
699702
*/
@@ -727,6 +730,7 @@ struct child_process *git_connect(int fd[2], const char *url,
727730
int putty;
728731
char *ssh_host = hostandport;
729732
const char *port = NULL;
733+
transport_check_allowed("ssh");
730734
get_host_and_port(&ssh_host, &port);
731735

732736
if (!port)
@@ -768,6 +772,7 @@ struct child_process *git_connect(int fd[2], const char *url,
768772
/* remove repo-local variables from the environment */
769773
conn->env = local_repo_env;
770774
conn->use_shell = 1;
775+
transport_check_allowed("file");
771776
}
772777
argv_array_push(&conn->args, cmd.buf);
773778

t/lib-proto-disable.sh

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Test routines for checking protocol disabling.
2+
3+
# test cloning a particular protocol
4+
# $1 - description of the protocol
5+
# $2 - machine-readable name of the protocol
6+
# $3 - the URL to try cloning
7+
test_proto () {
8+
desc=$1
9+
proto=$2
10+
url=$3
11+
12+
test_expect_success "clone $1 (enabled)" '
13+
rm -rf tmp.git &&
14+
(
15+
GIT_ALLOW_PROTOCOL=$proto &&
16+
export GIT_ALLOW_PROTOCOL &&
17+
git clone --bare "$url" tmp.git
18+
)
19+
'
20+
21+
test_expect_success "fetch $1 (enabled)" '
22+
(
23+
cd tmp.git &&
24+
GIT_ALLOW_PROTOCOL=$proto &&
25+
export GIT_ALLOW_PROTOCOL &&
26+
git fetch
27+
)
28+
'
29+
30+
test_expect_success "push $1 (enabled)" '
31+
(
32+
cd tmp.git &&
33+
GIT_ALLOW_PROTOCOL=$proto &&
34+
export GIT_ALLOW_PROTOCOL &&
35+
git push origin HEAD:pushed
36+
)
37+
'
38+
39+
test_expect_success "push $1 (disabled)" '
40+
(
41+
cd tmp.git &&
42+
GIT_ALLOW_PROTOCOL=none &&
43+
export GIT_ALLOW_PROTOCOL &&
44+
test_must_fail git push origin HEAD:pushed
45+
)
46+
'
47+
48+
test_expect_success "fetch $1 (disabled)" '
49+
(
50+
cd tmp.git &&
51+
GIT_ALLOW_PROTOCOL=none &&
52+
export GIT_ALLOW_PROTOCOL &&
53+
test_must_fail git fetch
54+
)
55+
'
56+
57+
test_expect_success "clone $1 (disabled)" '
58+
rm -rf tmp.git &&
59+
(
60+
GIT_ALLOW_PROTOCOL=none &&
61+
export GIT_ALLOW_PROTOCOL &&
62+
test_must_fail git clone --bare "$url" tmp.git
63+
)
64+
'
65+
}
66+
67+
# set up an ssh wrapper that will access $host/$repo in the
68+
# trash directory, and enable it for subsequent tests.
69+
setup_ssh_wrapper () {
70+
test_expect_success 'setup ssh wrapper' '
71+
write_script ssh-wrapper <<-\EOF &&
72+
echo >&2 "ssh: $*"
73+
host=$1; shift
74+
cd "$TRASH_DIRECTORY/$host" &&
75+
eval "$*"
76+
EOF
77+
GIT_SSH="$PWD/ssh-wrapper" &&
78+
export GIT_SSH &&
79+
export TRASH_DIRECTORY
80+
'
81+
}
82+
83+
# set up a wrapper that can be used with remote-ext to
84+
# access repositories in the "remote" directory of trash-dir,
85+
# like "ext::fake-remote %S repo.git"
86+
setup_ext_wrapper () {
87+
test_expect_success 'setup ext wrapper' '
88+
write_script fake-remote <<-\EOF &&
89+
echo >&2 "fake-remote: $*"
90+
cd "$TRASH_DIRECTORY/remote" &&
91+
eval "$*"
92+
EOF
93+
PATH=$TRASH_DIRECTORY:$PATH &&
94+
export TRASH_DIRECTORY
95+
'
96+
}

t/t5810-proto-disable-local.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#!/bin/sh
2+
3+
test_description='test disabling of local paths in clone/fetch'
4+
. ./test-lib.sh
5+
. "$TEST_DIRECTORY/lib-proto-disable.sh"
6+
7+
test_expect_success 'setup repository to clone' '
8+
test_commit one
9+
'
10+
11+
test_proto "file://" file "file://$PWD"
12+
test_proto "path" file .
13+
14+
test_done

t/t5811-proto-disable-git.sh

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#!/bin/sh
2+
3+
test_description='test disabling of git-over-tcp in clone/fetch'
4+
. ./test-lib.sh
5+
. "$TEST_DIRECTORY/lib-proto-disable.sh"
6+
. "$TEST_DIRECTORY/lib-git-daemon.sh"
7+
start_git_daemon
8+
9+
test_expect_success 'create git-accessible repo' '
10+
bare="$GIT_DAEMON_DOCUMENT_ROOT_PATH/repo.git" &&
11+
test_commit one &&
12+
git --bare init "$bare" &&
13+
git push "$bare" HEAD &&
14+
>"$bare/git-daemon-export-ok" &&
15+
git -C "$bare" config daemon.receivepack true
16+
'
17+
18+
test_proto "git://" git "$GIT_DAEMON_URL/repo.git"
19+
20+
test_done

t/t5812-proto-disable-http.sh

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#!/bin/sh
2+
3+
test_description='test disabling of git-over-http in clone/fetch'
4+
. ./test-lib.sh
5+
. "$TEST_DIRECTORY/lib-proto-disable.sh"
6+
. "$TEST_DIRECTORY/lib-httpd.sh"
7+
start_httpd
8+
9+
test_expect_success 'create git-accessible repo' '
10+
bare="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
11+
test_commit one &&
12+
git --bare init "$bare" &&
13+
git push "$bare" HEAD &&
14+
git -C "$bare" config http.receivepack true
15+
'
16+
17+
test_proto "smart http" http "$HTTPD_URL/smart/repo.git"
18+
19+
stop_httpd
20+
test_done

t/t5813-proto-disable-ssh.sh

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#!/bin/sh
2+
3+
test_description='test disabling of git-over-ssh in clone/fetch'
4+
. ./test-lib.sh
5+
. "$TEST_DIRECTORY/lib-proto-disable.sh"
6+
7+
setup_ssh_wrapper
8+
9+
test_expect_success 'setup repository to clone' '
10+
test_commit one &&
11+
mkdir remote &&
12+
git init --bare remote/repo.git &&
13+
git push remote/repo.git HEAD
14+
'
15+
16+
test_proto "host:path" ssh "remote:repo.git"
17+
test_proto "ssh://" ssh "ssh://remote/$PWD/remote/repo.git"
18+
test_proto "git+ssh://" ssh "git+ssh://remote/$PWD/remote/repo.git"
19+
20+
test_done

t/t5814-proto-disable-ext.sh

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#!/bin/sh
2+
3+
test_description='test disabling of remote-helper paths in clone/fetch'
4+
. ./test-lib.sh
5+
. "$TEST_DIRECTORY/lib-proto-disable.sh"
6+
7+
setup_ext_wrapper
8+
9+
test_expect_success 'setup repository to clone' '
10+
test_commit one &&
11+
mkdir remote &&
12+
git init --bare remote/repo.git &&
13+
git push remote/repo.git HEAD
14+
'
15+
16+
test_proto "remote-helper" ext "ext::fake-remote %S repo.git"
17+
18+
test_done

transport-helper.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,8 @@ int transport_helper_init(struct transport *transport, const char *name)
10381038
struct helper_data *data = xcalloc(1, sizeof(*data));
10391039
data->name = name;
10401040

1041+
transport_check_allowed(name);
1042+
10411043
if (getenv("GIT_TRANSPORT_HELPER_DEBUG"))
10421044
debug = 1;
10431045

transport.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,20 @@ static int external_specification_len(const char *url)
909909
return strchr(url, ':') - url;
910910
}
911911

912+
void transport_check_allowed(const char *type)
913+
{
914+
struct string_list allowed = STRING_LIST_INIT_DUP;
915+
const char *v = getenv("GIT_ALLOW_PROTOCOL");
916+
917+
if (!v)
918+
return;
919+
920+
string_list_split(&allowed, v, ':', -1);
921+
if (!unsorted_string_list_has_string(&allowed, type))
922+
die("transport '%s' not allowed", type);
923+
string_list_clear(&allowed, 0);
924+
}
925+
912926
struct transport *transport_get(struct remote *remote, const char *url)
913927
{
914928
const char *helper;
@@ -940,12 +954,14 @@ struct transport *transport_get(struct remote *remote, const char *url)
940954
if (helper) {
941955
transport_helper_init(ret, helper);
942956
} else if (starts_with(url, "rsync:")) {
957+
transport_check_allowed("rsync");
943958
ret->get_refs_list = get_refs_via_rsync;
944959
ret->fetch = fetch_objs_via_rsync;
945960
ret->push = rsync_transport_push;
946961
ret->smart_options = NULL;
947962
} else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {
948963
struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
964+
transport_check_allowed("file");
949965
ret->data = data;
950966
ret->get_refs_list = get_refs_from_bundle;
951967
ret->fetch = fetch_refs_from_bundle;
@@ -957,7 +973,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
957973
|| starts_with(url, "ssh://")
958974
|| starts_with(url, "git+ssh://")
959975
|| starts_with(url, "ssh+git://")) {
960-
/* These are builtin smart transports. */
976+
/*
977+
* These are builtin smart transports; "allowed" transports
978+
* will be checked individually in git_connect.
979+
*/
961980
struct git_transport_data *data = xcalloc(1, sizeof(*data));
962981
ret->data = data;
963982
ret->set_option = NULL;

0 commit comments

Comments
 (0)