[PATCH] git-send-email: ssh/login style password requests

[PATCH] git-send-email: ssh/login style password requests

[Michael Witten]

Whilst convenient, it is most unwise to record passwords
in any place but one's brain. Moreover, it is especially
foolish to store them in configuration files, even with
access permissions set accordingly.

git-send-email has been amended, so that if it detects
an smtp username without a password, it promptly prompts
for the password and masks the input for privacy.

Furthermore, the argument to --smtp-pass has been rendered
optional.

The documentation has been updated to reflect these changes.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---

	This was sent with the updated git-send-email ;-)

 Documentation/git-send-email.txt |   19 +++++++++++++++----
 git-send-email.perl              |   23 +++++++++++++++++++----
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0554f2b..6d5abf4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -96,12 +96,23 @@ The --cc option must be repeated for each user you want on the cc list.
 	servers typically listen to smtp port 25 and ssmtp port
 	465).
 
---smtp-user, --smtp-pass::
-	Username and password for SMTP-AUTH. Defaults are the values of
-	the configuration values 'sendemail.smtpuser' and
-	'sendemail.smtppass', but see also 'sendemail.identity'.
+--smtp-user::
+	Username for SMTP-AUTH. The default value can be specified
+	with the configuration variable 'sendemail.smtpuser' or
+	sendemail.<identity>.smtpuser (see sendemail.identity).
 	If not set, authentication is not attempted.
 
+--smtp-pass::
+	Password for SMTP-AUTH. The default value can be specified
+	with the configuration variable 'sendemail.smtppass' or
+	sendemail.<identity>.smtppass (see sendemail.identity).
+	The argument is optional: If no argument is specified, then
+	the password is assumed to be the empty string.
+	If a username has been set, but no password has been set,
+	the user is prompted for a password with masked input for
+	privacy; passwords need not be recorded in configuration
+	files.
+
 --smtp-ssl::
 	If set, connects to the SMTP server using SSL.
 	Default is the value of the 'sendemail.smtpssl' configuration value;
diff --git a/git-send-email.perl b/git-send-email.perl
index a1a9d14..ed0a473 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -157,7 +157,7 @@ my $compose_filename = ".msg.$$";
 
 # Variables we fill in automatically, or via prompting:
 my (@to,@cc,@initial_cc,@bcclist,@xh,
-	$initial_reply_to,$initial_subject,@files,$author,$sender,$compose,$time);
+	$initial_reply_to,$initial_subject,@files,$author,$sender,$smtp_authpass,$compose,$time);
 
 my $envelope_sender;
 
@@ -177,7 +177,7 @@ my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
-my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);
+my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_ssl);
 my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 my ($no_validate);
 
@@ -214,7 +214,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-port=s" => \$smtp_server_port,
 		    "smtp-user=s" => \$smtp_authuser,
-		    "smtp-pass=s" => \$smtp_authpass,
+		    "smtp-pass:s" => \$smtp_authpass,
 		    "smtp-ssl!" => \$smtp_ssl,
 		    "identity=s" => \$identity,
 		    "compose" => \$compose,
@@ -647,9 +647,24 @@ X-Mailer: git-send-email $gitversion
 			die "Unable to initialize SMTP properly.  Is there something wrong with your config?";
 		}
 
-		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
+		if (defined $smtp_authuser) {
+
+			if (!defined $smtp_authpass) {
+				
+				system "stty -echo";
+
+				do {
+					$_ = $term->readline("Password: ");
+				} while (!defined $_);
+
+				system "stty echo";
+
+				$smtp_authpass = $_ if ($_);
+			}
+
 			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
 		}
+
 		$smtp->mail( $raw_from ) or die $smtp->message;
 		$smtp->to( @recipients ) or die $smtp->message;
 		$smtp->data or die $smtp->message;
-- 
1.5.4.rc5.17.g536ee

Re: [PATCH] git-send-email: ssh/login style password requests

[Junio C Hamano]

Michael Witten <mfwitten@MIT.EDU> writes:

> +--smtp-pass::
> +	Password for SMTP-AUTH. The default value can be specified
> +	with the configuration variable 'sendemail.smtppass' or
> +	sendemail.<identity>.smtppass (see sendemail.identity).
> +	The argument is optional: If no argument is specified, then
> +	the password is assumed to be the empty string.
> +	If a username has been set, but no password has been set,
> +	the user is prompted for a password with masked input for
> +	privacy; passwords need not be recorded in configuration
> +	files.

