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>

diff --git a/http-walker.c b/http-walker.c
index c2f81cd6af9d9da0f10fa2e657e6fe62bdbdc4bc..b34b6ace7cd80a482eae16e2063aa9d8960e8729 100644 (file)
--- a/http-walker.c
+++ b/http-walker.c
@@ -3,6 +3,7 @@
 #include "walker.h"
 #include "http.h"
 #include "list.h"
 #include "walker.h"
 #include "http.h"
 #include "list.h"

+#include "transport.h"

 
 struct alt_base {
        char *base;
 
 struct alt_base {
        char *base;


@@ -160,6 +161,32 @@ static void prefetch(struct walker *walker, unsigned char *sha1)
 #endif
 }
 
 #endif
 }
 

+static int is_alternate_allowed(const char *url)
+{
+       const char *protocols[] = {
+               "http", "https", "ftp", "ftps"
+       };
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(protocols); i++) {
+               const char *end;
+               if (skip_prefix(url, protocols[i], &end) &&
+                   starts_with(end, "://"))
+                       break;
+       }
+
+       if (i >= ARRAY_SIZE(protocols)) {
+               warning("ignoring alternate with unknown protocol: %s", url);
+               return 0;
+       }
+       if (!is_transport_allowed(protocols[i], 0)) {
+               warning("ignoring alternate with restricted protocol: %s", url);
+               return 0;
+       }
+
+       return 1;
+}
+

 static void process_alternates_response(void *callback_data)
 {
        struct alternates_request *alt_req =
 static void process_alternates_response(void *callback_data)
 {
        struct alternates_request *alt_req =


@@ -274,17 +301,20 @@ static void process_alternates_response(void *callback_data)
                                struct strbuf target = STRBUF_INIT;
                                strbuf_add(&target, base, serverlen);
                                strbuf_add(&target, data + i, posn - i - 7);
                                struct strbuf target = STRBUF_INIT;
                                strbuf_add(&target, base, serverlen);
                                strbuf_add(&target, data + i, posn - i - 7);

-                               warning("adding alternate object store: %s",
-                                       target.buf);
-                               newalt = xmalloc(sizeof(*newalt));
-                               newalt->next = NULL;
-                               newalt->base = strbuf_detach(&target, NULL);
-                               newalt->got_indices = 0;
-                               newalt->packs = NULL;
-
-                               while (tail->next != NULL)
-                                       tail = tail->next;
-                               tail->next = newalt;
+
+                               if (is_alternate_allowed(target.buf)) {
+                                       warning("adding alternate object store: %s",
+                                               target.buf);
+                                       newalt = xmalloc(sizeof(*newalt));
+                                       newalt->next = NULL;
+                                       newalt->base = strbuf_detach(&target, NULL);
+                                       newalt->got_indices = 0;
+                                       newalt->packs = NULL;
+
+                                       while (tail->next != NULL)
+                                               tail = tail->next;
+                                       tail->next = newalt;
+                               }

                        }
                }
                i = posn + 1;
                        }
                }
                i = posn + 1;

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 22011f0b68870a2390710f580edec60d0d8066dd..c0ee29c65df25604be9501ef07228d317f1d670f 100755 (executable)
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -360,5 +360,15 @@ test_expect_success 'http-alternates cannot point at funny protocols' '
                clone "$HTTPD_URL/dumb/evil.git" evil-file
 '
 
                clone "$HTTPD_URL/dumb/evil.git" evil-file
 '
 

+test_expect_success 'http-alternates triggers not-from-user protocol check' '
+       echo "$HTTPD_URL/dumb/victim.git/objects" \
+               >"$evil/objects/info/http-alternates" &&
+       test_config_global http.followRedirects true &&
+       test_must_fail git -c protocol.http.allow=user \
+               clone $HTTPD_URL/dumb/evil.git evil-user &&
+       git -c protocol.http.allow=always \
+               clone $HTTPD_URL/dumb/evil.git evil-user
+'
+

 stop_httpd
 test_done
 stop_httpd
 test_done