Commit Graph

296 Commits

Author SHA1 Message Date
Tom G. Christensen
dd5df538b5 http: use a feature check to enable GSSAPI delegation control
Turn the version check into a feature check to ensure this functionality
is also enabled with vendor supported curl versions where the feature
may have been backported.

Signed-off-by: Tom G. Christensen <tgc@jupiterrise.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-11 15:12:41 -07:00
Tom G. Christensen
f18777ba6e http: fix handling of missing CURLPROTO_*
Commit aeae4db1 refactored the handling of the curl protocol
restriction support into a function but failed to add a version
check for older versions of curl that lack CURLPROTO_* support.

Add the missing check and at the same time convert it to a feature
check instead of a version based check.  This is done to ensure that
vendor supported curl versions that have had CURLPROTO_* support
backported are handled correctly.

Signed-off-by: Tom G. Christensen <tgc@jupiterrise.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-11 15:12:07 -07:00
Junio C Hamano
50f03c6676 Merge branch 'ab/free-and-null'
A common pattern to free a piece of memory and assign NULL to the
pointer that used to point at it has been replaced with a new
FREE_AND_NULL() macro.

* ab/free-and-null:
  *.[ch] refactoring: make use of the FREE_AND_NULL() macro
  coccinelle: make use of the "expression" FREE_AND_NULL() rule
  coccinelle: add a rule to make "expression" code use FREE_AND_NULL()
  coccinelle: make use of the "type" FREE_AND_NULL() rule
  coccinelle: add a rule to make "type" code use FREE_AND_NULL()
  git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
2017-06-24 14:28:41 -07:00
Ævar Arnfjörð Bjarmason
6a83d90207 coccinelle: make use of the "type" FREE_AND_NULL() rule
Apply the result of the just-added coccinelle rule. This manually
excludes a few occurrences, mostly things that resulted in many
FREE_AND_NULL() on one line, that'll be manually fixed in a subsequent
change.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-16 12:44:03 -07:00
Brandon Williams
b2141fc1d2 config: don't include config.h by default
Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 12:56:22 -07:00
Junio C Hamano
4c01f67d91 Merge branch 'dt/http-postbuffer-can-be-large'
Allow the http.postbuffer configuration variable to be set to a
size that can be expressed in size_t, which can be larger than
ulong on some platforms.

* dt/http-postbuffer-can-be-large:
  http.postbuffer: allow full range of ssize_t values
2017-04-23 22:07:45 -07:00
Junio C Hamano
6b51cb6181 Merge branch 'sr/http-proxy-configuration-fix'
"http.proxy" set to an empty string is used to disable the usage of
proxy.  We broke this early last year.

* sr/http-proxy-configuration-fix:
  http: fix the silent ignoring of proxy misconfiguraion
  http: honor empty http.proxy option to bypass proxy
2017-04-23 22:07:44 -07:00
David Turner
37ee680d9b http.postbuffer: allow full range of ssize_t values
Unfortunately, in order to push some large repos where a server does
not support chunked encoding, the http postbuffer must sometimes
exceed two gigabytes.  On a 64-bit system, this is OK: we just malloc
a larger buffer.

This means that we need to use CURLOPT_POSTFIELDSIZE_LARGE to set the
buffer size.

Signed-off-by: David Turner <dturner@twosigma.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-13 18:24:32 -07:00
Sergey Ryazanov
ae51d91105 http: fix the silent ignoring of proxy misconfiguraion
Earlier, the whole http.proxy option string was passed to curl without
any preprocessing so curl could complain about the invalid proxy
configuration.

After the commit 372370f167 ("http: use credential API to handle proxy
authentication", 2016-01-26), if the user specified an invalid HTTP
proxy option in the configuration, then the option parsing silently
fails and NULL will be passed to curl as a proxy. This forces curl to
fall back to detecting the proxy configuration from the environment,
causing the http.proxy option ignoring.

Fix this issue by checking the proxy option parsing result. If parsing
failed then print an error message and die. Such behaviour allows the
user to quickly figure the proxy misconfiguration and correct it.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-13 15:51:33 -07:00
Sergey Ryazanov
57415089bd http: honor empty http.proxy option to bypass proxy
Curl distinguishes between an empty proxy address and a NULL proxy
address. In the first case it completely disables proxy usage, but if
the proxy address option is NULL then curl attempts to determine the
proxy address from the http_proxy environment variable.

According to the documentation, if the http.proxy option is set to an
empty string, git should bypass proxy and connect to the server
directly:

    export http_proxy=http://network-proxy/
    cd ~/foobar-project
    git config remote.origin.proxy ""
    git fetch

Previously, proxy host was configured by one line:

    curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);