I am a bit puzzled about the above description, though.  It is
not clear if there is any difference between "the empty string"
password, and "no password has been set".  It makes me wonder
what the possible cases are.

 (1) no *.smtppass configuration present; no --smtp-pass parameter
     is given on the command line.

 (2) *.smtppass configuration present but is an empty string; no
     --smtp-pass parameter is given on the command line.

 (3) *.smtppass configuration present with a non-empty string; no
     --smtp-pass parameter is given on the command line.

 (4) parameter is given with an empty string (i.e. "--smtp-pass=")
     on the command line.

 (5) parameter is given with a non-empty string (i.e. "--smtp-pass=foo")
     on the command line.

My _guess_ is that command line always trump configuration so
in cases (4) and (5) it does not matter what you have (or do not
have) in *.smtppass configuration.  In other cases, lack of *.smtppass
and having *.smtppass with an empty string as its value are equivalent.
And in any case, an empty string (or lack of specification) results
in prompting.

But you shouldn't make _me_ guess.  You certainly would be
forcing many other people to guess with this.

> +		if (defined $smtp_authuser) {
> +
> +			if (!defined $smtp_authpass) {
> +				
> +				system "stty -echo";
> +
> +				do {
> +					$_ = $term->readline("Password: ");
> +				} while (!defined $_);
> +
> +				system "stty echo";
> +
> +				$smtp_authpass = $_ if ($_);
> +			}
> +

I like what the patch tries to do, but the system() there feels
a tad ugly.  In addition it makes me wonder (1) what happens if
you ^C out from this while loop, and (2) $term->readline()
interface might already have a method to turn echo off.

We luckily have people handy at Perl on the list, so I'll wait
for our resident Perl experts to suggest a better alterantive.

We are not in a hurry, so I expect a polished resubmit after
1.5.4.  Thanks.

Re: [PATCH] git-send-email: ssh/login style password requests

[Michael Witten]

On 1 Feb 2008, at 5:09 AM, Junio C Hamano wrote:

> Michael Witten <mfwitten@MIT.EDU> writes:
>
>> +--smtp-pass::
>> +	Password for SMTP-AUTH. The default value can be specified
>> +	with the configuration variable 'sendemail.smtppass' or
>> +	sendemail.<identity>.smtppass (see sendemail.identity).
>> +	The argument is optional: If no argument is specified, then
>> +	the password is assumed to be the empty string.
>> +	If a username has been set, but no password has been set,
>> +	the user is prompted for a password with masked input for
>> +	privacy; passwords need not be recorded in configuration
>> +	files.
>
> I am a bit puzzled about the above description, though.  It is
> not clear if there is any difference between "the empty string"
> password, and "no password has been set".  It makes me wonder
> what the possible cases are.

How are these descriptions:

	http://web.mit.edu/mfwitten/git-send-email.html

I've attempted to keep them short, but complete. Also, the
resulting man page versions are formatted similarly (though
at the expense of some ASCIIdoc readability).

> In other cases, lack of *.smtppass
> and having *.smtppass with an empty string as its value are  
> equivalent.
> And in any case, an empty string (or lack of specification) results
> in prompting.

Actually, a lack of *.smtp<user|pass> results in variables without
definition rather than with empty-string values; the original code,
too, makes use of this fact. Consequently, an empty string for
*.smtppass does not result in prompting, as tested.

>> +		if (defined $smtp_authuser) {
>> +
>> +			if (!defined $smtp_authpass) {
>> +				
>> +				system "stty -echo";
>> +
>> +				do {
>> +					$_ = $term->readline("Password: ");
>> +				} while (!defined $_);
>> +
>> +				system "stty echo";
>> +
>> +				$smtp_authpass = $_ if ($_);
>> +			}
>> +
>
> I like what the patch tries to do, but the system() there feels
> a tad ugly.  In addition it makes me wonder (1) what happens if
> you ^C out from this while loop

Good question! Though I'm not sure you can handle this any other
way than to intercept the signal.

> (2) $term->readline() interface might already have a method to turn  
> echo off.

I have very limited knowledge of these things, especially of Perl's  
interfaces,
but this is what `perldoc -q password` has to say about it:

> 	There's an example of this in "crypt" in perlfunc).
> 	First, you put the terminal into "no echo" mode,
> 	then just read the password normally.  You may do
> 	this with an old-style ioctl() function, POSIX
> 	terminal control (see POSIX or its documentation
> 	the Camel Book), or a call to the stty program,
> 	with varying degrees of portability.
> 	
> 	You can also do this for most systems using the
> 	Term::ReadKey module from CPAN, which is easier
> 	to use and in theory more portable.
> 	
> 	   use Term::ReadKey;
> 	
> 	   ReadMode('noecho');
> 	   $password = ReadLine(0);

My code is from the "crypt" version. The other versions require
extra module dependencies, and the portability doesn't extend
immediately to Windows with any of these methods.

> We are not in a hurry, so I expect a polished resubmit after
> 1.5.4.  Thanks.

Great!

Sincerely,
Michael Witten

Re: [PATCH] git-send-email: ssh/login style password requests

[Michael Witten]

On 1 Feb 2008, at 1:38 PM, Michael Witten wrote:

>> I like what the patch tries to do, but the system() there feels
>> a tad ugly.  In addition it makes me wonder (1) what happens if
>> you ^C out from this while loop
>
> Good question! Though I'm not sure you can handle this any other
> way than to intercept the signal.

It turns out that --compose will also leave some temporary files
behind if git-send-email is terminated out of course.

Should I establish a general handler for SIGTERM and SIGINT?

Michael Witten

Re: [PATCH] git-send-email: ssh/login style password requests

[Junio C Hamano]

Michael Witten <mfwitten@MIT.EDU> writes:

> It turns out that --compose will also leave some temporary files
> behind if git-send-email is terminated out of course.
>
> Should I establish a general handler for SIGTERM and SIGINT?

I was mostly worried about ^C leaving the terminal in a funny
state with your system(stty).

I do not think you should lose the message the user carefully
composed using an editor upon seeing ^C.  The user may start to
compose, edit the message, and then at some later time decide to
stop the mail-sending procedure by ^C-ing out.  The user would
thank you if you somehow left an escape hatch to allow salvaging
the message, instead of having it to be re-typed from scratch.

Re: [PATCH] git-send-email: ssh/login style password requests

[Michael Witten]

On 1 Feb 2008, at 6:34 PM, Junio C Hamano wrote:

> Michael Witten <mfwitten@MIT.EDU> writes:
>
>> It turns out that --compose will also leave some temporary files
>> behind if git-send-email is terminated out of course.
>>
>> Should I establish a general handler for SIGTERM and SIGINT?
>
> I was mostly worried about ^C leaving the terminal in a funny
> state with your system(stty).
>
> I do not think you should lose the message the user carefully
> composed using an editor upon seeing ^C.  The user may start to
> compose, edit the message, and then at some later time decide to
> stop the mail-sending procedure by ^C-ing out.  The user would
> thank you if you somehow left an escape hatch to allow salvaging
> the message, instead of having it to be re-typed from scratch.

Indeed. How about a message stating that these files exist?

In any case, I'm looking for your endorsement of a signal handler
as the means to solve the main problem.

Re: [PATCH] git-send-email: ssh/login style password requests

[Junio C Hamano]

Michael Witten <mfwitten@MIT.EDU> writes:

> In any case, I'm looking for your endorsement of a signal handler
> as the means to solve the main problem.

Heh, unfortunately I am not "the resident Perl expert on the
list", so I would be waiting for suggestions from them.

[PATCH 1/2][Perlers?] git-send-email: ssh/login style password requests

[Michael Witten]

Whilst convenient, it is most unwise to record passwords
in any place but one's brain. Moreover, it is especially
foolish to store them in configuration files, even with
access permissions set accordingly.

git-send-email has been amended, so that if it detects
an smtp username without a password, it promptly prompts
for the password and masks the input for privacy.

Furthermore, the argument to --smtp-pass has been rendered
optional.

The documentation has been updated to reflect these changes.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---

	Perlers, please comment.
	
	Because AsciiDoc is a nuisance, the modifications
	below are somewhat difficult to discern. Please
	take look here:
	
		http://web.mit.edu/mfwitten/git-send-email.html
	
	Also, I have quoted the man page text below:
	
>	--smtp-user
>	   Username for SMTP-AUTH. In place of this option, the following
>	   configuration variables can be specified:
>	
>	   o  sendemail.smtpuser
>	
>	   o  sendemail.<identity>.smtpuser (see sendemail.identity).
>	
>	   However, --smtp-user always overrides these variables.
>	
>	   If a username is not specified (with --smtp-user or a configuration
>	   variable), then authentication is not attempted.
>	
>	--smtp-pass
>	   Password for SMTP-AUTH. The argument is optional: If no argument is
>	   specified, then the empty string is used as the password.
>	
>	   In place of this option, the following configuration variables can
>	   be specified:
>	
>	   o  sendemail.smtppass
>	
>	   o  sendemail.<identity>.smtppass (see sendemail.identity).
>	
>	   However, --smtp-pass always overrides these variables.
>	
>	   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 configuration variable), but no password has been
>	   specified (with --smtp-pass or a configuration variable), then the
>	   user is prompted for a password while the input is masked for
>	   privacy.

 Documentation/git-send-email.txt |   39 +++++++++++++++++++++++++++++++++----
 git-send-email.perl              |   23 ++++++++++++++++++---
 2 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0554f2b..4f4caa4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -96,11 +96,40 @@ The --cc option must be repeated for each user you want on the cc list.
 	servers typically listen to smtp port 25 and ssmtp port
 	465).
 
