Re: [PATCH][v2] http authentication via prompts (with correct line lengths)

Mike Gaffney [off-list ref] writes:

quoted

quoted

+static const char *curl_http_username = NULL;
+static const char *curl_http_password = NULL;
+

Please do not introduce new initializations of static variables to 0 or
NULL.  As a clean-up, before your patch, you can send in a patch to fix
existing such initializations.

...
Or do you mean that I can send in a patch to fix other static variables
(not mine) which are being initialized to NULL?

Yeah, a preparatory patch to clean things up, like the one I sent out
earlier this evening, was what I meant.

2) Being that I'm not a big C guy, I'm not sure the best way to go about 
parsing the username out of the URL to pull it into a variable to pass
to CURLOPT_USERPASS. Any advice from the community would be greatly
appreciated.

I am sort of a C guy, but I am by no means a libcurl person.  A quick and
dirty patch is attached, which is partly based on yours, but is stripped
of version dependency and also I suspect it handles only the http-walker
side.  It is on top of the two clean-up patch I sent this evening.

It hasn't seen any test, but I just ran this once:

    $ git clone http://junio@my.private.machine/test-repo.git/

from a repository that requires authentication but I have no .netrc and no
http.password configuration; I was asked for the password once, of course.

3) From my experience with curl, many of the options do
not work the same across versions or platforms. For example, the new
CURLOPT_USERNAME/PASSWORD options worked fine in 7.19.4 on cygwin but not
on FC9, which is why I used the older USERPWD. Also, my curl never prompted
me for the password when I supplied a username in the URL which is what 
prompted me to do this patch in the first place. As such, I think it is
better to pull the username & password prompting logic into git make this 
stable and fix the bug. 

Heh, 7.19.4 was only released on a few days ago if I am reading its
download page correctly.

The version of libcurl on my box is 7.18.something, and it does not seem
to ask for password when the URL has only username but not colon-password.
I also expected it to ask for password when $HOME/.netrc has login but not
password for a given machine, but that does not seem to happen either.
Perhaps the version is too old.

4) I'm not really impressed that file permissions actually make the .netrc
file a secure option. However, it's already in there and would break
backwards compatibility to take it out. I also realize that there is a need
for automated builds to be able to pull the source. So I would like to add a nice 
warning section to the http docs explaining the repercussions of using it.

I agree with the first two sentences and am not happy with http.password
because of it.


---
 http.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index f4f0bf6..3d5caa6 100644
--- a/http.c
+++ b/http.c

@@ -25,6 +25,7 @@ static long curl_low_speed_limit = -1;
 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;
 
 static struct curl_slist *pragma_header;
 

