http: respect protocol.*.allow=user for http-alternates

The http-walker may fetch the http-alternates (or alternates) file from a remote in order to find more objects. This should count as a "not from the user" use of the protocol. But because we implement the redirection ourselves and feed the new URL to curl, it will use the CURLOPT_PROTOCOLS rules, not the more restrictive CURLOPT_REDIR_PROTOCOLS. The ideal solution would be for each curl request we make to know whether or not is directly from the user or part of an alternates redirect, and then set CURLOPT_PROTOCOLS as appropriate. However, that would require plumbing that information through all of the various layers of the http code. Instead, let's check the protocol at the source: when we are parsing the remote http-alternates file. The only downside is that if there's any mismatch between what protocol we think it is versus what curl thinks it is, it could violate the policy. To address this, we'll make the parsing err on the picky side, and only allow protocols that it can parse definitively. So for example, you can't elude the "http" policy by asking for "HTTP://", even though curl might handle it; we would reject it as unknown. The only unsafe case would be if you have a URL that starts with "http://" but curl interprets as another protocol. That seems like an unlikely failure mode (and we are still protected by our base CURLOPT_PROTOCOL setting, so the worst you could do is trigger one of https, ftp, or ftps). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 14, 2016 at 14:39 UTC abcbdc03895ff3f00280e54af11fee92d6877044
2 files changed +51 -11
http-walker.c
+41 -11
@@ -3,6 +3,7 @@
3 #include "walker.h"
4 #include "http.h"
5 #include "list.h"
6 +#include "transport.h"
7
8 struct alt_base {
9 char *base;
@@ -160,6 +161,32 @@ static void prefetch(struct walker *walker, unsigned char *sha1)
161 #endif
162 }
163
164 +static int is_alternate_allowed(const char *url)
165 +{
166 + const char *protocols[] = {
167 + "http", "https", "ftp", "ftps"
168 + };
169 + int i;
170 +
171 + for (i = 0; i < ARRAY_SIZE(protocols); i++) {
172 + const char *end;
173 + if (skip_prefix(url, protocols[i], &end) &&
174 + starts_with(end, "://"))
175 + break;
176 + }
177 +
178 + if (i >= ARRAY_SIZE(protocols)) {
179 + warning("ignoring alternate with unknown protocol: %s", url);
180 + return 0;
181 + }
182 + if (!is_transport_allowed(protocols[i], 0)) {
183 + warning("ignoring alternate with restricted protocol: %s", url);
184 + return 0;
185 + }
186 +
187 + return 1;
188 +}
189 +
190 static void process_alternates_response(void *callback_data)
191 {
192 struct alternates_request *alt_req =
@@ -274,17 +301,20 @@ static void process_alternates_response(void *callback_data)
301 struct strbuf target = STRBUF_INIT;
302 strbuf_add(&target, base, serverlen);
303 strbuf_add(&target, data + i, posn - i - 7);
277 - warning("adding alternate object store: %s",
278 - target.buf);
279 - newalt = xmalloc(sizeof(*newalt));
280 - newalt->next = NULL;
281 - newalt->base = strbuf_detach(&target, NULL);
282 - newalt->got_indices = 0;
283 - newalt->packs = NULL;
284 -
285 - while (tail->next != NULL)
286 - tail = tail->next;
287 - tail->next = newalt;
304 +
305 + if (is_alternate_allowed(target.buf)) {
306 + warning("adding alternate object store: %s",
307 + target.buf);
308 + newalt = xmalloc(sizeof(*newalt));
309 + newalt->next = NULL;
310 + newalt->base = strbuf_detach(&target, NULL);
311 + newalt->got_indices = 0;
312 + newalt->packs = NULL;
313 +
314 + while (tail->next != NULL)
315 + tail = tail->next;
316 + tail->next = newalt;
317 + }
318 }
319 }
320 i = posn + 1;
t/t5550-http-fetch-dumb.sh
+10
@@ -360,5 +360,15 @@ test_expect_success 'http-alternates cannot point at funny protocols' '
360 clone "$HTTPD_URL/dumb/evil.git" evil-file
361 '
362
363 +test_expect_success 'http-alternates triggers not-from-user protocol check' '
364 + echo "$HTTPD_URL/dumb/victim.git/objects" \
365 + >"$evil/objects/info/http-alternates" &&
366 + test_config_global http.followRedirects true &&
367 + test_must_fail git -c protocol.http.allow=user \
368 + clone $HTTPD_URL/dumb/evil.git evil-user &&
369 + git -c protocol.http.allow=always \
370 + clone $HTTPD_URL/dumb/evil.git evil-user
371 +'
372 +
373 stop_httpd
374 test_done