---smtp-user, --smtp-pass::
-	Username and password for SMTP-AUTH. Defaults are the values of
-	the configuration values 'sendemail.smtpuser' and
-	'sendemail.smtppass', but see also 'sendemail.identity'.
-	If not set, authentication is not attempted.
+--smtp-user::
+	Username for SMTP-AUTH. In place of this option, the following
+	configuration variables can be specified:
++
+--
+		* sendemail.smtpuser
+		* sendemail.<identity>.smtpuser (see sendemail.identity).
+--
++
+However, --smtp-user always overrides these variables.
++
+If a username is not specified (with --smtp-user or a
+configuration variable), then authentication is not attempted.
+
+--smtp-pass::
+	Password for SMTP-AUTH. The argument is optional: If no
+	argument is specified, then the empty string is used as
+	the password.
++
+In place of this option, the following configuration variables
+can be specified:
++
+--
+		* sendemail.smtppass
+		* sendemail.<identity>.smtppass (see sendemail.identity).
+--
++
+However, --smtp-pass always overrides these variables.
++
+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 configuration variable), but no password has been
+specified (with --smtp-pass or a configuration variable), then the
+user is prompted for a password while the input is masked for privacy.
 
 --smtp-ssl::
 	If set, connects to the SMTP server using SSL.
