Re: [PATCH 4/6] http: make redirects more obvious

[Ramsay Jones]

On 01/12/16 09:04, Jeff King wrote:
> We instruct curl to always follow HTTP redirects. This is
> convenient, but it creates opportunities for malicious
> servers to create confusing situations. For instance,
> imagine Alice is a git user with access to a private
> repository on Bob's server. Mallory runs her own server and

Ahem, so Mallory is female? (-blush-) :(

ATB,
Ramsay Jones

[PATCH 4/6] http: make redirects more obvious

[Jeff King]

We instruct curl to always follow HTTP redirects. This is
convenient, but it creates opportunities for malicious
servers to create confusing situations. For instance,
imagine Alice is a git user with access to a private
repository on Bob's server. Mallory runs her own server and
wants to access objects from Bob's repository.

Mallory may try a few tricks that involve asking Alice to
clone from her, build on top, and then push the result:

  1. Mallory may simply redirect all fetch requests to Bob's
 server. Git will transparently follow those redirects
 and fetch Bob's history, which Alice may believe she
 got from Mallory. The subsequent push seems like it is
 just feeding Mallory back her own objects, but is
 actually leaking Bob's objects. There is nothing in
 git's output to indicate that Bob's repository was
 involved at all.

 The downside (for Mallory) of this attack is that Alice
 will have received Bob's entire repository, and is
 likely to notice that when building on top of it.

  2. If Mallory happens to know the sha1 of some object X in
 Bob's repository, she can instead build her own history
 that references that object. She then runs a dumb http
 server, and Alice's client will fetch each object
 individually. When it asks for X, Mallory redirects her
 to Bob's server. The end result is that Alice obtains
 objects from Bob, but they may be buried deep in
 history. Alice is less likely to notice.

Both of these attacks are fairly hard to pull off. There's a
social component in getting Mallory to convince Alice to
work with her. Alice may be prompted for credentials in
accessing Bob's repository (but not always, if she is using
a credential helper that caches). Attack (1) requires a
certain amount of obliviousness on Alice's part while making
a new commit. Attack (2) requires that Mallory knows a sha1
in Bob's repository, that Bob's server supports dumb http,
and that the object in question is loose on Bob's server.

But we can probably make things a bit more obvious without
any loss of functionality. This patch does two things to
that end.

First, when we encounter a whole-repo redirect during the
initial ref discovery, we now inform the user on stderr,
making attack (1) much more obvious.

Second, the decision to follow redirects is now
configurable. The truly paranoid can set the new
http.followRedirects to false to avoid any redirection
entirely. But for a more practical default, we will disallow
redirects only after the initial ref discovery. This is
enough to thwart attacks similar to (2), while still
allowing the common use of redirects at the repository
level. Since c93c92f30 (http: update base URLs when we see
redirects, 2013-09-28) we re-root all further requests from
the redirect destination, which should generally mean that
no further redirection is necessary.

As an escape hatch, in case there really is a server that
needs to redirect individual requests, the user can set
http.followRedirects to "true" (and this can be done on a
per-server basis via http.*.followRedirects config).

Reported-by: Jann Horn 
Signed-off-by: Jeff King 
---
 Documentation/config.txt   | 10 ++
 http.c | 33 ++---
 http.h | 10 +-
 remote-curl.c  |  4 
 t/lib-httpd/apache.conf|  6 ++
 t/t5550-http-fetch-dumb.sh | 23 +++
 6 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a0ab66aae..d51182a06 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1891,6 +1891,16 @@ http.userAgent::
of common USER_AGENT strings (but not including those like git/1.7.1).
Can be overridden by the `GIT_HTTP_USER_AGENT` environment variable.
 
+http.followRedirects::
+   Whether git should follow HTTP redirects. If set to `true`, git
+   will transparently follow any redirect issued by a server it
+   encounters. If set to `false`, git will treat all redirects as
+   errors. If set to `initial`, git will follow redirects only for
+   the initial request to a remote, but not for subsequent
+   follow-up HTTP requests. Since git uses the redirected URL as
+   the base for the follow-up requests, this is generally
+   sufficient. The default is `initial`.
+
 http..*::
Any of the http.* options above can be applied selectively to some URLs.
For a config key to match a URL, each element of the config key is
diff --git a/http.c b/http.c
index ba03beead..825118481 100644
--- a/http.c
+++ b/http.c
@@ -111,6 +111,8 @@ static int http_proactive_auth;
 static const char *user_agent;
 static int curl_empty_auth;
 
+enum http_follow_config http_follow_config = HTTP_FOLLOW_INITIAL;
+
 #if LIBCURL_VERSION_NUM >= 0x071700
 /* Use CURLOPT_KEYPASSWD as is */
 #elif