Commit 372370f167 ("http: use credential API to handle proxy
authentication", 2016-01-26) parses the proxy option, then extracts the
proxy host address and updates the curl configuration, making the
previous call a noop:

    credential_from_url(&proxy_auth, curl_http_proxy);
    curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host);

But if the proxy option is empty then the proxy host field becomes NULL.
This forces curl to fall back to detecting the proxy configuration from
the environment, causing the http.proxy option to not work anymore.

Fix this issue by explicitly handling http.proxy being set the empty
string. This also makes the code a bit more clear and should help us
avoid such regressions in the future.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-13 15:51:19 -07:00
Jeff King
1a168e5c86 convert unchecked snprintf into xsnprintf
These calls to snprintf should always succeed, because their
input is small and fixed. Let's use xsnprintf to make sure
this is the case (and to make auditing for actual truncation
easier).

These could be candidates for turning into heap buffers, but
they fall into a few broad categories that make it not worth
doing:

  - formatting single numbers is simple enough that we can
    see the result should fit

  - the size of a sha1 is likewise well-known, and I didn't
    want to cause unnecessary conflicts with the ongoing
    process to convert these constants to GIT_MAX_HEXSZ

  - the interface for curl_errorstr is dictated by curl

Signed-off-by: Jeff King <peff@peff.net>
2017-03-30 14:59:50 -07:00
Junio C Hamano
68e12d7d97 Merge branch 'jt/http-base-url-update-upon-redirect' into maint
When a redirected http transport gets an error during the
redirected request, we ignored the error we got from the server,
and ended up giving a not-so-useful error message.

* jt/http-base-url-update-upon-redirect:
  http: attempt updating base URL only if no error
2017-03-16 13:56:42 -07:00
Junio C Hamano
d880bfd947 Merge branch 'jk/http-auth' into maint
Reduce authentication round-trip over HTTP when the server supports
just a single authentication method.

* jk/http-auth:
  http: add an "auto" mode for http.emptyauth
  http: restrict auth methods to what the server advertises
2017-03-16 13:56:41 -07:00
Junio C Hamano
d0f549f403 Merge branch 'jt/http-base-url-update-upon-redirect'
When a redirected http transport gets an error during the
redirected request, we ignored the error we got from the server,
and ended up giving a not-so-useful error message.

* jt/http-base-url-update-upon-redirect:
  http: attempt updating base URL only if no error
2017-03-10 13:24:23 -08:00
Junio C Hamano
92718f57c2 Merge branch 'jk/http-auth'
Reduce authentication round-trip over HTTP when the server supports
just a single authentication method.

* jk/http-auth:
  http: add an "auto" mode for http.emptyauth
  http: restrict auth methods to what the server advertises
2017-03-10 13:24:23 -08:00
Jonathan Tan
8e27391a5f http: attempt updating base URL only if no error
http.c supports HTTP redirects of the form

  http://foo/info/refs?service=git-upload-pack
  -> http://anything
  -> http://bar/info/refs?service=git-upload-pack

(that is to say, as long as the Git part of the path and the query
string is preserved in the final redirect destination, the intermediate
steps can have any URL). However, if one of the intermediate steps
results in an HTTP exception, a confusing "unable to update url base
from redirection" message is printed instead of a Curl error message
with the HTTP exception code.