diff --git a/git-send-email.perl b/git-send-email.perl
index a1a9d14..ed0a473 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -157,7 +157,7 @@ my $compose_filename = ".msg.$$";
 
 # Variables we fill in automatically, or via prompting:
 my (@to,@cc,@initial_cc,@bcclist,@xh,
-	$initial_reply_to,$initial_subject,@files,$author,$sender,$compose,$time);
+	$initial_reply_to,$initial_subject,@files,$author,$sender,$smtp_authpass,$compose,$time);
 
 my $envelope_sender;
 
@@ -177,7 +177,7 @@ my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
-my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);
+my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_ssl);
 my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 my ($no_validate);
 
@@ -214,7 +214,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-port=s" => \$smtp_server_port,
 		    "smtp-user=s" => \$smtp_authuser,
-		    "smtp-pass=s" => \$smtp_authpass,
+		    "smtp-pass:s" => \$smtp_authpass,
 		    "smtp-ssl!" => \$smtp_ssl,
 		    "identity=s" => \$identity,
 		    "compose" => \$compose,
@@ -647,9 +647,24 @@ X-Mailer: git-send-email $gitversion
 			die "Unable to initialize SMTP properly.  Is there something wrong with your config?";
 		}
 
-		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
+		if (defined $smtp_authuser) {
+
+			if (!defined $smtp_authpass) {
+				
+				system "stty -echo";
+
+				do {
+					$_ = $term->readline("Password: ");
+				} while (!defined $_);
+
+				system "stty echo";
+
+				$smtp_authpass = $_ if ($_);
+			}
+
 			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
 		}
+
 		$smtp->mail( $raw_from ) or die $smtp->message;
 		$smtp->to( @recipients ) or die $smtp->message;
 		$smtp->data or die $smtp->message;
-- 
1.5.4.rc5.17.g8ca921-dirty

[PATCH 2/2][Perlers?] git-send-email: SIG{TERM,INT} handlers

[Michael Witten]

A single signal handler is used for both SIGTERM
and SIGINT in order to clean up after an uncouth
termination of git-send-email.

