Re: [PATCH] http(s): automatically try NTLM authentication first

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 22, 2017, 20:19 UTC
Message-ID: <xmqqpoiaasgj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170222173936.25016-1-dturner@twosigma.com>

David Turner <dturner@twosigma.com> writes:

Show 17 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> It is common in corporate setups to have permissions managed via a
> domain account. That means that the user does not really have to log in
> when accessing a central repository via https://, but that the login
> credentials are used to authenticate with that repository.
>
> The common way to do that used to require empty credentials, i.e. hitting
> Enter twice when being asked for user name and password, or by using the
> very funny notation https://:@server/repository
>
> A recent commit (5275c3081c (http: http.emptyauth should allow empty (not
> just NULL) usernames, 2016-10-04)) broke that usage, though, all of a
> sudden requiring users to set http.emptyAuth = true.
>
> Which brings us to the bigger question why http.emptyAuth defaults to
> false, to begin with.

This is a valid question, and and I do not see it explicitly asked in the thread:

https://public-inbox.org/git/CAPig+cSphEu3iRJrkdBA+BRhi9HnopLJnKOHVuGhUqavtV1RXg@mail.gmail.com/#t

even though there is a hint of it already there.

Show 7 quoted lines

> It would be one thing if cURL would not let the user specify credentials
> interactively after attempting NTLM authentication (i.e. login
> credentials), but that is not the case.
>
> It would be another thing if attempting NTLM authentication was not
> usually what users need to do when trying to authenticate via https://.
> But that is also not the case.

Some other possible worries we may have had I can think of are:

 - With this enabled unconditionally, would we leak some information?

 - With this enabled unconditionally, would we always incur an extra
   roundtrip for people who are not running NTLM at all?

I do not think the former is the case, but what would I know (adding a few people involved in the original thread to CC: ;-)

Show 30 quoted lines

>  Documentation/config.txt | 3 ++-
>  http.c                   | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index fc5a28a320..b0da64ed33 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1742,7 +1742,8 @@ http.emptyAuth::
>  	Attempt authentication without seeking a username or password.  This
>  	can be used to attempt GSS-Negotiate authentication without specifying
>  	a username in the URL, as libcurl normally requires a username for
> -	authentication.
> +	authentication.  Default is true, since if this fails, git will fall
> +	back to asking the user for their username/password.
>  
>  http.delegation::
>  	Control GSSAPI credential delegation. The delegation is disabled
> diff --git a/http.c b/http.c
> index 90a1c0f113..943e630ea6 100644
> --- a/http.c
> +++ b/http.c
> @@ -109,7 +109,7 @@ static int curl_save_cookies;
>  struct credential http_auth = CREDENTIAL_INIT;
>  static int http_proactive_auth;
>  static const char *user_agent;
> -static int curl_empty_auth;
> +static int curl_empty_auth = 1;
>  
>  enum http_follow_config http_follow_config = HTTP_FOLLOW_INITIAL;