This was introduced by 2 commits. Commit c93c92f ("http: update base
URLs when we see redirects", 2013-09-28) introduced a best-effort
optimization that required checking if only the "base" part of the URL
differed between the initial request and the final redirect destination,
but it performed the check before any HTTP status checking was done. If
something went wrong, the normal code path was still followed, so this
did not cause any confusing error messages until commit 6628eb4 ("http:
always update the base URL for redirects", 2016-12-06), which taught
http to die if the non-"base" part of the URL differed.

Therefore, teach http to check the HTTP status before attempting to
check if only the "base" part of the URL differed. This commit teaches
http_request_reauth to return early without updating options->base_url
upon an error; the only invoker of this function that passes a non-NULL
"options" is remote-curl.c (through "http_get_strbuf"), which only uses
options->base_url for an informational message in the situations that
this commit cares about (that is, when the return value is not HTTP_OK).

The included test checks that the redirect scheme at the beginning of
this commit message works, and that returning a 502 in the middle of the
redirect scheme produces the correct result. Note that this is different
from the test in commit 6628eb4 ("http: always update the base URL for
redirects", 2016-12-06) in that this commit tests that a Git-shaped URL
(http://.../info/refs?service=git-upload-pack) works, whereas commit
6628eb4 tests that a non-Git-shaped URL
(http://.../info/refs/foo?service=git-upload-pack) does not work (even
though Git is processing that URL) and is an error that is fatal, not
silently swallowed.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-28 11:35:53 -08:00
Jeff King
40a18fc77c http: add an "auto" mode for http.emptyauth
This variable needs to be specified to make some types of
non-basic authentication work, but ideally this would just
work out of the box for everyone.

However, simply setting it to "1" by default introduces an
extra round-trip for cases where it _isn't_ useful. We end
up sending a bogus empty credential that the server rejects.

Instead, let's introduce an automatic mode, that works like
this:

  1. We won't try to send the bogus credential on the first
     request. We'll wait to get an HTTP 401, as usual.

  2. After seeing an HTTP 401, the empty-auth hack will kick
     in only when we know there is an auth method available
     that might make use of it (i.e., something besides
     "Basic" or "Digest").

That should make it work out of the box, without incurring
any extra round-trips for people hitting Basic-only servers.

This _does_ incur an extra round-trip if you really want to
use "Basic" but your server advertises other methods (the
emptyauth hack will kick in but fail, and then Git will
actually ask for a password).

The auto mode may incur an extra round-trip over setting
http.emptyauth=true, because part of the emptyauth hack is
to feed this blank password to curl even before we've made a
single request.

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-27 10:35:24 -08:00
Jeff King
840398feb8 http: restrict auth methods to what the server advertises
By default, we tell curl to use CURLAUTH_ANY, which does not
limit its set of auth methods. However, this results in an
extra round-trip to the server when authentication is
required. After we've fed the credential to curl, it wants
to probe the server to find its list of available methods
before sending an Authorization header.

We can shortcut this by limiting our http_auth_methods by
what the server told us it supports. In some cases (such as
when the server only supports Basic), that lets curl skip
the extra probe request.

The end result should look the same to the user, but you can
use GIT_TRACE_CURL to verify the sequence of requests:

  GIT_TRACE_CURL=1 \
  git ls-remote https://example.com/repo.git \
  2>&1 >/dev/null |
  egrep '(Send|Recv) header: (GET|HTTP|Auth)'

Before this patch, hitting a Basic-only server like
github.com results in:

  Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1
  Recv header: HTTP/1.1 401 Authorization Required
  Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1
  Recv header: HTTP/1.1 401 Authorization Required
  Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1
  Send header: Authorization: Basic <redacted>
  Recv header: HTTP/1.1 200 OK

And after:

  Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1
  Recv header: HTTP/1.1 401 Authorization Required
  Send header: GET /repo.git/info/refs?service=git-upload-pack HTTP/1.1
  Send header: Authorization: Basic <redacted>
  Recv header: HTTP/1.1 200 OK

The possible downsides are:

  - This only helps for a Basic-only server; for a server
    with multiple auth options, curl may still send a probe
    request to see which ones are available (IOW, there's no
    way to say "don't probe, I already know what the server
    will say").

  - The http_auth_methods variable is global, so this will
    apply to all further requests. That's acceptable for
    Git's usage of curl, though, which also treats the
    credentials as global. I.e., in any given program
    invocation we hit only one conceptual server (we may be
    redirected at the outset, but in that case that's whose
    auth_avail field we'd see).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-23 11:11:56 -08:00
Junio C Hamano
5ce6f51ff7 Merge branch 'jk/http-walker-limit-redirect' into maint
Update the error messages from the dumb-http client when it fails
to obtain loose objects; we used to give sensible error message
only upon 404 but we now forbid unexpected redirects that needs to
be reported with something sensible.

* jk/http-walker-limit-redirect:
  http-walker: complain about non-404 loose object errors
  http: treat http-alternates like redirects
  http: make redirects more obvious
  remote-curl: rename shadowed options variable
  http: always update the base URL for redirects
  http: simplify update_url_from_redirect
2017-01-17 14:49:29 -08:00
Junio C Hamano
9d540e9726 Merge branch 'bw/transport-protocol-policy'
Finer-grained control of what protocols are allowed for transports
during clone/fetch/push have been enabled via a new configuration
mechanism.

* bw/transport-protocol-policy:
  http: respect protocol.*.allow=user for http-alternates
  transport: add from_user parameter to is_transport_allowed
  http: create function to get curl allowed protocols
  transport: add protocol policy config option
  http: always warn if libcurl version is too old
  lib-proto-disable: variable name fix
2016-12-27 00:11:41 -08:00
Junio C Hamano
da72ee87fb Merge branch 'jk/http-walker-limit-redirect'
Update the error messages from the dumb-http client when it fails
to obtain loose objects; we used to give sensible error message
only upon 404 but we now forbid unexpected redirects that needs to
be reported with something sensible.

* jk/http-walker-limit-redirect:
  http-walker: complain about non-404 loose object errors
2016-12-19 14:45:32 -08:00
Junio C Hamano
8a2882f23e Merge branch 'jk/http-walker-limit-redirect-2.9'
Transport with dumb http can be fooled into following foreign URLs
that the end user does not intend to, especially with the server
side redirects and http-alternates mechanism, which can lead to
security issues.  Tighten the redirection and make it more obvious
to the end user when it happens.

* jk/http-walker-limit-redirect-2.9:
  http: treat http-alternates like redirects
  http: make redirects more obvious
  remote-curl: rename shadowed options variable
  http: always update the base URL for redirects
  http: simplify update_url_from_redirect
2016-12-19 14:45:32 -08:00
Brandon Williams
a768a02265 transport: add from_user parameter to is_transport_allowed
Add a from_user parameter to is_transport_allowed() to allow http to be
able to distinguish between protocol restrictions for redirects versus
initial requests.  CURLOPT_REDIR_PROTOCOLS can now be set differently
from CURLOPT_PROTOCOLS to disallow use of protocols with the "user"
policy in redirects.

This change allows callers to query if a transport protocol is allowed,
given that the caller knows that the protocol is coming from the user
(1) or not from the user (0) such as redirects in libcurl.  If unknown a
-1 should be provided which falls back to reading
`GIT_PROTOCOL_FROM_USER` to determine if the protocol came from the
user.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-15 09:29:13 -08:00
Brandon Williams
aeae4db174 http: create function to get curl allowed protocols
Move the creation of an allowed protocols whitelist to a helper
function. This will be useful when we need to compute the set of
allowed protocols differently for normal and redirect cases.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-15 09:29:13 -08:00
Brandon Williams
f962ddf6ed http: always warn if libcurl version is too old
Always warn if libcurl version is too old because:

1. Even without a protocol whitelist, newer versions of curl have all
   non-standard protocols disabled by default.
2. A future patch will introduce default "known-good" and "known-bad"
   protocols which are allowed/disallowed by 'is_transport_allowed'
   which older version of libcurl can't respect.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-15 09:28:37 -08:00
Jeff King
3680f16f9d http-walker: complain about non-404 loose object errors
Since commit 17966c0a6 (http: avoid disconnecting on 404s
for loose objects, 2016-07-11), we turn off curl's
FAILONERROR option and instead manually deal with failing
HTTP codes.

However, the logic to do so only recognizes HTTP 404 as a
failure. This is probably the most common result, but if we
were to get another code, the curl result remains CURLE_OK,
and we treat it as success. We still end up detecting the
failure when we try to zlib-inflate the object (which will
fail), but instead of reporting the HTTP error, we just
claim that the object is corrupt.

Instead, let's catch anything in the 300's or above as an
error (300's are redirects which are not an error at the
HTTP level, but are an indication that we've explicitly
disabled redirects, so we should treat them as such; we
certainly don't have the resulting object content).

Note that we also fill in req->errorstr, which we didn't do
before. Without FAILONERROR, curl will not have filled this
in, and it will remain a blank string. This never mattered
for the 404 case, because in the logic below we hit the
"missing_target()" branch and print nothing. But for other
errors, we'd want to say _something_, if only to fill in the
blank slot in the error message.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-06 12:43:34 -08:00
Junio C Hamano
43ec089eea Merge branch 'ew/http-walker' into jk/http-walker-limit-redirect
* ew/http-walker:
  list: avoid incompatibility with *BSD sys/queue.h
  http-walker: reduce O(n) ops with doubly-linked list
  http: avoid disconnecting on 404s for loose objects
  http-walker: remove unused parameter from fetch_object
2016-12-06 12:43:23 -08:00
Jeff King
cb4d2d35c4 http: treat http-alternates like redirects
The previous commit made HTTP redirects more obvious and
tightened up the default behavior. However, there's another
way for a server to ask a git client to fetch arbitrary
content: by having an http-alternates file (or a regular
alternates file, which is used as a backup).

Similar to the HTTP redirect case, a malicious server can
claim to have refs pointing at object X, return a 404 when
the client asks for X, but point to some other URL via
http-alternates, which the client will transparently fetch.
The end result is that it looks from the user's perspective
like the objects came from the malicious server, as the
other URL is not mentioned at all.

Worse, because we feed the new URL to curl ourselves, the
usual protocol restrictions do not kick in (neither curl's
default of disallowing file://, nor the protocol
whitelisting in f4113cac0 (http: limit redirection to
protocol-whitelist, 2015-09-22).

Let's apply the same rules here as we do for HTTP redirects.
Namely:

  - unless http.followRedirects is set to "always", we will
    not follow remote redirects from http-alternates (or
    alternates) at all

  - set CURLOPT_PROTOCOLS alongside CURLOPT_REDIR_PROTOCOLS
    restrict ourselves to a known-safe set and respect any
    user-provided whitelist.

  - mention alternate object stores on stderr so that the
    user is aware another source of objects may be involved

The first item may prove to be too restrictive. The most
common use of alternates is to point to another path on the
same server. While it's possible for a single-server
redirect to be an attack, it takes a fairly obscure setup
(victim and evil repository on the same host, host speaks
dumb http, and evil repository has access to edit its own
http-alternates file).

So we could make the checks more specific, and only cover
cross-server redirects. But that means parsing the URLs
ourselves, rather than letting curl handle them. This patch
goes for the simpler approach. Given that they are only used
with dumb http, http-alternates are probably pretty rare.
And there's an escape hatch: the user can allow redirects on
a specific server by setting http.<url>.followRedirects to
"always".

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-06 12:32:48 -08:00
Jeff King
50d3413740 http: make redirects more obvious
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 <jannh@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-06 12:32:48 -08:00
Jeff King
6628eb41db http: always update the base URL for redirects
If a malicious server redirects the initial ref
advertisement, it may be able to leak sha1s from other,
unrelated servers that the client has access to. For
example, imagine that Alice is a git user, she has access to
a private repository on a server hosted by Bob, and Mallory
runs a malicious server and wants to find out about Bob's
private repository.

Mallory asks Alice to clone an unrelated repository from her
over HTTP. When Alice's client contacts Mallory's server for
the initial ref advertisement, the server issues an HTTP
redirect for Bob's server. Alice contacts Bob's server and
gets the ref advertisement for the private repository. If
there is anything to fetch, she then follows up by asking
the server for one or more sha1 objects. But who is the
server?

If it is still Mallory's server, then Alice will leak the
existence of those sha1s to her.

Since commit c93c92f30 (http: update base URLs when we see
redirects, 2013-09-28), the client usually rewrites the base
URL such that all further requests will go to Bob's server.
But this is done by textually matching the URL. If we were
originally looking for "http://mallory/repo.git/info/refs",
and we got pointed at "http://bob/other.git/info/refs", then
we know that the right root is "http://bob/other.git".

If the redirect appears to change more than just the root,
we punt and continue to use the original server. E.g.,
imagine the redirect adds a URL component that Bob's server
will ignore, like "http://bob/other.git/info/refs?dummy=1".

We can solve this by aborting in this case rather than
silently continuing to use Mallory's server. In addition to
protecting from sha1 leakage, it's arguably safer and more
sane to refuse a confusing redirect like that in general.
For example, part of the motivation in c93c92f30 is
avoiding accidentally sending credentials over clear http,
just to get a response that says "try again over https". So
even in a non-malicious case, we'd prefer to err on the side
of caution.

The downside is that it's possible this will break a
legitimate but complicated server-side redirection scheme.
The setup given in the newly added test does work, but it's
convoluted enough that we don't need to care about it. A
more plausible case would be a server which redirects a
request for "info/refs?service=git-upload-pack" to just
"info/refs" (because it does not do smart HTTP, and for some
reason really dislikes query parameters).  Right now we
would transparently downgrade to dumb-http, but with this
patch, we'd complain (and the user would have to set
GIT_SMART_HTTP=0 to fetch).

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-06 12:32:48 -08:00
Jeff King
986d7f4d37 http: simplify update_url_from_redirect
This function looks for a common tail between what we asked
for and where we were redirected to, but it open-codes the
comparison. We can avoid some confusing subtractions by
using strip_suffix_mem().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-06 12:32:48 -08:00
Junio C Hamano
c6400bf8d5 Merge branch 'dt/http-empty-auth'
http.emptyauth configuration is a way to allow an empty username to
pass when attempting to authenticate using mechanisms like
Kerberos.  We took an unspecified (NULL) username and sent ":"
(i.e. no username, no password) to CURLOPT_USERPWD, but did not do
the same when the username is explicitly set to an empty string.

* dt/http-empty-auth:
  http: http.emptyauth should allow empty (not just NULL) usernames
2016-10-17 13:25:18 -07:00
Junio C Hamano
fbfe878f97 Merge branch 'ps/http-gssapi-cred-delegation'
In recent versions of cURL, GSSAPI credential delegation is
disabled by default due to CVE-2011-2192; introduce a configuration
to selectively allow enabling this.

* ps/http-gssapi-cred-delegation:
  http: control GSSAPI credential delegation
2016-10-06 14:53:11 -07:00
David Turner
5275c3081c http: http.emptyauth should allow empty (not just NULL) usernames
When using Kerberos authentication with newer versions of libcurl,
CURLOPT_USERPWD must be set to a value, even if it is an empty value.
The value is never sent to the server.  Previous versions of libcurl
did not require this variable to be set.  One way that some users
express the empty username/password is http://:@gitserver.example.com,
which http.emptyauth was designed to support.  Another, equivalent,
URL is http://@gitserver.example.com.  The latter leads to a username
of zero-length, rather than a NULL username, but CURLOPT_USERPWD still
needs to be set (if http.emptyauth is set).  Do so.

Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-04 12:02:00 -07:00
Petr Stodulka
26a7b23429 http: control GSSAPI credential delegation
Delegation of credentials is disabled by default in libcurl since
version 7.21.7 due to security vulnerability CVE-2011-2192. Which
makes troubles with GSS/kerberos authentication when delegation
of credentials is required. This can be changed with option
CURLOPT_GSSAPI_DELEGATION in libcurl with set expected parameter
since libcurl version 7.22.0.

This patch provides new configuration variable http.delegation
which corresponds to curl parameter "--delegation" (see man 1 curl).

The following values are supported:

* none (default).
* policy
* always

Signed-off-by: Petr Stodulka <pstodulk@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-29 20:39:23 -07:00
Junio C Hamano
e007a094d4 Merge branch 'ew/http-do-not-forget-to-call-curl-multi-remove-handle' into maint
The http transport (with curl-multi option, which is the default
these days) failed to remove curl-easy handle from a curlm session,
which led to unnecessary API failures.

* ew/http-do-not-forget-to-call-curl-multi-remove-handle:
  http: always remove curl easy from curlm session on release
  http: consolidate #ifdefs for curl_multi_remove_handle
  http: warn on curl_multi_add_handle failures
2016-09-29 16:49:39 -07:00
Junio C Hamano
35ec7fd479 Merge branch 'jk/fix-remote-curl-url-wo-proto' into maint
"git fetch http::/site/path" did not die correctly and segfaulted
instead.

* jk/fix-remote-curl-url-wo-proto:
  remote-curl: handle URLs without protocol
2016-09-29 16:49:38 -07:00
Junio C Hamano
ac8ddd7ba3 Merge branch 'ew/http-do-not-forget-to-call-curl-multi-remove-handle'
The http transport (with curl-multi option, which is the default
these days) failed to remove curl-easy handle from a curlm session,
which led to unnecessary API failures.

* ew/http-do-not-forget-to-call-curl-multi-remove-handle:
  http: always remove curl easy from curlm session on release
  http: consolidate #ifdefs for curl_multi_remove_handle
  http: warn on curl_multi_add_handle failures
2016-09-21 15:15:23 -07:00
Junio C Hamano
c13f458d86 Merge branch 'jk/fix-remote-curl-url-wo-proto'
"git fetch http::/site/path" did not die correctly and segfaulted
instead.

* jk/fix-remote-curl-url-wo-proto:
  remote-curl: handle URLs without protocol
2016-09-15 14:11:15 -07:00
Eric Wong
2abc848d54 http: always remove curl easy from curlm session on release
We must call curl_multi_remove_handle when releasing the slot to
prevent subsequent calls to curl_multi_add_handle from failing
with CURLM_ADDED_ALREADY (in curl 7.32.1+; older versions
returned CURLM_BAD_EASY_HANDLE)

Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 13:34:04 -07:00
Eric Wong
d8b6b84df0 http: consolidate #ifdefs for curl_multi_remove_handle
I find #ifdefs makes code difficult-to-follow.

An early version of this patch had error checking for
curl_multi_remove_handle calls, but caused some tests (e.g.
t5541) to fail under curl 7.26.0 on old Debian wheezy.

Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 13:34:03 -07:00
Eric Wong
9f1b58842a http: warn on curl_multi_add_handle failures
This will be useful for tracking down curl usage errors.

Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 13:34:01 -07:00
Jeff King
d63ed6ef24 remote-curl: handle URLs without protocol
Generally remote-curl would never see a URL that did not
have "proto:" at the beginning, as that is what tells git to
run the "git-remote-proto" helper (and git-remote-http, etc,
are aliases for git-remote-curl).

However, the special syntax "proto::something" will run
git-remote-proto with only "something" as the URL. So a
malformed URL like:

  http::/example.com/repo.git

will feed the URL "/example.com/repo.git" to
git-remote-http. The resulting URL has no protocol, but the
code added by 372370f (http: use credential API to handle
proxy authentication, 2016-01-26) does not handle this case
and segfaults.

For the purposes of this code, we don't really care what the
exact protocol; only whether or not it is https. So let's
just assume that a missing protocol is not, and curl will
handle the real error (which is that the URL is nonsense).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-08 11:23:43 -07:00
Junio C Hamano
940622bc8b Merge branch 'rs/use-strbuf-addstr'
* rs/use-strbuf-addstr:
  use strbuf_addstr() instead of strbuf_addf() with "%s"
  use strbuf_addstr() for adding constant strings to a strbuf
2016-08-08 14:48:41 -07:00
René Scharfe
bc57b9c0cc use strbuf_addstr() instead of strbuf_addf() with "%s"
Call strbuf_addstr() for adding a simple string to a strbuf instead of
using the heavier strbuf_addf().  This is shorter and documents the
intent more clearly.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-05 15:09:25 -07:00
Junio C Hamano
4067a45438 Merge branch 'ew/http-walker'
Dumb http transport on the client side has been optimized.

* ew/http-walker:
  list: avoid incompatibility with *BSD sys/queue.h
  http-walker: reduce O(n) ops with doubly-linked list
  http: avoid disconnecting on 404s for loose objects
  http-walker: remove unused parameter from fetch_object
2016-08-03 15:10:24 -07:00
Eric Wong
17966c0a63 http: avoid disconnecting on 404s for loose objects
404s are common when fetching loose objects on static HTTP
servers, and reestablishing a connection for every single
404 adds additional latency.

Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-12 15:17:42 -07:00
Junio C Hamano
2f84df2ca0 Merge branch 'ep/http-curl-trace'
HTTP transport gained an option to produce more detailed debugging
trace.

* ep/http-curl-trace:
  imap-send.c: introduce the GIT_TRACE_CURL enviroment variable
  http.c: implement the GIT_TRACE_CURL environment variable
2016-07-06 13:38:06 -07:00
Junio C Hamano
e646a82ce2 Merge branch 'bn/http-cookiefile-config' into maint
"http.cookieFile" configuration variable clearly wants a pathname,
but we forgot to treat it as such by e.g. applying tilde expansion.

* bn/http-cookiefile-config:
  http: expand http.cookieFile as a path
  Documentation: config: improve word ordering for http.cookieFile
2016-05-31 14:08:28 -07:00
Elia Pinto
74c682d3c6 http.c: implement the GIT_TRACE_CURL environment variable
Implement the GIT_TRACE_CURL environment variable to allow a
greater degree of detail of GIT_CURL_VERBOSE, in particular
the complete transport header and all the data payload exchanged.
It might be useful if a particular situation could require a more
thorough debugging analysis. Document the new GIT_TRACE_CURL
environment variable.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-24 15:48:18 -07:00