In particular, the handler resets the text color
(this cleanup was already present), turns on tty
echoing (in case termination occurrs during a
masked Password prompt), and informs the user of
of any temporary files created by --compose.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---

	Perlers, please comment.

 git-send-email.perl |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ed0a473..a0c9e8f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -24,8 +24,6 @@ use Data::Dumper;
 use Term::ANSIColor;
 use Git;
 
-$SIG{INT} = sub { print color("reset"), "\n"; exit };
-
 package FakeTerm;
 sub new {
 	my ($class, $reason) = @_;
@@ -201,6 +199,29 @@ my %config_settings = (
     "aliasesfile" => \@alias_files,
 );
 
+# Handle Uncouth Termination
+sub signal_handler{
+	
+	# Make text normal
+	print color("reset"), "\n";
+
+	# SMTP password masked
+	system "stty echo";
+
+	# tmp files from --compose
+	if (-e $compose_filename) {
+		print "'$compose_filename' contains an intermediate version of the email you were composing.\n";
+	}
+	if (-e ($compose_filename . ".final")) {
+		print "'$compose_filename.final' contains the composed email.\n"
+	}
+
+	exit;
+};
+
+$SIG{TERM} = \&signal_handler;
+$SIG{INT}  = \&signal_handler;
+
 # Begin by accumulating all the variables (defined above), that we will end up
 # needing, first, from the command line:
 
-- 
1.5.4.rc5.17.g8ca921-dirty

Re: [PATCH 2/2][Perlers?] git-send-email: SIG{TERM,INT} handlers

[Junio C Hamano]

Michael Witten <mfwitten@MIT.EDU> writes:

> +# Handle Uncouth Termination
> +sub signal_handler{
> +	
> +	# Make text normal
> +	print color("reset"), "\n";
> +
> +	# SMTP password masked
> +	system "stty echo";
> +
> +	# tmp files from --compose
> +	if (-e $compose_filename) {
> +		print "'$compose_filename' contains an intermediate version of the email you were composing.\n";
> +	}
> +	if (-e ($compose_filename . ".final")) {
> +		print "'$compose_filename.final' contains the composed email.\n"
> +	}
> +
> +	exit;
> +};
> +
> +$SIG{TERM} = \&signal_handler;
> +$SIG{INT}  = \&signal_handler;
> +
>  # Begin by accumulating all the variables (defined above), that we will end up
>  # needing, first, from the command line:

Now I think this patch (except the part about "stty echo") makes
sense, although I did not try killing a send-email session with
signals myself.

Success stories, people?

Re: [PATCH 1/2][Perlers?] git-send-email: ssh/login style password requests

[Junio C Hamano]

Michael Witten <mfwitten@MIT.EDU> writes:

>  2 files changed, 53 insertions(+), 9 deletions(-)

Documentation part looks very clear.  Thanks.

> +		if (defined $smtp_authuser) {
> +
> +			if (!defined $smtp_authpass) {
> +				
> +				system "stty -echo";
> +
> +				do {
> +					$_ = $term->readline("Password: ");
> +				} while (!defined $_);
> +
> +				system "stty echo";
> +
> +				$smtp_authpass = $_ if ($_);
> +			}
> +

Another example which appears in PerlFAQ #8 uses ReadKey with
its ReadLine, like this:

    use Term::ReadKey;
    ReadMode('noecho');
    $password = ReadLine(0);

which is different from Term::ReadLine's "ReadLine".  An earlier
example you cited from perlfunc.pod's crypt() entry does:

    system "stty -echo";
    print "Password: ";
    chomp($word = <STDIN>);
    print "\n";
    system "stty echo";

In either case, I was worried about the interaction between the
Term::ReadLine backend implementation and "stty".

Actually, I just tried this myself:

    #!/usr/bin/perl -w

    use Term::ReadLine;
    my $term = new Term::ReadLine 'foobar';

    my ($user, $password);
    while (!defined $user) {
            $user = $term->readline("User: ");
    }
    system 'stty -echo';
    while (!defined $password) {
            $password = $term->readline("Password: ");
    }
    system 'stty echo';
    print "You said <$user><$password>\n";
    print "ReadLine backend used was ", $term->ReadLine, "\n";

In my case, the backend was "Term::ReadLine::Perl".  A few
problems:

 * After typing "junio <Enter>" to "User:", an extra newline is
   left before "Password:" prompt;

 * "Password:" prompt still echoed password "abc".  There was no
   extra newline before "You said <junio><abc>".

 * In either case, typing <Enter> returns an empty string from
   $term->readline() so the "while (!defined)" loop does not buy
   us anything.


 

Re: [PATCH 1/2][Perlers?] git-send-email: ssh/login style password requests

[Michael Witten]

The crux of my conclusion comes after the ---------------------------- .

On 2 Feb 2008, at 4:31 PM, Junio C Hamano wrote:

> Actually, I just tried this myself:
>
>    #!/usr/bin/perl -w
>
>    use Term::ReadLine;
>    my $term = new Term::ReadLine 'foobar';
>
>    my ($user, $password);
>    while (!defined $user) {
>            $user = $term->readline("User: ");
>    }
>    system 'stty -echo';
>    while (!defined $password) {
>            $password = $term->readline("Password: ");
>    }
>    system 'stty echo';
>    print "You said <$user><$password>\n";
>    print "ReadLine backend used was ", $term->ReadLine, "\n";
>
> In my case, the backend was "Term::ReadLine::Perl".  A few
> problems:
>
> * After typing "junio <Enter>" to "User:", an extra newline is
>   left before "Password:" prompt;

I didn't have this problem.

> * "Password:" prompt still echoed password "abc".  There was no
>   extra newline before "You said <junio><abc>".

Indeed. I tested your code and my git-send-email code with all three
backend implementations (Term::ReadLine::Stub, Term::ReadLine::Gnu, and
Term::ReadLine::Perl). The problem with echoing seems to be a fault of
the Term::ReadLine::Perl implementation.

> * In either case, typing <Enter> returns an empty string from
>   $term->readline() so the "while (!defined)" loop does not buy
>   us anything.

Frankly, I wrote that readline code according to the other uses of  
readline,
as I am not well versed in these things.

Because all other uses of readline are wrapped in such while loops, I  
assume
that they are meant to cover systems with non-blocking (unbuffered)  
IO, rather
than to continue to prompt the user due to empty strings.

Should empty-string passwords not be allowed?

-------------------------------------------------

> Another example which appears in PerlFAQ #8 uses ReadKey with
> its ReadLine, like this:
>
>    use Term::ReadKey;
>    ReadMode('noecho');
>    $password = ReadLine(0);
>
> which is different from Term::ReadLine's "ReadLine".  An earlier
> example you cited from perlfunc.pod's crypt() entry does:
>
>    system "stty -echo";
>    print "Password: ";
>    chomp($word = <STDIN>);
>    print "\n";
>    system "stty echo";
>
> In either case, I was worried about the interaction between the
> Term::ReadLine backend implementation and "stty".

This got me thinking: At first I wanted to use readline for the  
password prompt,
because I figured it would allow the user better editing facilities,  
especially
with regard to the arrow keys. However, it occurred to me that perhaps  
this should
not be the case; perhaps arrow keys are meant to be useable in  
passwords, etc.

Well, this turns out to be the case! (which may not be a surprise to  
most people,
but it was to me). Passwords can actually contain some of the keycodes  
that read-
line converts into editing commands (now you know which characters you  
don't have
to test when cracking my password). Therefore, we shouldn't even  
bother using the
readline backend for password prompting; neither passwd nor ssh use  
readline, for
example.

I support the 'crypt' method, because the Term::ReadKey approach  
requires another
module dependency.

Sincerely,
Michael Witten

Re: [PATCH 1/2][Perlers?] git-send-email: ssh/login style password requests

[Michael Witten]

On 3 Feb 2008, at 12:59 PM, Michael Witten wrote:

>> * "Password:" prompt still echoed password "abc".  There was no
>>  extra newline before "You said <junio><abc>".
>
> Indeed. I tested your code and my git-send-email code with all three
> backend implementations (Term::ReadLine::Stub, Term::ReadLine::Gnu,  
> and
> Term::ReadLine::Perl). The problem with echoing seems to be a fault of
> the Term::ReadLine::Perl implementation.

Yiarg!

It would appear that Term::ReadLine::Gnu's implementation (of at least
readline) delays signal handling, so that ^c (C-d) doesn't do anything  
while
the prompt is up; I suppose this is an artifact of of this:

	http://perldoc.perl.org/perlipc.html#Deferred-Signals-(Safe-Signals)

For instance:

	What subject should the initial email start with?<^c><ENTER>

Only after <ENTER> is the handler invoked.

>> * In either case, typing <Enter> returns an empty string from
>>  $term->readline() so the "while (!defined)" loop does not buy
>>  us anything.
>
> Frankly, I wrote that readline code according to the other uses of  
> readline,
> as I am not well versed in these things.
>
> Because all other uses of readline are wrapped in such while loops,  
> I assume
> that they are meant to cover systems with non-blocking (unbuffered)  
> IO, rather
> than to continue to prompt the user due to empty strings.
>
> Should empty-string passwords not be allowed?


Perhaps these loops are meant to handle ^d (C-d; VEOF). This is  
because issuing
EOF causes input operators/functions to return undefined; in fact,  
using just the
'crypt' method, ^d is unhandled, causing a runtime error. Is there a  
way to setup
a signal handler for this control character rather than using checks?

Michael Witten

[PATCH 1/3][V.2] git-send-email: ssh/login style password requests

[Michael Witten]

Whilst convenient, it is most unwise to record passwords
in any place but one's brain. Moreover, it is especially
foolish to store them in configuration files, even with
access permissions set accordingly.

git-send-email has been amended, so that if it detects
an smtp username without a password, it promptly prompts
for the password and masks the input for privacy.

Furthermore, the argument to --smtp-pass has been rendered
optional.

The documentation has been updated to reflect these changes.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---

	The backend ambiguity of Term:ReadLine is avoided, and EOF
	is handled for the Password Prompt.

 Documentation/git-send-email.txt |   39 +++++++++++++++++++++++++++++++++----
 git-send-email.perl              |   25 ++++++++++++++++++++---
 2 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0554f2b..4f4caa4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -96,11 +96,40 @@ The --cc option must be repeated for each user you want on the cc list.
 	servers typically listen to smtp port 25 and ssmtp port
 	465).
 
