[PATCH] Allow git to use any HTTP authentication method.

[PATCH] Allow git to use any HTTP authentication method.

[Lénaïc Huard]

[-- Attachment #1: Type: Text/Plain, Size: 1113 bytes --]

Hello,

As I need to access some of my git repositories behind a corporate company 
firewall, I use the http access method. And, as I don't want my passwords to be 
sent in clear text over the network, I have configured my web server to use « 
Digest » authentication instead of the old « Basic » authentication.
This authentication method is now well handled by modern software.

Unfortunately, current git version only handles « Basic » authentication.
When attempting to access my repository, I get the following error message:

error: The requested URL returned error: 401 while accessing 
http://XXX@YYY.ZZ/test.git/info/refs

The web server, on its side, has refused the transaction because of the wrong 
authentication method used:

Digest: client used wrong authentication scheme `Basic': /test.git/info/refs

The attached patch makes git configure libcurl to negotiate the most suitable 
HTTP authentication method.
Thanks to that patch, I manage to clone and fetch my git repository hosted on 
my web server requesting an authentication through the « Digest  » method.

Lénaïc.

[-- Attachment #2: 0001-Allow-git-to-use-any-HTTP-authentication-method.patch --]
[-- Type: text/x-patch, Size: 966 bytes --]

From 2acab3ae894c3ea835279a864e654e1c5e956e80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C3=A9na=C3=AFc=20Huard?= <lenaic@lhuard.fr.eu.org>
Date: Mon, 28 Dec 2009 10:52:35 +0100
Subject: [PATCH] Allow git to use any HTTP authentication method.

By default, libcurl performs "Basic" HTTP authentication.
This method transmits passwords in clear text.
libcurl needs some settings in order to use a safest HTTP authentication
method like "Digest" for example.
---
 http.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index ed6414a..2d9df76 100644
--- a/http.c
+++ b/http.c
@@ -233,6 +233,10 @@ static CURL *get_curl_handle(void)
 
 	init_curl_http_auth(result);
 
+#if LIBCURL_VERSION_NUM >= 0x070a06
+	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
+#endif
+
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
-- 
1.6.5.7

Re: [PATCH] Allow git to use any HTTP authentication method.

[Martin Storsjö]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 582 bytes --]

Hi Lénaïc,

On Mon, 28 Dec 2009, Lénaïc Huard wrote:

> The attached patch makes git configure libcurl to negotiate the most suitable 
> HTTP authentication method.
> Thanks to that patch, I manage to clone and fetch my git repository hosted on 
> my web server requesting an authentication through the « Digest  » method.

Something similar has already been queued for inclusion, and is available 
in the branch 'next', in commit b8ac923010484908d8426cb8ded5ad7e8c21a7f6. 
The patch available there requires you to set http.authAny for the libcurl 
option to be enabled.

// Martin

Re: [PATCH] Allow git to use any HTTP authentication method.

[Tay Ray Chuan]

Hi,

On Mon, Dec 28, 2009 at 8:09 PM, Martin Storsjö <martin@martin.st> wrote:
> Hi Lénaďc,
>
> On Mon, 28 Dec 2009, Lénaďc Huard wrote:
>
>> The attached patch makes git configure libcurl to negotiate the most suitable
>> HTTP authentication method.
>> Thanks to that patch, I manage to clone and fetch my git repository hosted on
>> my web server requesting an authentication through the « Digest  » method.
>
> Something similar has already been queued for inclusion, and is available
> in the branch 'next', in commit b8ac923010484908d8426cb8ded5ad7e8c21a7f6.
> The patch available there requires you to set http.authAny for the libcurl
> option to be enabled.

...or setting the environment variable GIT_HTTP_AUTH_ANY.

(by the way, Martin was referring to setting http.authAny in your git
configuration.)

-- 
Cheers,
Ray Chuan

Re: [PATCH] Allow git to use any HTTP authentication method.

[Matthieu Moy]

Lénaïc Huard <lenaic@lhuard.fr.eu.org> writes:

> The attached patch makes git configure libcurl to negotiate the most suitable 
> HTTP authentication method.

Thanks for your contribution.

Read other people's reply about the need for this patch.

Other than that, please read git/Documentation/SubmittingPatches in
Git's source code. In short: inline patches, don't attach them, and
use Signed-Off-By to acknowledge that your patch can be legally
included in Git's official version.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Allow git to use any HTTP authentication method.

[Shawn O. Pearce]

Tay Ray Chuan <rctay89@gmail.com> wrote:
> On Mon, Dec 28, 2009 at 8:09 PM, Martin Storsj?? <martin@martin.st> wrote:
> > On Mon, 28 Dec 2009, L??na??c Huard wrote:
> >
> >> The attached patch makes git configure libcurl to negotiate the most suitable
> >> HTTP authentication method.
...
> > Something similar has already been queued for inclusion, and is available
> > in the branch 'next', in commit b8ac923010484908d8426cb8ded5ad7e8c21a7f6.
> > The patch available there requires you to set http.authAny for the libcurl
> > option to be enabled.

Uh, stupid question, but why must we enable this option?  I don't
have to enable something in my browser before I use digest auth to
visit a website, why do I need to enable it in git?

How does one use git clone with an http:// URL with digest
authentication?  Its not obvious to the user that you would need
to first export an obtuse environment variable to get something
that should Just Work(tm).

Yes, I realize you may need to perform an extra HTTP request to
start the transaction, but why aren't we doing that?  Isn't it only
1 additional request to discover the desired authentication method?

-- 
Shawn.

Re: [PATCH] Allow git to use any HTTP authentication method.

[Martin Storsjö]

On Mon, 28 Dec 2009, Shawn O. Pearce wrote:

> Uh, stupid question, but why must we enable this option?  I don't
> have to enable something in my browser before I use digest auth to
> visit a website, why do I need to enable it in git?
> 
> How does one use git clone with an http:// URL with digest
> authentication?  Its not obvious to the user that you would need
> to first export an obtuse environment variable to get something
> that should Just Work(tm).
> 
> Yes, I realize you may need to perform an extra HTTP request to
> start the transaction, but why aren't we doing that?  Isn't it only
> 1 additional request to discover the desired authentication method?

Initially when I added support for this, curl sessions weren't reused, so 
every single request had to be duplicated if authentication was used, 
adding quite a bit of overhead.

Now that sessions are reused properly, I tend to agree that this should be 
enabled automatically.

Any other opinions on this, Tay or Junio?

Should I send in a new patch that removes the http.authAny option and 
always enables this, or send a rewritten version of the patch that already 
is in 'next'?

// Martin

Re: [PATCH] Allow git to use any HTTP authentication method.

[Shawn O. Pearce]

Martin Storsj? <martin@martin.st> wrote:
> Initially when I added support for this, curl sessions weren't reused, so 
> every single request had to be duplicated if authentication was used, 
> adding quite a bit of overhead.
> 
> Now that sessions are reused properly, I tend to agree that this should be 
> enabled automatically.

Oh, that makes sense, thanks for the explanation.
 
> Should I send in a new patch that removes the http.authAny option and 
> always enables this, or send a rewritten version of the patch that already 
> is in 'next'?

I'm not Junio, but I would suggest sending in a new patch series,
and asking Junio politely to revert the one that is currently in
next before merging in the new series.

If we really are killing http.authAny before it hits master, there
is no reason for it to appear in the final project history.

-- 
Shawn.

Re: [PATCH] Allow git to use any HTTP authentication method.

[Tay Ray Chuan]

Hi,

On Mon, Dec 28, 2009 at 11:53 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Martin Storsj? <martin@martin.st> wrote:
>> Should I send in a new patch that removes the http.authAny option and
>> always enables this, or send a rewritten version of the patch that already
>> is in 'next'?
>
> I'm not Junio, but I would suggest sending in a new patch series,
> and asking Junio politely to revert the one that is currently in
> next before merging in the new series.
>
> If we really are killing http.authAny before it hits master, there
> is no reason for it to appear in the final project history.

hmm, a few days back Junio (added to Cc list) sent out an email
regarding branch shuffling and dropping topics from 'next'. Junio,
could we piggyback on this?

-- 
Cheers,
Ray Chuan

Re: [PATCH] Allow git to use any HTTP authentication method.

[Junio C Hamano]

Tay Ray Chuan <rctay89@gmail.com> writes:

> On Mon, Dec 28, 2009 at 11:53 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
>> Martin Storsj? <martin@martin.st> wrote:
>>> Should I send in a new patch that removes the http.authAny option and
>>> always enables this, or send a rewritten version of the patch that already
>>> is in 'next'?
>>
>> I'm not Junio, but I would suggest sending in a new patch series,
>> and asking Junio politely to revert the one that is currently in
>> next before merging in the new series.
>>
>> If we really are killing http.authAny before it hits master, there
>> is no reason for it to appear in the final project history.
>
> hmm, a few days back Junio (added to Cc list) sent out an email
> regarding branch shuffling and dropping topics from 'next'. Junio,
> could we piggyback on this?

If people want to go that way that is fine by me, but unlike the ones that
are _ejected_ from next without trace, if we are going to have the primary
feature the patch introduces and changing only a minor detail of external
interface, I don't think we gain much by hinding that story from the
history, especially for something that has been cooking in 'next'.

A separate follow-up commit would be more honest about the feature's
history.  Also a follow-up patch to remove conditionals is much easier to
review than a resend of a rewritten patch, especially when the original
was reviewed adequately for its primary codepath to implement the feature.

Would it be just a matter of queueing something like this on top of
b8ac923 (Add an option for using any HTTP authentication scheme, not only
basic, 2009-11-27)?

-- >8 -- 
From: "Shawn O. Pearce" <spearce@spearce.org>,
Subject: Remove http.authAny

Back when the feature to use different HTTP authentication methods was
originally written, it needed an extra HTTP request for everything when
the feature was in effect, because we didn't reuse curl sessions.

However, b8ac923 (Add an option for using any HTTP authentication scheme,
not only basic, 2009-11-27) builds on top of an updated codebase that does
reuse curl sessions; there is no need to manually avoid the extra overhead
by making this configurable anymore.

---
 Documentation/config.txt |    7 -------
 http.c                   |   17 +----------------
 2 files changed, 1 insertions(+), 23 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a54ede3..b77d66d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1158,13 +1158,6 @@ http.noEPSV::
 	support EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV'
 	environment variable. Default is false (curl will use EPSV).
 
-http.authAny::
-	Allow any HTTP authentication method, not only basic. Enabling
-	this lowers the performance slightly, by having to do requests
-	without any authentication to discover the authentication method
-	to use. Can be overridden by the 'GIT_HTTP_AUTH_ANY'
-	environment variable. Default is false.
-
 i18n.commitEncoding::
 	Character encoding the commit messages are stored in; git itself
 	does not care per se, but this information is necessary e.g. when
diff --git a/http.c b/http.c
index aeb69b3..01e0fdc 100644
--- a/http.c
+++ b/http.c
@@ -40,9 +40,6 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static char *user_name, *user_pass;
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-static int curl_http_auth_any = 0;
-#endif
 
 #if LIBCURL_VERSION_NUM >= 0x071700
 /* Use CURLOPT_KEYPASSWD as is */
@@ -197,12 +194,6 @@ static int http_options(const char *var, const char *value, void *cb)
 			http_post_buffer = LARGE_PACKET_MAX;
 		return 0;
 	}
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-	if (!strcmp("http.authany", var)) {
-		curl_http_auth_any = git_config_bool(var, value);
-		return 0;
-	}
-#endif
 
 	/* Fall back on the default ones */
 	return git_default_config(var, value, cb);
@@ -254,8 +245,7 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
 #endif
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-	if (curl_http_auth_any)
-		curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
+	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
 	init_curl_http_auth(result);
@@ -408,11 +398,6 @@ void http_init(struct remote *remote)
 	if (getenv("GIT_CURL_FTP_NO_EPSV"))
 		curl_ftp_no_epsv = 1;
 
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-	if (getenv("GIT_HTTP_AUTH_ANY"))
-		curl_http_auth_any = 1;
-#endif
-
 	if (remote && remote->url && remote->url[0]) {
 		http_auth_init(remote->url[0]);
 		if (!ssl_cert_password_required &&

Re: [PATCH] Allow git to use any HTTP authentication method.

[Martin Storsjö]

On Mon, 28 Dec 2009, Junio C Hamano wrote:

> Would it be just a matter of queueing something like this on top of
> b8ac923 (Add an option for using any HTTP authentication scheme, not only
> basic, 2009-11-27)?

Looks good to me:

Acked-by: Martin Storsjo <martin@martin.st>

// Martin

Re: [PATCH] Allow git to use any HTTP authentication method.

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> -- >8 -- 
> From: "Shawn O. Pearce" <spearce@spearce.org>,
> Subject: Remove http.authAny

Ack.

But I'm not sure I should be the author, you did all of the legwork
and therefore should get credit for it.  :-)

-- 
Shawn.