All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] http authentication via prompts
@ 2009-03-05  1:07 Mike Gaffney
  2009-03-05  7:34 ` Junio C Hamano
  2009-03-05 10:55 ` Johannes Schindelin
  0 siblings, 2 replies; 6+ messages in thread
From: Mike Gaffney @ 2009-03-05  1:07 UTC (permalink / raw)
  To: git

Currently git over http only works with a .netrc file which required that you store your password on the file system in plaintext. This commit adds to configuration options for http for a username and an optional password. If a http.username is set, then the .netrc file is ignored and the username is used instead. If a http.password is set, then that is used as well, otherwise the user is prompted for their password.

With the old .netrc working, this patch provides backwards compatibility while adding a more secure option for users whose http password may be sensitive (such as if its a domain controller password) and do not wish to have it on the filesystem.

Signed-off-by: Mike Gaffney <mike@uberu.com>
---
 Documentation/config.txt                           |    7 +++
 Documentation/howto/setup-git-server-over-http.txt |   38 ++++++++++++++++--
 http.c                                             |   41 ++++++++++++++++++-
 http.h                                             |    2 +
 4 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f5152c5..821bf48 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -920,6 +920,13 @@ help.autocorrect::
 	value is 0 - the command will be just shown but not executed.
 	This is the default.
 
+http.username, http.password:
+    The username and password for http authentication. http.username is
+    required, http.password is optional. If supplied, the .netrc file will
+    be ignored. If a password is not supplied, git will prompt for it.
+    Be careful when configuring a password as it will be stored in plain text
+    on the filesystem.
+
 http.proxy::
 	Override the HTTP proxy, normally configured using the 'http_proxy'
 	environment variable (see linkgit:curl[1]).  This can be overridden
diff --git a/Documentation/howto/setup-git-server-over-http.txt b/Documentation/howto/setup-git-server-over-http.txt
index 622ee5c..462a9d4 100644
--- a/Documentation/howto/setup-git-server-over-http.txt
+++ b/Documentation/howto/setup-git-server-over-http.txt
@@ -189,8 +189,19 @@ Make sure that you have HTTP support, i.e. your git was built with
 libcurl (version more recent than 7.10). The command 'git http-push' with
 no argument should display a usage message.
 
-Then, add the following to your $HOME/.netrc (you can do without, but will be
-asked to input your password a _lot_ of times):
+There are 2 ways to authenticate with git http, netrc and via the git config.
+The netrc option requires that you put the username and password for the connection
+in $HOME/.netrc. The configuration method allows you to specify a username and
+optionally a password. If the password is not supplied then git will prompt you
+for the password. The downside to the netrc method is that you must have your
+username and password in plaintext on the filesystem, albeit in a protected file.
+If the username/password combo is a sensitive one, you may wish to use the
+git config method. The downside of the config method is that you will be prompted
+for your password every time you push or pull to the remote repository.
+
+Using netrc:
+
+Using your favourite ext editor, add the following to your $HOME/.netrc:
 
     machine <servername>
     login <username>
@@ -204,7 +215,7 @@ instead of the server name.
 
 To check whether all is OK, do:
 
-   curl --netrc --location -v http://<username>@<servername>/my-new-repo.git/HEAD
+   curl --netrc --location -v http://<servername>/my-new-repo.git/HEAD
 
 ...this should give something like 'ref: refs/heads/master', which is
 the content of the file HEAD on the server.
@@ -213,12 +224,31 @@ Now, add the remote in your existing repository which contains the project
 you want to export:
 
    $ git-config remote.upload.url \
-       http://<username>@<servername>/my-new-repo.git/
+       http://<servername>/my-new-repo.git/
 
 It is important to put the last '/'; Without it, the server will send
 a redirect which git-http-push does not (yet) understand, and git-http-push
 will repeat the request infinitely.
 
