Re: [PATCH] git-send-email: add ~/.authinfo parsing

Michal Nazarewicz [off-list ref] writes:

From: Michal Nazarewicz <redacted>

Make git-send-email read password from a ~/.authinfo file instead of
requiring it to be stored in git configuration, passed as command line
argument or typed in.

Makes one wonder why .authinfo and not .netrc; 

http://www.gnu.org/software/emacs/manual/html_node/auth/Help-for-users.html

phrases it amusingly:

        “Netrc” files are usually called .authinfo or .netr
        nowadays .authinfo seems to be more popular and the
        auth-source library encourages this confusion by accepting
        both

Either way it still encourages a plaintext password to be on disk,
which may not be what we want, even though it may be slight if not
really much of an improvement.  Again the Help-for-users has this
amusing bit:

	You could just say (but we don't recommend it, we're just
	showing that it's possible)

	     password mypassword

	to use the same password everywhere. Again, DO NOT DO THIS
	or you will be pwned as the kids say.

+The '~/.authinfo' file is read if Text::CSV Perl module is installed
+on the system; if it's missing, a notification message will be printed
+and the file ignored altogether.  The file should contain a line with
+the following format:
++
+  machine <domain> port <port> login <user> password <pass>

It is rather strange to require a comma-separated-values parser to
read a file format this simple, isn't it?

++
+Contrary to other tools, 'git-send-email' does not support symbolic
+port names like 'imap' thus `<port>` must be a number.

Perhaps you can convert at least some popular ones yourself?  After
all, the user may be using an _existing_ .authinfo/.netrc that she
has been using with other programs that do understand symbolic port
names.  Rather than forcing all such users to update their files,
the patch can work a bit harder for them and the world will be a
better place, no?

From: Michal Nazarewicz <redacted>

Make git-send-email read password from a ~/.authinfo or a ~/.netrc
file instead of requiring it to be stored in git configuration, passed
as command line argument or typed in.

There are various other applications that use this file for
authentication information so letting users use it for git-send-email
is convinient.  Furthermore, some users store their ~/.gitconfig file
in a public repository and having to store password there makes it
easy to publish the password.

Signed-off-by: Michal Nazarewicz <redacted>
---
 Documentation/git-send-email.txt |  34 +++++++++--
 git-send-email.perl              | 124 +++++++++++++++++++++++++++++++++++----
 2 files changed, 140 insertions(+), 18 deletions(-)

On Tue, Jan 29 2013, Junio C Hamano wrote:

Makes one wonder why .authinfo and not .netrc; 

Fine… Let's parse both. ;)

Either way it still encourages a plaintext password to be on disk,
which may not be what we want, even though it may be slight if not
really much of an improvement.

Well… Users store passwords on disks in a lot of places.  I wager that
most have mail clients configured not to ask for password but instead
store it on hard drive.  I don't see that changing any time soon, so
at least we can try and minimise number of places where a password is
stored.

It is rather strange to require a comma-separated-values parser to
read a file format this simple, isn't it?

I was worried about spaces in password.  CVS should handle such case
nicely, whereas simple split won't.  Nonetheless, I guess that in the
end this is not likely enough to add the dependency.

Perhaps you can convert at least some popular ones yourself?  After
all, the user may be using an _existing_ .authinfo/.netrc that she
has been using with other programs that do understand symbolic port
names.  Rather than forcing all such users to update their files,
the patch can work a bit harder for them and the world will be a
better place, no?

Parsing /etc/services added.

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index eeb561c..ee20714 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt

@@ -158,14 +158,36 @@ Sending
 --smtp-pass[=<password>]::
 	Password for SMTP-AUTH. The argument is optional: If no
 	argument is specified, then the empty string is used as
-	the password. Default is the value of 'sendemail.smtppass',
-	however '--smtp-pass' always overrides this value.
+	the password. Default is the value of 'sendemail.smtppass'
+	or value read from ~/.authinfo file, however '--smtp-pass'
+	always overrides this value.
 +
-Furthermore, passwords need not be specified in configuration files
-or on the command line. If a username has been specified (with
+Furthermore, passwords need not be specified in configuration files or
+on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass', 'sendemail.smtppass' or via
+~/.authinfo file), then the user is prompted for a password while
+the input is masked for privacy.
++
+The ~/.authinfo file should contain a line with the following
+format:
++
+  machine <domain> port <port> login <user> password <pass>
++
+Each pair (expect for `password <pass>`) can be omitted which will
+skip matching of the given value.  Lines are interpreted in order and
+password from the first line that matches will be used.  `<port>` can
+be either an integer or a symbolic name.  In the latter case, it is
+looked up in `/etc/services` file (if it exists).  For instance, you
+can put
++
+  machine example.com login testuser port ssmtp password smtppassword
+  machine example.com login testuser            password testpassword
++
+if you want to use `smtppassword` for authenticating to a service at
+port 465 (SSMTP) and `testpassword` for all other services.  As shown
+in the example, `<port>` can use   If ~/.authinfo file is
+missing, 'git-send-email' will also try ~/.netrc file.
 
 --smtp-server=<host>::
 	If set, specifies the outgoing SMTP server to use (e.g.

diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..2d8fd1b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl

@@ -1045,6 +1045,117 @@ sub maildomain {
 	return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+
+sub read_password_from_stdin {
+	my $line;
+
+	system "stty -echo";
+
+	do {
+		print "Password: ";
+		$line = <STDIN>;
+		print "\n";
+	} while (!defined $line);
+
+	system "stty echo";
+
+	chomp $line;
+	return $line;
+}
+
+sub read_etc_services {
+	my $fd;
+	if (!open $fd, '<', '/etc/services') {
+		return {};
+	}
+
+	my $ret = {};
+	while (my $line = <$fd>) {
+		$line =~ s/^\s+|\s*(?:#.*)?$//g;
+		my @line = split /\s+/, $line;
+		if (@line < 2 || $line[1] !~ m~^(\d+)/tcp$~) {
+			next;
+		}
+
+		my $num = int $1;
+		undef $line[1];
+		for my $service (@line) {
+			if (defined $service && !defined $ret->{$service}) {
+				$ret->{$service} = $num;
+			}
+		}
+	}
+
+	close $fd;
+	return $ret;
+}
+
+my $authinfo_parse_port;
+
+sub authinfo_is_eq_port {
+	my ($from_file, $value, $filename) = @_;
+
+	if (!defined $from_file) {
+		return 1;
+	} elsif ($from_file =~ /^\d+$/) {
+		return $from_file == $value;
+	}
+
+	if (!defined $authinfo_parse_port) {
+		$authinfo_parse_port = read_etc_services;
+	}
+
+	my $port = $authinfo_parse_port->{$from_file};
+	if (!defined $port) {
+		print STDERR "$filename: invalid port name: $from_file\n";
+		return;
+	}
+
+	return $port == $value;
+}
+
+sub authinfo_is_eq {
+	my ($from_file, $value) = @_;
+	return defined $from_file || $from_file eq $value;
+}
+
+sub read_password_from_authinfo {
+	my $filename = join '/', $ENV{'HOME'}, $_[0] // '.authinfo';
+	my $fd;
+	if (!open $fd, '<', $filename) {
+		return;
+	}
+
+	my $password;
+	while (my $line = <$fd>) {
+		$line =~ s/^\s+|\s+$//g;
+		my @line = split /\s+/, $line;
+		if (@line % 2) {
+			next;
+		}
+
+		my %line = @line;
+		if (defined $line{'password'} &&
+		    authinfo_is_eq $line{'machine'}, $smtp_server &&
+		    authinfo_is_eq $line{'login'}, $smtp_authuser &&
+		    authinfo_is_eq_port $line{'port'}, $smtp_server_port, $filename) {
+			$password = $line{'password'};
+			last;
+		}
+	}
+
+	close $fd;
+	return $password;
+}
+
+sub read_password {
+	return
+	  read_password_from_authinfo '.authinfo' ||
+	  read_password_from_authinfo '.netrc' ||
+	  read_password_from_stdin;
+}
+
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.

@@ -1194,18 +1305,7 @@ X-Mailer: git-send-email $gitversion
 			};
 
 			if (!defined $smtp_authpass) {
-
-				system "stty -echo";
-
-				do {
-					print "Password: ";
-					$_ = <STDIN>;
-					print "\n";
-				} while (!defined $_);
-
-				chomp($smtp_authpass = $_);
-
-				system "stty echo";
+				$smtp_authpass = read_password
 			}
 
 			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;

-- 
1.8.1

On Tue, Jan 29, 2013 at 11:53:19AM -0800, Junio C Hamano wrote:

Either way it still encourages a plaintext password to be on disk,
which may not be what we want, even though it may be slight if not
really much of an improvement.  Again the Help-for-users has this
amusing bit:

I do not mind a .netrc or .authinfo parser, because while those formats
do have security problems, they are standard files that may already be
in use. So as long as we are not encouraging their use, I do not see a
problem in supporting them (and we already do the same with curl's netrc
support).

But it would probably make sense for send-email to support the existing
git-credential subsystem, so that it can take advantage of secure
system-specific storage. And that is where we should be pointing new
users. I think contrib/mw-to-git even has credential support written in
perl, so it would just need to be factored out to Git.pm.

-Peff

On Tue, 29 Jan 2013 11:53:19 -0800 Junio C Hamano [off-list ref] wrote: 

JCH> Makes one wonder why .authinfo and not .netrc; 

JCH> http://www.gnu.org/software/emacs/manual/html_node/auth/Help-for-users.html

JCH> phrases it amusingly:

JCH>         “Netrc” files are usually called .authinfo or .netr
JCH>         nowadays .authinfo seems to be more popular and the
JCH>         auth-source library encourages this confusion by accepting
JCH>         both

I wrote this and the auth-source.el library in Emacs (I'm glad it was
amusing :).  The confusion is further perpetuated by our (in Emacs)
encouragement to use a .authinfo.gpg file, which is then decrypted on
the fly by Emacs through GPG.  The format is the same; by the time
auth-source.el sees the contents, they are plain text since the decoding
happens at the file handler level.

I think it makes sense to write the code to support both
`git-send-email' and credentials.  I have had it in my TODO list for
almost 2 years now to work on credential support, and to support the
~/.authinfo.gpg decoding specifically.  Ideally this would also support
the other formats... Michal, would you be interested in that feature?  I
promise to get off my rear and help out.

quoted

+The '~/.authinfo' file is read if Text::CSV Perl module is installed
+on the system; if it's missing, a notification message will be printed
+and the file ignored altogether.  The file should contain a line with
+the following format:
++
+  machine <domain> port <port> login <user> password <pass>

JCH> It is rather strange to require a comma-separated-values parser to
JCH> read a file format this simple, isn't it?

I'd recommend a hand-crafted parser.  Among other things, you should
accept both "strings" and 'strings' if possible (I've seen both formats
in the wild), and the format is simple enough to avoid the module
dependency.

quoted

++
+Contrary to other tools, 'git-send-email' does not support symbolic
+port names like 'imap' thus `<port>` must be a number.

JCH> Perhaps you can convert at least some popular ones yourself?  After
JCH> all, the user may be using an _existing_ .authinfo/.netrc that she
JCH> has been using with other programs that do understand symbolic port
JCH> names.  Rather than forcing all such users to update their files,
JCH> the patch can work a bit harder for them and the world will be a
JCH> better place, no?

I agree, "port imap" is a nice self-documenting token.  Maybe it can be
interpreted by the program that requests the token with a services
lookup, where supported.

Ted

On Wed, Jan 30 2013, Jeff King wrote:

I do not mind a .netrc or .authinfo parser, because while those formats
do have security problems, they are standard files that may already be
in use. So as long as we are not encouraging their use, I do not see a
problem in supporting them (and we already do the same with curl's netrc
support).

But it would probably make sense for send-email to support the existing
git-credential subsystem, so that it can take advantage of secure
system-specific storage. And that is where we should be pointing new
users. I think contrib/mw-to-git even has credential support written in
perl, so it would just need to be factored out to Git.pm.

As far as I understand, there could be a git-credential helper that
reads ~/.authinfo and than git-send-email would just call “git
credential fill”, right?

I've noticed though, that git-credential does not support port argument,
which makes it slightly incompatible with ~/.authinfo.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--