---smtp-user, --smtp-pass::
-	Username and password for SMTP-AUTH. Defaults are the values of
-	the configuration values 'sendemail.smtpuser' and
-	'sendemail.smtppass', but see also 'sendemail.identity'.
-	If not set, authentication is not attempted.
+--smtp-user::
+	Username for SMTP-AUTH. In place of this option, the following
+	configuration variables can be specified:
++
+--
+		* sendemail.smtpuser
+		* sendemail.<identity>.smtpuser (see sendemail.identity).
+--
++
+However, --smtp-user always overrides these variables.
++
+If a username is not specified (with --smtp-user or a
+configuration variable), then authentication is not attempted.
+
+--smtp-pass::
+	Password for SMTP-AUTH. The argument is optional: If no
+	argument is specified, then the empty string is used as
+	the password.
++
+In place of this option, the following configuration variables
+can be specified:
++
+--
+		* sendemail.smtppass
+		* sendemail.<identity>.smtppass (see sendemail.identity).
+--
++
+However, --smtp-pass always overrides these variables.
++
+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 configuration variable), but no password has been
+specified (with --smtp-pass or a configuration variable), then the
+user is prompted for a password while the input is masked for privacy.
 
 --smtp-ssl::
 	If set, connects to the SMTP server using SSL.