+Using git config:
+
+curl --user <username>:<password> --location -v http://<servername>/my-new-repo.git/HEAD
+
+...this should give something like 'ref: refs/heads/master', which is
+the content of the file HEAD on the server.
+
+Now, add the remote in your existing repository which contains the project
+you want to export:
+
+   $ git-config remote.upload.url \
+       http://<servername>/my-new-repo.git/
+
+Also, add in your username with:
+   $ git-config http.username <username>
+
+And optionally your password (you will be prompted for it if you do not):
+   $ git-config http.password <password>
+
 
 Step 4: make the initial push
 -----------------------------
diff --git a/http.c b/http.c
index ee58799..348b9fb 100644
--- a/http.c
+++ b/http.c
@@ -26,6 +26,9 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv = 0;
 static const char *curl_http_proxy = NULL;
 
+static const char *curl_http_username = NULL;
+static const char *curl_http_password = NULL;
+
 static struct curl_slist *pragma_header;
 
 static struct active_request_slot *active_queue_head = NULL;
@@ -153,11 +156,45 @@ static int http_options(const char *var, const char *value, void *cb)
 			return git_config_string(&curl_http_proxy, var, value);
 		return 0;
 	}
+	if (!strcmp("http.username", var)) {
+		if (curl_http_username == NULL)
+		{
+			return git_config_string(&curl_http_username, var, value);
+		}
+		return 0;
+	}
+	if (!strcmp("http.password", var)) {
+		if (curl_http_password == NULL)
+		{
+			return git_config_string(&curl_http_password, var, value);
+		}
+		return 0;
+	}
 
 	/* Fall back on the default ones */
 	return git_default_config(var, value, cb);
 }
 
+static void init_curl_http_auth(CURL* result){
+#if LIBCURL_VERSION_NUM >= 0x070907
+        struct strbuf userpass;
+        strbuf_init(&userpass, 0);
+        if (curl_http_username != NULL) {
+                strbuf_addstr(&userpass, curl_http_username);
+		strbuf_addstr(&userpass, ":");
+		if (curl_http_password != NULL) {
+			strbuf_addstr(&userpass, curl_http_password);
+		} else {
+			strbuf_addstr(&userpass, getpass("Password: "));
+		}
+		curl_easy_setopt(result, CURLOPT_USERPWD, userpass.buf);
+		curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_IGNORED);
+        } else {
+		curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
+        }
+#endif
+}
+
 static CURL* get_curl_handle(void)
 {
 	CURL* result = curl_easy_init();
@@ -172,9 +209,7 @@ static CURL* get_curl_handle(void)
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
 	}
 
-#if LIBCURL_VERSION_NUM >= 0x070907
-	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);
diff --git a/http.h b/http.h
index 905b462..71320d1 100644
--- a/http.h
+++ b/http.h
@@ -5,6 +5,8 @@
 
 #include <curl/curl.h>
 #include <curl/easy.h>
+#include <termios.h>
+#include <stdio.h>
 
 #include "strbuf.h"
 #include "remote.h"
-- 
1.6.1.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] http authentication via prompts
  2009-03-05  1:07 [PATCH] http authentication via prompts Mike Gaffney
@ 2009-03-05  7:34 ` Junio C Hamano
  2009-03-05 10:55 ` Johannes Schindelin
  1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2009-03-05  7:34 UTC (permalink / raw)
  To: Mike Gaffney; +Cc: git

Mike Gaffney <mr.gaffo@gmail.com> writes:

> Currently git over http only works with a .netrc file which required that you store your password on the file system in plaintext. This commit adds to configuration options for http for a username and an optional password. If a http.username is set, then the .netrc file is ignored and the username is used instead. If a http.password is set, then that is used as well, otherwise the user is prompted for their password.
>
> With the old .netrc working, this patch provides backwards compatibility while adding a more secure option for users whose http password may be sensitive (such as if its a domain controller password) and do not wish to have it on the filesystem.

