threads / discuss / 12336

RE: FW: git via http protocol _and_ a proxy using NTLM authentication -- git 1.5.4.2 & curl 7.18.0

Subject: RE: FW: git via http protocol _and_ a proxy using NTLM authentication -- git 1.5.4.2 & curl 7.18.0

## tl;dr

9 messages between Feb 26, 2008 and Feb 27, 2008.

replies: 8people: 4as markdown or json

ken.fuchs@bench.com· Feb 26, 2008, 23:46 UTC · lore
Show 6 quoted lines
> Ken Fuchs wrote:
> 
> > +       curl_easy_setopt(result, CURLOPT_PROXYAUTH, 
> (long)CURLAUTH_NTLM);
> > +       curl_easy_setopt(result, CURLOPT_PROXYUSERPWD,
> > +               "<user-id>:<password>");
Daniel Stenberg wrote:
Show 6 quoted lines
> First, you should rather allow any auth and not just the 
> specific one you want.
> 
> Then, the userid and password is probably better passed in 
> embedded in the proxy URL as that's given on the command
> line/environment already. Or as separate arguments.
Agreed.  (I'd just like to get it working first.)
Show 6 quoted lines
> > It seems that git fetch (via HTLM proxy) works until
> >
> >> fatal: Couldn't find remote ref HEAD
> 
> Well, the CURLOPT_PROXY is set in transport.c as well which 
> your patch didn't address.
Thanks, I just did a similar patch to transport.c:
$ diff -u ../git-1.5.4.2/transport.c.orig \
          ../git-1.5.4.2/transport.c
@@ -456,7 +456,8 @@
        if (transport->remote->http_proxy)
                curl_easy_setopt(slot->curl, CURLOPT_PROXY,
                                 transport->remote->http_proxy);
-
+       curl_easy_setopt(slot->curl, CURLOPT_PROXYAUTH,
(long)CURLAUTH_NTLM);
+       curl_easy_setopt(slot->curl, CURLOPT_PROXYUSERPWD,
"<user-id>:<password>");
        if (start_active_slot(slot)) {
                run_active_slot(slot);
                if (results.curl_result != CURLE_OK) {
$

> If that's the case, I figure the verbose output 
> should've shown some auth failures with the proxy?

No, actually the last 2 lines of debug output (unchanged) is:

* Connection #0 to host <proxy domain> left intact
fatal: Couldn't find remote ref HEAD

So, the proxy communication via NTLM authentication seems to be working.
The patch to transport.c did not change anything as far as I can see.

The fatal error is from remote.c.  Perhaps, it also requires some
changes.

--

Example of curl (sans git) working via an NTLM proxy:

$ curl --proxy-ntlm --proxy-user <user-id> \
    --proxy <proxy-domain>:<proxy-port> http://slashdot.org/
Enter proxy password for user '<user-id>': <non-echoed password>
<HTML of /. home page>

Thus, it seems that git could be modified to work via HTLM
authentication, but my simple changes to http.c and transport.c
above are probably not sufficient.

Suggestions are most welcome.

Thanks,

Ken Fuchs
Mike Hommey· Feb 27, 2008, 07:20 UTC · re: ken.fuchs@bench.com · lore

Re: FW: git via http protocol _and_ a proxy using NTLM authentication -- git 1.5.4.2 & curl 7.18.0

On Tue, Feb 26, 2008 at 05:46:21PM -0600, Ken.Fuchs@bench.com wrote:
Show 20 quoted lines
> > Well, the CURLOPT_PROXY is set in transport.c as well which 
> > your patch didn't address.
> 
> Thanks, I just did a similar patch to transport.c:
> 
> $ diff -u ../git-1.5.4.2/transport.c.orig \
>           ../git-1.5.4.2/transport.c
> @@ -456,7 +456,8 @@
>         if (transport->remote->http_proxy)
>                 curl_easy_setopt(slot->curl, CURLOPT_PROXY,
>                                  transport->remote->http_proxy);
> -
> +       curl_easy_setopt(slot->curl, CURLOPT_PROXYAUTH,
> (long)CURLAUTH_NTLM);
> +       curl_easy_setopt(slot->curl, CURLOPT_PROXYUSERPWD,
> "<user-id>:<password>");
>         if (start_active_slot(slot)) {
>                 run_active_slot(slot);
>                 if (results.curl_result != CURLE_OK) {
> $

Starting with curl 7.14.1, you're supposed to be able to use the http://user:pass@proxy/ syntax, though I'm not sure it deals well with NTLM domains. You can probably leave CURLOPT_PROXYUSERPWD out if you set your proxy url correctly.

As for CURLOPT_PROXYAUTH, it would be better to set it from another config.

Note that remote.<name>.proxy config doesn't work as expected, you should use http.proxy which just work (and the change in transport.c is useless, then). I have, as part of by http-refactoring topic, a patch for remote.<name>.proxy to work better, though it doesn't support changing the proxy authentication method.

Show 13 quoted lines
> > If that's the case, I figure the verbose output 
> > should've shown some auth failures with the proxy?
> 
> No, actually the last 2 lines of debug output (unchanged) is:
> 
> * Connection #0 to host <proxy domain> left intact
> fatal: Couldn't find remote ref HEAD
> 
> So, the proxy communication via NTLM authentication seems to be working.
> The patch to transport.c did not change anything as far as I can see.
> 
> The fatal error is from remote.c.  Perhaps, it also requires some
> changes.
Does your remote have a HEAD ref ?
Mike
Mike Hommey· Feb 27, 2008, 07:29 UTC · re: Mike Hommey · lore

Re: FW: git via http protocol _and_ a proxy using NTLM authentication -- git 1.5.4.2 & curl 7.18.0

On Wed, Feb 27, 2008 at 08:20:12AM +0100, Mike Hommey wrote:
Show 29 quoted lines
> On Tue, Feb 26, 2008 at 05:46:21PM -0600, Ken.Fuchs@bench.com wrote:
> > > Well, the CURLOPT_PROXY is set in transport.c as well which 
> > > your patch didn't address.
> > 
> > Thanks, I just did a similar patch to transport.c:
> > 
> > $ diff -u ../git-1.5.4.2/transport.c.orig \
> >           ../git-1.5.4.2/transport.c
> > @@ -456,7 +456,8 @@
> >         if (transport->remote->http_proxy)
> >                 curl_easy_setopt(slot->curl, CURLOPT_PROXY,
> >                                  transport->remote->http_proxy);
> > -
> > +       curl_easy_setopt(slot->curl, CURLOPT_PROXYAUTH,
> > (long)CURLAUTH_NTLM);
> > +       curl_easy_setopt(slot->curl, CURLOPT_PROXYUSERPWD,
> > "<user-id>:<password>");
> >         if (start_active_slot(slot)) {
> >                 run_active_slot(slot);
> >                 if (results.curl_result != CURLE_OK) {
> > $
> 
> Starting with curl 7.14.1, you're supposed to be able to use the
> http://user:pass@proxy/ syntax, though I'm not sure it deals well with
> NTLM domains. You can probably leave CURLOPT_PROXYUSERPWD out if you
> set your proxy url correctly.
> 
> As for CURLOPT_PROXYAUTH, it would be better to set it from another
> config.
Or we should set it to CURLOPT_AUTHANY by default.
Mike
Mike Hommey· Feb 27, 2008, 07:36 UTC · re: Mike Hommey · lore

[PATCH] Set proxy override with http_init()

In transport.c, proxy setting (the one from the remote conf) was set through curl_easy_setopt() call, while http.c already does the same with the http.proxy setting. We now just use this infrastructure instead, and make http_init() now take the proxy url as argument.

At the same time, we make get_http_walker() take a proxy argument too, and pass it to http_init(), which makes remote defined proxy be used for more than get_refs_via_curl().

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 > Note that remote.<name>.proxy config doesn't work as expected, you
 > should use http.proxy which just work (and the change in transport.c is
 > useless, then). I have, as part of by http-refactoring topic, a patch
 > for remote.<name>.proxy to work better, though it doesn't support
 > changing the proxy authentication method.
 
 And here is said patch.
 http-push.c   |    2 +-
 http-walker.c |    4 ++--
 http.c        |   10 +++++++++-
 http.h        |    2 +-
 transport.c   |    9 ++++-----
 walker.h      |    2 +-
 6 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/http-push.c b/http-push.c
index 0beb740..04e056d 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2240,7 +2240,7 @@ int main(int argc, char **argv)
 
 	memset(remote_dir_exists, -1, 256);
 
-	http_init();
+	http_init(NULL);
 
 	no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
 
diff --git a/http-walker.c b/http-walker.c
index 2c37868..02be6c8 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -902,13 +902,13 @@ static void cleanup(struct walker *walker)
 	curl_slist_free_all(data->no_pragma_header);
 }
 
-struct walker *get_http_walker(const char *url)
+struct walker *get_http_walker(const char *url, const char *proxy)
 {
 	char *s;
 	struct walker_data *data = xmalloc(sizeof(struct walker_data));
 	struct walker *walker = xmalloc(sizeof(struct walker));
 
-	http_init();
+	http_init(proxy);
 
 	data->no_pragma_header = curl_slist_append(NULL, "Pragma:");
 
diff --git a/http.c b/http.c
index 519621a..89194d7 100644
--- a/http.c
+++ b/http.c
@@ -219,13 +219,16 @@ static CURL* get_curl_handle(void)
 	return result;
 }
 
-void http_init(void)
+void http_init(const char *proxy)
 {
 	char *low_speed_limit;
 	char *low_speed_time;
 
 	curl_global_init(CURL_GLOBAL_ALL);
 
+	if (proxy)
+		curl_http_proxy = xstrdup(proxy);
+
 	pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
 
 #ifdef USE_CURL_MULTI
@@ -315,6 +318,11 @@ void http_cleanup(void)
 
 	curl_slist_free_all(pragma_header);
 	pragma_header = NULL;
+
+	if (curl_http_proxy) {
+		free(curl_http_proxy);
+		curl_http_proxy = NULL;
+	}
 }
 
 struct active_request_slot *get_active_slot(void)
diff --git a/http.h b/http.h
index 9bab2c8..dcd5cea 100644
--- a/http.h
+++ b/http.h
@@ -83,7 +83,7 @@ extern void add_fill_function(void *data, int (*fill)(void *));
 extern void step_active_slots(void);
 #endif
 
-extern void http_init(void);
+extern void http_init(const char *proxy);
 extern void http_cleanup(void);
 
 extern int data_received;
diff --git a/transport.c b/transport.c
index 397983d..26f0f02 100644
--- a/transport.c
+++ b/transport.c
@@ -442,7 +442,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	struct ref *last_ref = NULL;
 
 	if (!transport->data)
-		transport->data = get_http_walker(transport->url);
+		transport->data = get_http_walker(transport->url,
+						transport->remote->http_proxy);
 
 	refs_url = xmalloc(strlen(transport->url) + 11);
 	sprintf(refs_url, "%s/info/refs", transport->url);
@@ -453,9 +454,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, refs_url);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
-	if (transport->remote->http_proxy)
-		curl_easy_setopt(slot->curl, CURLOPT_PROXY,
-				 transport->remote->http_proxy);
 
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
@@ -509,7 +507,8 @@ static int fetch_objs_via_curl(struct transport *transport,
 				 int nr_objs, struct ref **to_fetch)
 {
 	if (!transport->data)
-		transport->data = get_http_walker(transport->url);
+		transport->data = get_http_walker(transport->url,
+						transport->remote->http_proxy);
 	return fetch_objs_via_walker(transport, nr_objs, to_fetch);
 }
 
diff --git a/walker.h b/walker.h
index ea2c363..2cc448a 100644
--- a/walker.h
+++ b/walker.h
@@ -32,6 +32,6 @@ int walker_fetch(struct walker *impl, int targets, char **target,
 
 void walker_free(struct walker *walker);
 
-struct walker *get_http_walker(const char *url);
+struct walker *get_http_walker(const char *url, const char *proxy);
 
 #endif /* WALKER_H */
-- 
1.5.4.1.48.g0d77
Junio C Hamano· Feb 27, 2008, 19:18 UTC · re: Mike Hommey · lore

Re: [PATCH] Set proxy override with http_init()

Mike Hommey <mh@glandium.org> writes:
Show 11 quoted lines
> In transport.c, proxy setting (the one from the remote conf) was set through
> curl_easy_setopt() call, while http.c already does the same with the
> http.proxy setting. We now just use this infrastructure instead, and make
> http_init() now take the proxy url as argument.
>
> At the same time, we make get_http_walker() take a proxy argument too, and
> pass it to http_init(), which makes remote defined proxy be used for more
> than get_refs_via_curl().
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
Thanks.  I am kicking this to Daniel for a review ;-).
Daniel Barkalow· Feb 27, 2008, 19:59 UTC · re: Mike Hommey · lore

Re: [PATCH] Set proxy override with http_init()

On Wed, 27 Feb 2008, Mike Hommey wrote:
Show 8 quoted lines
> In transport.c, proxy setting (the one from the remote conf) was set through
> curl_easy_setopt() call, while http.c already does the same with the
> http.proxy setting. We now just use this infrastructure instead, and make
> http_init() now take the proxy url as argument.
> 
> At the same time, we make get_http_walker() take a proxy argument too, and
> pass it to http_init(), which makes remote defined proxy be used for more
> than get_refs_via_curl().

It's a good idea, but maybe http_init() (and the call chain leading up to it) should take the struct remote, so that it can get anything else you might also have there in the future? For that matter, is it intentional that http-push ignore the proxy setting, which is equally available in http-push, but not used? If so, it should probably get a comment.

	-Daniel
*This .sig left intentionally blank*
Mike Hommey· Feb 27, 2008, 20:05 UTC · re: Daniel Barkalow · lore

Re: [PATCH] Set proxy override with http_init()

On Wed, Feb 27, 2008 at 02:59:38PM -0500, Daniel Barkalow wrote:
Show 16 quoted lines
> On Wed, 27 Feb 2008, Mike Hommey wrote:
> 
> > In transport.c, proxy setting (the one from the remote conf) was set through
> > curl_easy_setopt() call, while http.c already does the same with the
> > http.proxy setting. We now just use this infrastructure instead, and make
> > http_init() now take the proxy url as argument.
> > 
> > At the same time, we make get_http_walker() take a proxy argument too, and
> > pass it to http_init(), which makes remote defined proxy be used for more
> > than get_refs_via_curl().
> 
> It's a good idea, but maybe http_init() (and the call chain leading up to 
> it) should take the struct remote, so that it can get anything else you 
> might also have there in the future? For that matter, is it intentional 
> that http-push ignore the proxy setting, which is equally available in 
> http-push, but not used? If so, it should probably get a comment.

It is intentional, for the moment. I'll rework the patch to take the struct remote, and add a comment.

Mike
Mike Hommey· Feb 27, 2008, 20:35 UTC · re: Mike Hommey · lore

[PATCH] Set proxy override with http_init()

In transport.c, proxy setting (the one from the remote conf) was set through curl_easy_setopt() call, while http.c already does the same with the http.proxy setting. We now just use this infrastructure instead, and make http_init() now take the struct remote as argument so that it can take the http_proxy setting from there, and any other property that would be added later.

At the same time, we make get_http_walker() take a struct remote argument too, and pass it to http_init(), which makes remote defined proxy be used for more than get_refs_via_curl().

We leave out http-fetch and http-push, which don't use remotes for the moment, purposefully.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 > > It's a good idea, but maybe http_init() (and the call chain leading up to
 > > it) should take the struct remote, so that it can get anything else you
 > > might also have there in the future? For that matter, is it intentional
 > > that http-push ignore the proxy setting, which is equally available in
 > > http-push, but not used? If so, it should probably get a comment.
 > 
 > It is intentional, for the moment. I'll rework the patch to take the
 > struct remote, and add a comment.
 And here you are. Changing const char * to struct remote * had the nice side
 effect to make me detect a missing change in builtin-http-fetch.c.
 builtin-http-fetch.c |    2 +-
 http-push.c          |    2 +-
 http-walker.c        |    4 ++--
 http.c               |   10 +++++++++-
 http.h               |    3 ++-
 transport.c          |    9 ++++-----
 walker.h             |    4 +++-
 7 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/builtin-http-fetch.c b/builtin-http-fetch.c
index 7f450c6..48128c6 100644
--- a/builtin-http-fetch.c
+++ b/builtin-http-fetch.c
@@ -59,7 +59,7 @@ int cmd_http_fetch(int argc, const char **argv, const char *prefix)
 		url = rewritten_url;
 	}
 
-	walker = get_http_walker(url);
+	walker = get_http_walker(url, NULL);
 	walker->get_tree = get_tree;
 	walker->get_history = get_history;
 	walker->get_all = get_all;
diff --git a/http-push.c b/http-push.c
index 0beb740..04e056d 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2240,7 +2240,7 @@ int main(int argc, char **argv)
 
 	memset(remote_dir_exists, -1, 256);
 
-	http_init();
+	http_init(NULL);
 
 	no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
 
diff --git a/http-walker.c b/http-walker.c
index 2c37868..7bda34d 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -902,13 +902,13 @@ static void cleanup(struct walker *walker)
 	curl_slist_free_all(data->no_pragma_header);
 }
 
-struct walker *get_http_walker(const char *url)
+struct walker *get_http_walker(const char *url, struct remote *remote)
 {
 	char *s;
 	struct walker_data *data = xmalloc(sizeof(struct walker_data));
 	struct walker *walker = xmalloc(sizeof(struct walker));
 
-	http_init();
+	http_init(remote);
 
 	data->no_pragma_header = curl_slist_append(NULL, "Pragma:");
 
diff --git a/http.c b/http.c
index 519621a..64c53cd 100644
--- a/http.c
+++ b/http.c
@@ -219,13 +219,16 @@ static CURL* get_curl_handle(void)
 	return result;
 }
 
-void http_init(void)
+void http_init(struct remote *remote)
 {
 	char *low_speed_limit;
 	char *low_speed_time;
 
 	curl_global_init(CURL_GLOBAL_ALL);
 
+	if (remote && remote->http_proxy)
+		curl_http_proxy = xstrdup(remote->http_proxy);
+
 	pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
 
 #ifdef USE_CURL_MULTI
@@ -315,6 +318,11 @@ void http_cleanup(void)
 
 	curl_slist_free_all(pragma_header);
 	pragma_header = NULL;
+
+	if (curl_http_proxy) {
+		free(curl_http_proxy);
+		curl_http_proxy = NULL;
+	}
 }
 
 struct active_request_slot *get_active_slot(void)
diff --git a/http.h b/http.h
index 9bab2c8..04169d5 100644
--- a/http.h
+++ b/http.h
@@ -7,6 +7,7 @@
 #include <curl/easy.h>
 
 #include "strbuf.h"
+#include "remote.h"
 
 /*
  * We detect based on the cURL version if multi-transfer is
@@ -83,7 +84,7 @@ extern void add_fill_function(void *data, int (*fill)(void *));
 extern void step_active_slots(void);
 #endif
 
-extern void http_init(void);
+extern void http_init(struct remote *remote);
 extern void http_cleanup(void);
 
 extern int data_received;
diff --git a/transport.c b/transport.c
index 397983d..86e0374 100644
--- a/transport.c
+++ b/transport.c
@@ -442,7 +442,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	struct ref *last_ref = NULL;
 
 	if (!transport->data)
-		transport->data = get_http_walker(transport->url);
+		transport->data = get_http_walker(transport->url,
+						transport->remote);
 
 	refs_url = xmalloc(strlen(transport->url) + 11);
 	sprintf(refs_url, "%s/info/refs", transport->url);
@@ -453,9 +454,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, refs_url);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
-	if (transport->remote->http_proxy)
-		curl_easy_setopt(slot->curl, CURLOPT_PROXY,
-				 transport->remote->http_proxy);
 
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
@@ -509,7 +507,8 @@ static int fetch_objs_via_curl(struct transport *transport,
 				 int nr_objs, struct ref **to_fetch)
 {
 	if (!transport->data)
-		transport->data = get_http_walker(transport->url);
+		transport->data = get_http_walker(transport->url,
+						transport->remote);
 	return fetch_objs_via_walker(transport, nr_objs, to_fetch);
 }
 
diff --git a/walker.h b/walker.h
index ea2c363..e1d40de 100644
--- a/walker.h
+++ b/walker.h
@@ -1,6 +1,8 @@
 #ifndef WALKER_H
 #define WALKER_H
 
+#include "remote.h"
+
 struct walker {
 	void *data;
 	int (*fetch_ref)(struct walker *, char *ref, unsigned char *sha1);
@@ -32,6 +34,6 @@ int walker_fetch(struct walker *impl, int targets, char **target,
 
 void walker_free(struct walker *walker);
 
-struct walker *get_http_walker(const char *url);
+struct walker *get_http_walker(const char *url, struct remote *remote);
 
 #endif /* WALKER_H */
-- 
1.5.4.3.195.g48d21.dirty
Daniel Barkalow· Feb 27, 2008, 20:39 UTC · re: Mike Hommey · lore

Re: [PATCH] Set proxy override with http_init()

On Wed, 27 Feb 2008, Mike Hommey wrote:
Show 15 quoted lines
> In transport.c, proxy setting (the one from the remote conf) was set through
> curl_easy_setopt() call, while http.c already does the same with the
> http.proxy setting. We now just use this infrastructure instead, and make
> http_init() now take the struct remote as argument so that it can take the
> http_proxy setting from there, and any other property that would be added
> later.
> 
> At the same time, we make get_http_walker() take a struct remote argument
> too, and pass it to http_init(), which makes remote defined proxy be used
> for more than get_refs_via_curl().
> 
> We leave out http-fetch and http-push, which don't use remotes for the
> moment, purposefully.
> 
> Signed-off-by: Mike Hommey <mh@glandium.org>
Acked-by: Daniel Barkalow <barkalow@iabervon.org>
	-Daniel
*This .sig left intentionally blank*

← back to recent threads