@@ -135,6 +136,20 @@ static int http_options(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
+static void init_curl_http_auth(CURL *result)
+{
+	if (!user_name)
+		curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
+	else {
+		struct strbuf up = STRBUF_INIT;
+		if (!user_pass)
+			user_pass = xstrdup(getpass("Password: "));
+		strbuf_addf(&up, "%s:%s", user_name, user_pass);
+		curl_easy_setopt(result, CURLOPT_USERPWD,
+				 strbuf_detach(&up, NULL));
+	}
+}
+
 static CURL *get_curl_handle(void)
 {
 	CURL *result = curl_easy_init();

@@ -153,6 +168,8 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
 #endif
 
+	init_curl_http_auth(result);
+
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 #if LIBCURL_VERSION_NUM >= 0x070902

@@ -190,6 +207,46 @@ static CURL *get_curl_handle(void)
 	return result;
 }
 
+static void http_auth_init(const char *url)
+{
+	char *at, *colon, *cp, *slash;
+	int len;
+
+	cp = strstr(url, "://");
+	if (!cp)
+		return;
+
+	/*
+	 * Ok, the URL looks like "proto://something".  Which one?
+	 * "proto://<user>:<pass>@<host>/...",
+	 * "proto://<user>@<host>/...", or just
+	 * "proto://<host>/..."?
+	 */
+	cp += 3;
+	at = strchr(cp, '@');
+	colon = strchr(cp, ':');
+	slash = strchrnul(cp, '/');
+	if (!at || slash <= at)
+		return; /* No credentials */
+	if (!colon || at <= colon) {
+		/* Only username */
+		len = at - cp;
+		user_name = xmalloc(len + 1);
+		memcpy(user_name, cp, len);
+		user_name[len] = '\0';
+		user_pass = NULL;
+	} else {
+		len = colon - cp;
+		user_name = xmalloc(len + 1);
+		memcpy(user_name, cp, len);
+		user_name[len] = '\0';
+		len = at - (colon + 1);
+		user_pass = xmalloc(len + 1);
+		memcpy(user_pass, colon + 1, len);
+		user_pass[len] = '\0';
+	}
+}
+
 void http_init(struct remote *remote)
 {
 	char *low_speed_limit;

@@ -252,6 +309,9 @@ void http_init(struct remote *remote)
 	if (getenv("GIT_CURL_FTP_NO_EPSV"))
 		curl_ftp_no_epsv = 1;
 
+	if (remote && remote->url && remote->url[0])
+		http_auth_init(remote->url[0]);
+
 #ifndef NO_CURL_EASY_DUPHANDLE
 	curl_default = get_curl_handle();
 #endif

On Mon, 9 Mar 2009, Junio C Hamano wrote:

The version of libcurl on my box is 7.18.something, and it does not seem to 
ask for password when the URL has only username but not colon-password. I 
also expected it to ask for password when $HOME/.netrc has login but not 
password for a given machine, but that does not seem to happen either. 
Perhaps the version is too old.

No, that's entirely expected. libcurl has no "prompt the user if no password 
was given" logic but instead delegates that work to the application.

There was once functionality for this (removed in October 2003) but it was 
broken and violated internal guidelines so we cut out and threw that code 
away.

More recently there have been people interested in re-implementing this "the 
right way" but so far it hasn't been made and thus the application is left to 
perform this task.

-- 

  / daniel.haxx.se

2009/3/10 Junio C Hamano [off-list ref]:

Elsewhere we seem to protect use of CURL_NETRC_OPTIONAL by checking
for LIBCURL_VERSION_NUM >= 0x070907. I have an ancient curl here
(curl-7.9.3-2ssl) which doesn't seem to have this option, so building
next is broken on AIX for me from this morning (c33976cb).

Is there a specific minimum version of curl we want to continue supporting?

Mike

On Thu, 12 Mar 2009, Mike Ralphson wrote:

Elsewhere we seem to protect use of CURL_NETRC_OPTIONAL by checking for 
LIBCURL_VERSION_NUM >= 0x070907. I have an ancient curl here 
(curl-7.9.3-2ssl) which doesn't seem to have this option, so building next 
is broken on AIX for me from this morning (c33976cb).

Is there a specific minimum version of curl we want to continue supporting?

May I suggest perhaps require a libcurl version that is no older than three 
years or something like that?

Perhaps this list can serve as some help:

 	http://curl.haxx.se/docs/releases.html

(spoiler: libcurl 7.9.3 is more than seven years old!)

-- 

  / daniel.haxx.se

2009/3/12 Daniel Stenberg [off-list ref]:

On Thu, 12 Mar 2009, Mike Ralphson wrote:

quoted

Elsewhere we seem to protect use of CURL_NETRC_OPTIONAL by checking for
LIBCURL_VERSION_NUM >= 0x070907. I have an ancient curl here
(curl-7.9.3-2ssl) which doesn't seem to have this option, so building next
is broken on AIX for me from this morning (c33976cb).

Is there a specific minimum version of curl we want to continue
supporting?

May I suggest perhaps require a libcurl version that is no older than three
years or something like that?

It might be a plan 8-) Though I was thinking technically in terms of
features we think git needs. Though doubtless there are several
security fixes it would be beneficial to keep up to date with.

(spoiler: libcurl 7.9.3 is more than seven years old!)

And still the release IBM package for AIX [1]. 8-(

The summary of automatic builds (http://curl.haxx.se/auto/) is very
nicely presented. Is that custom code?

Thanks for curl, even the old versions!

Mike

[1] http://www-03.ibm.com/systems/power/software/aix/linux/toolbox/alpha.html

On Thu, 12 Mar 2009, Mike Ralphson wrote:

quoted

May I suggest perhaps require a libcurl version that is no older than three 
years or something like that?

It might be a plan 8-) Though I was thinking technically in terms of 
features we think git needs. Though doubtless there are several security 
fixes it would be beneficial to keep up to date with.

Right, but if you set a common lowest denominator first you know what features 
to expect to be there _at least_, then there might of course be a set of 
additional ones brought by newer versions. It would reduce the amount of 
conditionals in the code and what-if-this-is-used scenarios (in the code and 
in support/docs). It also reduces the risks of git getting odd problems due to 
very old libcurl bugs.

quoted

(spoiler: libcurl 7.9.3 is more than seven years old!)

And still the release IBM package for AIX [1]. 8-(

However, someone who's building/getting git might also be able to build/get a 
newer libcurl.

The summary of automatic builds (http://curl.haxx.se/auto/) is very nicely 
presented. Is that custom code?

The code is custom (perl) but present in the curl CVS repo for the web site 
and could probably fairly easy be adapted for other purposes/projects.

In the curl project we provide scripts for distributed automatic tests and 
then we have a central server that receives the reports by mail and the 
automatic summary script displays the status of those tests on that page.

-- 

  / daniel.haxx.se