Please wrap lines to readable length, such as under 72 cols.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index f5152c5..821bf48 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -920,6 +920,13 @@ help.autocorrect::
>  	value is 0 - the command will be just shown but not executed.
>  	This is the default.
>  
> +http.username, http.password:
> +    The username and password for http authentication. http.username is
> +    required, http.password is optional. If supplied, the .netrc file will
> +    be ignored. If a password is not supplied, git will prompt for it.
> +    Be careful when configuring a password as it will be stored in plain text
> +    on the filesystem.
> +
>  http.proxy::

List item ends with double colons; two headline items that are described
with the same description are listed each on its own line.  I.e.

	http.username::
        http.password::
        	The username and ...

> diff --git a/Documentation/howto/setup-git-server-over-http.txt b/Documentation/howto/setup-git-server-over-http.txt
> index 622ee5c..462a9d4 100644
> --- a/Documentation/howto/setup-git-server-over-http.txt
> +++ b/Documentation/howto/setup-git-server-over-http.txt
> @@ -189,8 +189,19 @@ Make sure that you have HTTP support, i.e. your git was built with
>  libcurl (version more recent than 7.10). The command 'git http-push' with
>  no argument should display a usage message.
>  
> -Then, add the following to your $HOME/.netrc (you can do without, but will be
> -asked to input your password a _lot_ of times):
> +There are 2 ways to authenticate with git http, netrc and via the git config.
> +The netrc option requires that you put the username and password for the connection
> +in $HOME/.netrc. The configuration method allows you to specify a username and
> +optionally a password. If the password is not supplied then git will prompt you
> +for the password. The downside to the netrc method is that you must have your
> +username and password in plaintext on the filesystem, albeit in a protected file.
> +If the username/password combo is a sensitive one, you may wish to use the
> +git config method. The downside of the config method is that you will be prompted
> +for your password every time you push or pull to the remote repository.

The contents look readable, but each line is a bit too long.

> @@ -204,7 +215,7 @@ instead of the server name.
>  
>  To check whether all is OK, do:
>  
> -   curl --netrc --location -v http://<username>@<servername>/my-new-repo.git/HEAD
> +   curl --netrc --location -v http://<servername>/my-new-repo.git/HEAD

Why?

> @@ -213,12 +224,31 @@ Now, add the remote in your existing repository which contains the project
>  you want to export:
>  
>     $ git-config remote.upload.url \
> -       http://<username>@<servername>/my-new-repo.git/
> +       http://<servername>/my-new-repo.git/

Why?

> +Using git config:

The bulk of text before this does not have a subtitle like this.  Perhaps
you would want to add one?  The presense of this subtitle makes it clear
that you are going to talk about _another_ way, but for first time readers
it is unclear how that other way is different and what its advantages are.

Perhaps drop this subtitle, and instead give one short paragraph, e.g.

    Instead of storing your password in plaintext $HOME/.netrc file, you
    can store only the username in the configuration file and have the
    program prompt for a password.  Here is how.

Alternatively keep the subtitle and explain what the subsection is about
in a similar way.

But I think this section raises a bigger usability and user perception
issue.

When "http://user@host/rest" URL is given to your "git push/fetch",
without .netrc nor http.username configuration, we allow the curl library
to prompt for a password, and because we do not reuse the password (and
reinstantiate the curl handle), we end up asking the user million times.

But from the end user's point of view, that's all implementation detail.
It does not explain why "http://user@host/rest" acts in a silly way, and
"http://host/rest" with http.username configuration doesn't.

Perhaps you instead inspect the URL and see if you have the "user" part
(without password), and then:

 (1) transform the URL to http://host/rest" before giving it to libcurl;
     and

 (2) use your CURLOPT_USERPWD logic with your own getpass() call in
     init_curl_http_auth()?

That way you do not have (even though you could) to introduce a new
configuration, nor have a new section in the documentation.  It will be a
straightforward fix of the "will be asked ... a lot of times" bug you
removed from the documentation, no?