diff --git a/git-send-email.perl b/git-send-email.perl
index a1a9d14..fec55ea 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -157,7 +157,7 @@ my $compose_filename = ".msg.$$";
 
 # Variables we fill in automatically, or via prompting:
 my (@to,@cc,@initial_cc,@bcclist,@xh,
-	$initial_reply_to,$initial_subject,@files,$author,$sender,$compose,$time);
+	$initial_reply_to,$initial_subject,@files,$author,$sender,$smtp_authpass,$compose,$time);
 
 my $envelope_sender;
 
@@ -177,7 +177,7 @@ my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
-my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);
+my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_ssl);
 my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 my ($no_validate);
 
@@ -214,7 +214,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-port=s" => \$smtp_server_port,
 		    "smtp-user=s" => \$smtp_authuser,
-		    "smtp-pass=s" => \$smtp_authpass,
+		    "smtp-pass:s" => \$smtp_authpass,
 		    "smtp-ssl!" => \$smtp_ssl,
 		    "identity=s" => \$identity,
 		    "compose" => \$compose,
@@ -647,9 +647,26 @@ X-Mailer: git-send-email $gitversion
 			die "Unable to initialize SMTP properly.  Is there something wrong with your config?";
 		}
 
-		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
+		if (defined $smtp_authuser) {
+
+			if (!defined $smtp_authpass) {
+
+				system "stty -echo";
+
+				do {
+					print "Password: ";
+					$_ = <STDIN>;
+					print "\n";
+				} while (!defined $_);
+
+				chomp($smtp_authpass = $_);
+
+				system "stty echo";
+			}
+
 			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
 		}
