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

8 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: <hidden>
Date: 2016-06-15 22:44:17

Ken Fuchs wrote:

quoted

+       curl_easy_setopt(result, CURLOPT_PROXYAUTH,

(long)CURLAUTH_NTLM);

quoted

+       curl_easy_setopt(result, CURLOPT_PROXYUSERPWD,
+               "<user-id>:<password>");

Daniel Stenberg wrote:

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.)

quoted

It seems that git fetch (via HTLM proxy) works until

quoted

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

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

From: Mike Hommey <hidden>
Date: 2016-06-15 22:44:17

On Tue, Feb 26, 2008 at 05:46:21PM -0600, Ken.Fuchs@bench.com wrote:

quoted hunk

quoted

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.

quoted

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

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

From: Mike Hommey <hidden>
Date: 2016-06-15 22:44:17

On Wed, Feb 27, 2008 at 08:20:12AM +0100, Mike Hommey wrote:

On Tue, Feb 26, 2008 at 05:46:21PM -0600, Ken.Fuchs@bench.com wrote:

quoted

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

[PATCH] Set proxy override with http_init()

From: Mike Hommey <hidden>
Date: 2016-06-15 22:44:17

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 <redacted>
---
 > 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

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

From: Daniel Barkalow <hidden>
Date: 2016-06-15 22:44:18

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.

	-Daniel
*This .sig left intentionally blank*

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

From: Mike Hommey <hidden>
Date: 2016-06-15 22:44:18

On Wed, Feb 27, 2008 at 02:59:38PM -0500, Daniel Barkalow wrote:

On Wed, 27 Feb 2008, Mike Hommey wrote:

quoted

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

[PATCH] Set proxy override with http_init()

From: Mike Hommey <hidden>
Date: 2016-06-15 22:44:18

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 <redacted>
---

 > > 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

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

From: Daniel Barkalow <hidden>
Date: 2016-06-15 22:44:18

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 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 <redacted>

Acked-by: Daniel Barkalow <redacted>

	-Daniel
*This .sig left intentionally blank*

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help