git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: David Turner <dturner@twosigma.com>,
	git@vger.kernel.org, sandals@crustytoothpaste.net,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH 1/2] http: restrict auth methods to what the server advertises
Date: Wed, 22 Feb 2017 18:34:37 -0500	[thread overview]
Message-ID: <20170222233436.l5c3ya53cl7y5x7z@sigill.intra.peff.net> (raw)
In-Reply-To: <20170222233333.dx5lknw4fpopu5hy@sigill.intra.peff.net>

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>
---
 http.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/http.c b/http.c
index 90a1c0f11..a05609766 100644
--- a/http.c
+++ b/http.c
@@ -1347,6 +1347,8 @@ static int handle_curl_result(struct slot_results *results)
 		} else {
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
 			http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+			if (results->auth_avail)
+				http_auth_methods &= results->auth_avail;
 #endif
 			return HTTP_REAUTH;
 		}
-- 
2.12.0.rc2.597.g959f68882


  reply	other threads:[~2017-02-22 23:42 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-22 17:39 [PATCH] http(s): automatically try NTLM authentication first David Turner
2017-02-22 20:19 ` Junio C Hamano
2017-02-22 21:04   ` David Turner
2017-02-22 21:16     ` Junio C Hamano
2017-02-22 21:34       ` Jeff King
2017-02-23 17:08         ` Johannes Schindelin
2017-02-23 19:06           ` Junio C Hamano
2017-02-23 19:42           ` Jeff King
2017-02-23 20:37             ` Junio C Hamano
2017-02-23 20:48               ` Jeff King
2017-02-25 11:51                 ` Johannes Schindelin
2017-02-22 23:34     ` brian m. carlson
2017-02-22 23:42       ` Jeff King
2017-02-23  2:15         ` Junio C Hamano
2017-02-23 19:11         ` Junio C Hamano
2017-02-23 19:35           ` Jeff King
2017-02-23  1:03       ` David Turner
2017-02-23  4:19         ` brian m. carlson
2017-02-23  9:13         ` Mantas Mikulėnas
2017-02-22 21:06   ` Jeff King
2017-02-22 21:25     ` Junio C Hamano
2017-02-22 21:35       ` Jeff King
2017-02-22 21:57         ` Junio C Hamano
2017-02-22 21:58           ` Jeff King
2017-02-22 22:35             ` Junio C Hamano
2017-02-22 23:33               ` Jeff King
2017-02-22 23:34                 ` Jeff King [this message]
2017-02-22 23:40                 ` [PATCH 2/2] http: add an "auto" mode for http.emptyauth Jeff King
2017-02-23  1:16                   ` David Turner
2017-02-23  1:37                     ` Jeff King
2017-02-23 16:31                       ` David Turner
2017-02-23 19:44                         ` Jeff King
2017-02-23 20:05                           ` David Turner
2017-02-25 11:48                       ` Johannes Schindelin
2017-02-25 19:15                         ` Jeff King
2017-02-25 19:18                           ` [PATCH] " Jeff King
2017-02-27 18:35                             ` Junio C Hamano
2017-02-28 10:18                               ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170222233436.l5c3ya53cl7y5x7z@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dturner@twosigma.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=sandals@crustytoothpaste.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).