> diff --git a/http.c b/http.c
> index ee58799..348b9fb 100644
> --- a/http.c
> +++ b/http.c
> @@ -26,6 +26,9 @@ static long curl_low_speed_time = -1;
>  static int curl_ftp_no_epsv = 0;
>  static const char *curl_http_proxy = NULL;
>  
> +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.

> +static void init_curl_http_auth(CURL* result){
> +#if LIBCURL_VERSION_NUM >= 0x070907
> +        struct strbuf userpass;
> +        strbuf_init(&userpass, 0);
> +        if (curl_http_username != NULL) {
> +                strbuf_addstr(&userpass, curl_http_username);
> +		strbuf_addstr(&userpass, ":");
> +		if (curl_http_password != NULL) {
> +			strbuf_addstr(&userpass, curl_http_password);
> +		} else {
> +			strbuf_addstr(&userpass, getpass("Password: "));
> +		}
> +		curl_easy_setopt(result, CURLOPT_USERPWD, userpass.buf);
> +		curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_IGNORED);

Why NETRC_IGNORED?

On CURLOPT_NETRC, http://curl.haxx.se/libcurl/c/curl_easy_setopt.html says 

	libcurl uses a user name (and supplied or prompted password)
	supplied with CURLOPT_USERPWD in preference to any of the options
	controlled by this parameter.

Does it not work as advertised?

In short, I think what you did in init_curl_http_auth() makes a lot of
sense except for the NETRC_IGNORED bit, but:

 (1) I think http.password configuration has the exact same "plaintext
     password in a read-protected file" issue as .netrc; it is
     unnecessary.

 (2) http.username is used by your patch primarily as a way to trigger the
     new logic to call getpass() and use CURLOPT_USERPWD.  It would be a
     lot nicer to inspect the URL to notice that there is a username part
     in it and triggering the same codepath.  That would be a genuine
     improvement (and you can even claim it is a bugfix).  And if that
     works, the new configuration variable does not add much value (except
     that different people involved in the same project can use the same
     URL but their own (username, password) pair to access it.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] http authentication via prompts
  2009-03-05  1:07 [PATCH] http authentication via prompts Mike Gaffney
  2009-03-05  7:34 ` Junio C Hamano
@ 2009-03-05 10:55 ` Johannes Schindelin
  2009-03-05 15:15   ` Mike Gaffney
  1 sibling, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2009-03-05 10:55 UTC (permalink / raw)
  To: Mike Gaffney; +Cc: git

Hi,

Disclaimer: if you are offended by constructive criticism, or likely to 
answer with insults to the comments I offer, please stop reading this mail 
now (and please do not answer my mail, either). :-)

Still with me?  Good.  Nice to meet you.

Just for the record: responding to a patch is my strongest way of saying 
that I appreciate your work.

On Wed, 4 Mar 2009, Mike Gaffney wrote:

> Currently git over http only works with a .netrc file which required 
> that you store your password on the file system in plaintext. This 
> commit adds to configuration options for http for a username and an 
> optional password. If a http.username is set, then the .netrc file is 
> ignored and the username is used instead. If a http.password is set, 
> then that is used as well, otherwise the user is prompted for their 
> password.

>From the subject, I would have expected a way to type in the password 
instead of storing it.  (Think getpass()... which would pose problems 
with Windows support, of course.)

FWIW by having it in .git/config (which is most likely more world-readable 
than $HOME/.netrc ever will be) does not provide any security over .netrc.

And I doubt that http.username is a good choice: what if you have multiple 
http:// URLs with different usernames/passwords?  So would it not make 
more sense to make this remote.<name>.user and ...password?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] http authentication via prompts
  2009-03-05 10:55 ` Johannes Schindelin
@ 2009-03-05 15:15   ` Mike Gaffney
  2009-03-05 16:37     ` Jeff King
  2009-03-06 22:52     ` Fredrik Skolmli
  0 siblings, 2 replies; 6+ messages in thread
From: Mike Gaffney @ 2009-03-05 15:15 UTC (permalink / raw)
  To: git

My thought was that if you had a password you didn't care about you could put it in the config.
It does ask you for a password with getpass, It compiles under cygwin, I havent tried it under
windows. However the man page for getpass shows the source so coding up getpass directly isn't a
big deal.

Junio, I'm new to this patch game and using Thunderbird. What's the best way
to wrap the patch?

-Mike

Johannes Schindelin wrote:
> Hi,
> 
> Disclaimer: if you are offended by constructive criticism, or likely to 
> answer with insults to the comments I offer, please stop reading this mail 
> now (and please do not answer my mail, either). :-)
> 
> Still with me?  Good.  Nice to meet you.
> 
> Just for the record: responding to a patch is my strongest way of saying 
> that I appreciate your work.
> 
> On Wed, 4 Mar 2009, Mike Gaffney wrote:
> 
>> Currently git over http only works with a .netrc file which required 
>> that you store your password on the file system in plaintext. This 
>> commit adds to configuration options for http for a username and an 
>> optional password. If a http.username is set, then the .netrc file is 
>> ignored and the username is used instead. If a http.password is set, 
>> then that is used as well, otherwise the user is prompted for their 
>> password.
> 
> From the subject, I would have expected a way to type in the password 
> instead of storing it.  (Think getpass()... which would pose problems 
> with Windows support, of course.)
> 
> FWIW by having it in .git/config (which is most likely more world-readable 
> than $HOME/.netrc ever will be) does not provide any security over .netrc.
> 
> And I doubt that http.username is a good choice: what if you have multiple 
> http:// URLs with different usernames/passwords?  So would it not make 
> more sense to make this remote.<name>.user and ...password?
> 
> Ciao,
> Dscho

-- 
-Mike Gaffney (http://rdocul.us)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] http authentication via prompts
  2009-03-05 15:15   ` Mike Gaffney
@ 2009-03-05 16:37     ` Jeff King
  2009-03-06 22:52     ` Fredrik Skolmli
  1 sibling, 0 replies; 6+ messages in thread
From: Jeff King @ 2009-03-05 16:37 UTC (permalink / raw)
  To: Mike Gaffney; +Cc: git

On Thu, Mar 05, 2009 at 09:15:29AM -0600, Mike Gaffney wrote:

> My thought was that if you had a password you didn't care about you
> could put it in the config.  It does ask you for a password with
> getpass, It compiles under cygwin, I havent tried it under windows.

I think part of the confusion is that your patch does two things, which
means it is probably more sensible as two separate patches:

  1. support http.username and http.password in the git config instead
     of the .netrc

  2. support getpass() if http.password (or .netrc password) is not
     available

-Peff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] http authentication via prompts
  2009-03-05 15:15   ` Mike Gaffney
  2009-03-05 16:37     ` Jeff King
@ 2009-03-06 22:52     ` Fredrik Skolmli
  1 sibling, 0 replies; 6+ messages in thread
From: Fredrik Skolmli @ 2009-03-06 22:52 UTC (permalink / raw)
  To: Mike Gaffney; +Cc: git

On Thu, Mar 05, 2009 at 09:15:29AM -0600, Mike Gaffney wrote:

Hi.
 
> Junio, I'm new to this patch game and using Thunderbird. What's the best way
> to wrap the patch?

I'm not Junio, but I'll give answering a shot anyway. ;-)

See Documentation/SubmittingPatches, line 374-456.

-- 
Fredrik Skolmli

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-03-06 22:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-05  1:07 [PATCH] http authentication via prompts Mike Gaffney
2009-03-05  7:34 ` Junio C Hamano
2009-03-05 10:55 ` Johannes Schindelin
2009-03-05 15:15   ` Mike Gaffney
2009-03-05 16:37     ` Jeff King
2009-03-06 22:52     ` Fredrik Skolmli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.