+
 		$smtp->mail( $raw_from ) or die $smtp->message;
 		$smtp->to( @recipients ) or die $smtp->message;
 		$smtp->data or die $smtp->message;
-- 
1.5.4.9.gcc769-dirty

[PATCH 2/3][V.2] git-send-email: SIG{TERM,INT} handlers

[Michael Witten]

A single signal handler is used for both SIGTERM
and SIGINT in order to clean up after an uncouth
termination of git-send-email.

In particular, the handler resets the text color
(this cleanup was already present), turns on tty
echoing (in case termination occurrs during a
masked Password prompt), and informs the user of
of any temporary files created by --compose.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
 git-send-email.perl |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fec55ea..14268fc 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -24,8 +24,6 @@ use Data::Dumper;
 use Term::ANSIColor;
 use Git;
 
-$SIG{INT} = sub { print color("reset"), "\n"; exit };
-
 package FakeTerm;
 sub new {
 	my ($class, $reason) = @_;
@@ -201,6 +199,29 @@ my %config_settings = (
     "aliasesfile" => \@alias_files,
 );
 
+# Handle Uncouth Termination
+sub signal_handler {
+
+	# Make text normal
+	print color("reset"), "\n";
+
+	# SMTP password masked
+	system "stty echo";
+
+	# tmp files from --compose
+	if (-e $compose_filename) {
+		print "'$compose_filename' contains an intermediate version of the email you were composing.\n";
+	}
+	if (-e ($compose_filename . ".final")) {
+		print "'$compose_filename.final' contains the composed email.\n"
+	}
+
+	exit;
+};
+
+$SIG{TERM} = \&signal_handler;
+$SIG{INT}  = \&signal_handler;
+
 # Begin by accumulating all the variables (defined above), that we will end up
 # needing, first, from the command line:
 
-- 
1.5.4.9.gcc769-dirty

[PATCH 3/3][V.2] git-send-email: Better handling of EOF [^D]

[Michael Witten]

Before, when the user sent the EOF control character,
the prompts would be repeated on the same line as the
previous prompt.

Now, repeat prompts display on separate lines.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
 git-send-email.perl |   46 +++++++++++++++++++++++++++++-----------------
 1 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 14268fc..6724810 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -376,9 +376,12 @@ if (@files) {
 my $prompting = 0;
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter;
-	do {
+
+	while (1) {
 		$_ = $term->readline("Who should the emails appear to be from? [$sender] ");
-	} while (!defined $_);
+		last if defined $_;
+		print "\n";
+	}
 
 	$sender = $_ if ($_);
 	print "Emails will be sent from: ", $sender, "\n";
@@ -386,10 +389,14 @@ if (!defined $sender) {
 }
 
 if (!@to) {
-	do {
-		$_ = $term->readline("Who should the emails be sent to? ",
-				"");
-	} while (!defined $_);
+
+
+	while (1) {
+		$_ = $term->readline("Who should the emails be sent to? ", "");
+		last if defined $_;
+		print "\n";
+	}
+
 	my $to = $_;
 	push @to, split /,/, $to;
 	$prompting++;
@@ -411,20 +418,23 @@ sub expand_aliases {
 @bcclist = expand_aliases(@bcclist);
 
 if (!defined $initial_subject && $compose) {
-	do {
-		$_ = $term->readline("What subject should the initial email start with? ",
-			$initial_subject);
-	} while (!defined $_);
+	while (1) {
+		$_ = $term->readline("What subject should the initial email start with? ", $initial_subject);
+		last if defined $_;
+		print "\n";
+	}
+
 	$initial_subject = $_;
 	$prompting++;
 }
 
 if ($thread && !defined $initial_reply_to && $prompting) {
-	do {
-		$_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ",
-			$initial_reply_to);
-	} while (!defined $_);
-
+	while (1) {
+		$_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ", $initial_reply_to);
+		last if defined $_;
+		print "\n";
+	}
+	
 	$initial_reply_to = $_;
 }
 if (defined $initial_reply_to && $_ ne "") {
@@ -474,9 +484,11 @@ EOT
 	close(C);
 	close(C2);
 
-	do {
+	while (1) {
 		$_ = $term->readline("Send this email? (y|n) ");
-	} while (!defined $_);
+		last if defined $_;
+		print "\n";
+	}
 
 	if (uc substr($_,0,1) ne 'Y') {
 		cleanup_compose_files();
-- 
1.5.4.9.gcc769-dirty