threads / patch / 28030

patchgit-send-email: Add AUTH LOGIN support

Subject: [PATCH] git-send-email: Add AUTH LOGIN support

## tl;dr

19 messages between Aug 6, 2011 and Sep 29, 2011. Diffs are folded; open one to read it.

replies: 18people: 5as markdown or json

Joe Perches· Aug 6, 2011, 00:40 UTC · lore

The current perl Net::SMTP support will not use AUTH LOGIN when other authentication options are available.

Add an option to force the use of AUTH LOGIN when necessary. (Like when using my current hosted email server, grumble)

Signed-off-by: Joe Perches <joe@perches.com>
---
 Documentation/git-send-email.txt |    3 +++
 git-send-email.perl              |   19 +++++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)
Show changes to 2 files +20 −2

Documentation/git-send-email.txt, git-send-email.perl

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 327233c..9595773 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -198,6 +198,9 @@ must be used for each option.
 	if a username is not specified (with '--smtp-user' or 'sendemail.smtpuser'),
 	then authentication is not attempted.
 
+--smtp-auth=<authorization_type>::
+	Force the smtp authentication to use a particular type.
+	Currently supported forced style is "login"
 
 Automating
 ~~~~~~~~~~
diff --git a/git-send-email.perl b/git-send-email.perl
index 98ab33a..37dfbe7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catfile);
 use Error qw(:try);
 use Git;
+use MIME::Base64;
 
 Getopt::Long::Configure qw/ pass_through /;
 
@@ -193,7 +194,7 @@ sub do_edit {
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
-my ($smtp_authuser, $smtp_encryption);
+my ($smtp_authuser, $smtp_encryption, $smtp_auth);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -218,6 +219,7 @@ my %config_settings = (
     "smtpserveroption" => \@smtp_server_options,
     "smtpuser" => \$smtp_authuser,
     "smtppass" => \$smtp_authpass,
+    "smtpauth" => \$smtp_auth,
     "smtpdomain" => \$smtp_domain,
     "to" => \@initial_to,
     "tocmd" => \$to_cmd,
@@ -293,6 +295,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "smtp-pass:s" => \$smtp_authpass,
 		    "smtp-ssl" => sub { $smtp_encryption = 'ssl' },
 		    "smtp-encryption=s" => \$smtp_encryption,
+		    "smtp-auth=s" => \$smtp_auth,
 		    "smtp-debug:i" => \$debug_net_smtp,
 		    "smtp-domain:s" => \$smtp_domain,
 		    "identity=s" => \$identity,
@@ -1111,7 +1114,19 @@ X-Mailer: git-send-email $gitversion
 				system "stty echo";
 			}
 
-			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
+			if (defined $smtp_auth && $smtp_auth =~ /^login$/i) {
+
+			    $smtp->datasend("AUTH LOGIN\n");
+			    $smtp->response();
+			    $smtp->datasend(encode_base64("$smtp_authuser"));
+			    $smtp->response();
+			    $smtp->datasend(encode_base64("$smtp_authpass"));
+			    $smtp->response();
+
+			} else {
+
+			    $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
+			}
 		}
 
 		$smtp->mail( $raw_from ) or die $smtp->message;
-- 
1.7.6.131.g99019
Junio C Hamano· Aug 6, 2011, 04:54 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email: Add AUTH LOGIN support

Joe Perches <joe@perches.com> writes:
> The current perl Net::SMTP support will not use AUTH LOGIN
> when other authentication options are available.
Even after reading this excuse,...
Show 13 quoted lines
> +			if (defined $smtp_auth && $smtp_auth =~ /^login$/i) {
> +
> +			    $smtp->datasend("AUTH LOGIN\n");
> +			    $smtp->response();
> +			    $smtp->datasend(encode_base64("$smtp_authuser"));
> +			    $smtp->response();
> +			    $smtp->datasend(encode_base64("$smtp_authpass"));
> +			    $smtp->response();
> +
> +			} else {
> +
> +			    $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> +			}

... this makes me feel dirty X-(. Not the desire to force "AUTH LOGIN", but the implementation to hand-roll the protocol exchange.

I'd rather want to know _why_ Net::SMTP does not support it in the first place, and what it does for "other authentication options" that are available. Does it try them in turn until it finds the one that works? Why doesn't it fall back on "AUTH LOGIN" then?

Specifically, if there is a reason to avoid this plaintext authentication method when other options are _available_ (which presumably would be the reason why Net::SMTP chooses not to support it), and if there is a reason on the user's side to _force_ this method even when people who wrote Net::SMTP does not recommend it be used, wouldn't it be natural to expect that there should be a way to configure the connection to use it, without resorting to coding the protocol exchange by hand line this?

It probably is not as simple as installing Authen::SASL::*::LOGIN, but still...

Joe Perches· Aug 6, 2011, 05:21 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-send-email: Add AUTH LOGIN support

On Fri, 2011-08-05 at 21:54 -0700, Junio C Hamano wrote:
Show 30 quoted lines
> Joe Perches <joe@perches.com> writes:
> > The current perl Net::SMTP support will not use AUTH LOGIN
> > when other authentication options are available.
> Even after reading this excuse,...
> > +			if (defined $smtp_auth && $smtp_auth =~ /^login$/i) {
> > +
> > +			    $smtp->datasend("AUTH LOGIN\n");
> > +			    $smtp->response();
> > +			    $smtp->datasend(encode_base64("$smtp_authuser"));
> > +			    $smtp->response();
> > +			    $smtp->datasend(encode_base64("$smtp_authpass"));
> > +			    $smtp->response();
> > +
> > +			} else {
> > +
> > +			    $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> > +			}
> ... this makes me feel dirty X-(. Not the desire to force "AUTH LOGIN",
> but the implementation to hand-roll the protocol exchange.
> I'd rather want to know _why_ Net::SMTP does not support it in the first
> place, and what it does for "other authentication options" that are
> available. Does it try them in turn until it finds the one that works? Why
> doesn't it fall back on "AUTH LOGIN" then?
> Specifically, if there is a reason to avoid this plaintext authentication
> method when other options are _available_ (which presumably would be the
> reason why Net::SMTP chooses not to support it), and if there is a reason
> on the user's side to _force_ this method even when people who wrote
> Net::SMTP does not recommend it be used, wouldn't it be natural to expect
> that there should be a way to configure the connection to use it, without
> resorting to coding the protocol exchange by hand line this?
I needed something now.

You are right but I believe it would take too long to get updates to Net::SMTP in place. Doing this admitted ugliness in git-send-email works for me and seems to me to be appropriate for now.

I looked, there isn't a method to force a particular AUTH type documented. I also didn't care to rewrite Net::SMTP right now. This "works for me"...

> It probably is not as simple as installing Authen::SASL::*::LOGIN, but
> still...
cheers, Joe
Joe Perches· Aug 19, 2011, 01:16 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email: Add AUTH LOGIN support

On Fri, 2011-08-05 at 22:21 -0700, Joe Perches wrote:
Show 45 quoted lines
> On Fri, 2011-08-05 at 21:54 -0700, Junio C Hamano wrote:
> > Joe Perches <joe@perches.com> writes:
> > > The current perl Net::SMTP support will not use AUTH LOGIN
> > > when other authentication options are available.
> > Even after reading this excuse,...
> > > +			if (defined $smtp_auth && $smtp_auth =~ /^login$/i) {
> > > +
> > > +			    $smtp->datasend("AUTH LOGIN\n");
> > > +			    $smtp->response();
> > > +			    $smtp->datasend(encode_base64("$smtp_authuser"));
> > > +			    $smtp->response();
> > > +			    $smtp->datasend(encode_base64("$smtp_authpass"));
> > > +			    $smtp->response();
> > > +
> > > +			} else {
> > > +
> > > +			    $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> > > +			}
> > ... this makes me feel dirty X-(. Not the desire to force "AUTH LOGIN",
> > but the implementation to hand-roll the protocol exchange.
> > I'd rather want to know _why_ Net::SMTP does not support it in the first
> > place, and what it does for "other authentication options" that are
> > available. Does it try them in turn until it finds the one that works? Why
> > doesn't it fall back on "AUTH LOGIN" then?
> > Specifically, if there is a reason to avoid this plaintext authentication
> > method when other options are _available_ (which presumably would be the
> > reason why Net::SMTP chooses not to support it), and if there is a reason
> > on the user's side to _force_ this method even when people who wrote
> > Net::SMTP does not recommend it be used, wouldn't it be natural to expect
> > that there should be a way to configure the connection to use it, without
> > resorting to coding the protocol exchange by hand line this?
> 
> I needed something now.
> 
> You are right but I believe it would take too long
> to get updates to Net::SMTP in place. Doing this
> admitted ugliness in git-send-email works for me and
> seems to me to be appropriate for now.
> 
> I looked, there isn't a method to force a particular
> AUTH type documented.  I also didn't care to rewrite
> Net::SMTP right now.  This "works for me"...
> 
> > It probably is not as simple as installing Authen::SASL::*::LOGIN, but
> > still...
I think my patch should be applied until Net::SMTP is updated.
Junio C Hamano· Aug 19, 2011, 23:09 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email: Add AUTH LOGIN support

Joe Perches <joe@perches.com> writes:
Show 12 quoted lines
> On Fri, 2011-08-05 at 22:21 -0700, Joe Perches wrote:
>> 
>> I needed something now.
>> 
>> You are right but I believe it would take too long
>> to get updates to Net::SMTP in place. Doing this
>> admitted ugliness in git-send-email works for me and
>> seems to me to be appropriate for now.
>> 
>> I looked, there isn't a method to force a particular
>> AUTH type documented.  I also didn't care to rewrite
>> Net::SMTP right now.  This "works for me"...
Good for you ;-)
>> > It probably is not as simple as installing Authen::SASL::*::LOGIN, but
>> > still...
>
> I think my patch should be applied until Net::SMTP is updated.
And you already have applied to your copy, no?

I understand you needed something _now_ and that is why you wrote it, but the thing is, I don't need the ugliness nor an ability to force AUTH LOGIN right now, so I do not necessarily agree with you that the patch _should_ be applied to my tree.

Joe Perches· Aug 19, 2011, 23:24 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-send-email: Add AUTH LOGIN support

On Fri, 2011-08-19 at 16:09 -0700, Junio C Hamano wrote:
> Joe Perches <joe@perches.com> writes:
> > I think my patch should be applied until Net::SMTP is updated.
> And you already have applied to your copy, no?
True.
> I do not necessarily agree with you that the patch _should_
> be applied to my tree.

Your choice. It does make tracking git development a bit of a pain for me though.

There doesn't seem to be anything like a patchwork queue for git development to check status of patches.

I've sent a couple of other patches to git-send-email.

http://marc.info/?l=git&m=131190328311281&w=2 http://marc.info/?l=git&m=131131975804893&w=2

Any comments or plans to ack/nack those?
Junio C Hamano· Aug 20, 2011, 01:01 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email: Add AUTH LOGIN support

Joe Perches <joe@perches.com> writes:
Show 6 quoted lines
> I've sent a couple of other patches to git-send-email.
>
> http://marc.info/?l=git&m=131190328311281&w=2
> http://marc.info/?l=git&m=131131975804893&w=2
>
> Any comments or plans to ack/nack those?

Sorry, but I am not personally interested in send-email enough to go to marc.info archive that does not give me the message in ready-to-apply format with "git am".

On-list discussion is the most important signal I use to convince myself that the issues that discussed patches attempt to tackle are worth addressing. When I do not see any discussion, I myself may decide that they are important, or I may not.

Re-sending them might get others with similar needs for the patches involved in the discussion.

Here are what _I_ think about the above two.
 - I've seen enough complaints that send-email sends too many Cc:s to
   unintended parties, and of an opinion that these extra recipients should
   be added to the files you feed to send-email as headers, instead of adding
   noise to commit log messages, so adding new Cc sources and then giving a
   way to suppress them didn't look like a good change to me.
 - I think I saw a few positive responses to the "editor cruft" patch, but
   the way the patch was implemented, it will invite low-value "my obscure
   editor uses this pattern, so add it" follow-up patches, and compared to
   that downside, I didn't like the benefit of the patch well enough to pick
   it up.
Zbigniew Jędrzejewski-Szmek· Sep 24, 2011, 15:49 UTC · re: Junio C Hamano · lore

[PATCH] send-email: auth plain/login fix

git send-email was not authenticating properly when communicating over TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is e.g. the standard server setup under debian with exim4 and probably everywhere where system accounts are used.

The solution comes from this forum thread: http://www.perlmonks.org/?node_id=904354

This patch is tested by sending it.

Before: Net::SMTP>>> Net::SMTP(2.31) Net::SMTP>>> Net::Cmd(2.29) Net::SMTP>>> Exporter(5.64_01) Net::SMTP>>> IO::Socket::INET(1.31) Net::SMTP>>> IO::Socket(1.31) Net::SMTP>>> IO::Handle(1.28) ... Net::SMTP=GLOB(0x238f668)>>> STARTTLS Net::SMTP=GLOB(0x238f668)<<< 220 2.0.0 Ready to start TLS Net::SMTP::SSL=GLOB(0x238f668)>>> EHLO localhost.localdomain Net::SMTP::SSL=GLOB(0x238f668)<<< 250-yyy.yyy.yyy Hello xxx.xxx [xxx.xxx.xxx.xxx], pleased to meet you Net::SMTP::SSL=GLOB(0x238f668)<<< 250-ENHANCEDSTATUSCODES Net::SMTP::SSL=GLOB(0x238f668)<<< 250-PIPELINING Net::SMTP::SSL=GLOB(0x238f668)<<< 250-8BITMIME Net::SMTP::SSL=GLOB(0x238f668)<<< 250-SIZE 80000000 Net::SMTP::SSL=GLOB(0x238f668)<<< 250-DSN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH=LOGIN PLAIN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH LOGIN PLAIN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-DELIVERBY Net::SMTP::SSL=GLOB(0x238f668)<<< 250 HELP Password: Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified 5.5.2 AUTH mechanism must be specified

After: Net::SMTP=GLOB(0x1ac4a60)>>> STARTTLS Net::SMTP=GLOB(0x1ac4a60)<<< 220 2.0.0 Ready to start TLS Net::SMTP::SSL=GLOB(0x1ac4a60)>>> EHLO localhost.localdomain Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-yyy.yyy.yyy Hello xxx.xxx [xxx.xxx.xxx.xxx], pleased to meet you Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-ENHANCEDSTATUSCODES Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-PIPELINING Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-8BITMIME Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-SIZE 80000000 Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-DSN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-AUTH=LOGIN PLAIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-AUTH LOGIN PLAIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-DELIVERBY Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250 HELP Password: Net::SMTP::SSL=GLOB(0x1ac4a60)>>> AUTH LOGIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 334 VXNlcm5hbWU6 Net::SMTP::SSL=GLOB(0x1ac4a60)>>> emJ5c3plaw== Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 334 UGFzc3dvcmQ6 Net::SMTP::SSL=GLOB(0x1ac4a60)>>> dGVzdA== Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 535 5.7.0 authentication failed 5.7.0 authentication failed

The password is incorrect in this snippet, but the protocol works correctly.
---
 git-send-email.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to diff +1 −0
diff --git git-send-email.perl git-send-email.perl
index 37dfbe7..100fbd9 100755
--- git-send-email.perl
+++ git-send-email.perl
@@ -27,6 +27,7 @@ use Term::ANSIColor;
 use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catfile);
 use Error qw(:try);
+use Authen::SASL qw(Perl);
 use Git;
 use MIME::Base64;
 
-- 
1.7.6
Junio C Hamano· Sep 26, 2011, 16:38 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH] send-email: auth plain/login fix

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> git send-email was not authenticating properly when communicating over
> TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is
> e.g. the standard server setup under debian with exim4 and probably
> everywhere where system accounts are used.
Now that's a solution that makes me feel less dirty than
  http://thread.gmane.org/gmane.comp.version-control.git/178818/focus=178824

even though, by forcing Authen::SASL::Perl to be used bypassing XS and Cyrus variants, this _might_ be introducing regression for others. We'll find out soon enough if anybody screams ;-)

I see you already researched previous discussion and have Joe Perches in the loop; Thanks.

Joe Perches· Sep 26, 2011, 16:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] send-email: auth plain/login fix

On Mon, 2011-09-26 at 09:38 -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> 
> > git send-email was not authenticating properly when communicating over
> > TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is
> > e.g. the standard server setup under debian with exim4 and probably
> > everywhere where system accounts are used.
> 
> Now that's a solution that makes me feel less dirty than
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/178818/focus=178824

Dirty is dirty. I don't see this as better or worse really, but thanks Zbigniew for bringing it up again.

It'd be best if Graham Barr, the module author (cc'd), could respond to the issue.

cheers, Joe
Show 6 quoted lines
> even though, by forcing Authen::SASL::Perl to be used bypassing XS and
> Cyrus variants, this _might_ be introducing regression for others. We'll
> find out soon enough if anybody screams ;-)
> 
> I see you already researched previous discussion and have Joe Perches in
> the loop; Thanks.
Jakub Narebski· Sep 26, 2011, 17:24 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH] send-email: auth plain/login fix

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
Show 7 quoted lines
> git send-email was not authenticating properly when communicating over
> TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is
> e.g. the standard server setup under debian with exim4 and probably
> everywhere where system accounts are used.
> 
> The solution comes from this forum thread:
> http://www.perlmonks.org/?node_id=904354
Signoff?
Show 15 quoted lines
> ---
>  git-send-email.perl |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git git-send-email.perl git-send-email.perl
> index 37dfbe7..100fbd9 100755
> --- git-send-email.perl
> +++ git-send-email.perl
> @@ -27,6 +27,7 @@ use Term::ANSIColor;
>  use File::Temp qw/ tempdir tempfile /;
>  use File::Spec::Functions qw(catfile);
>  use Error qw(:try);
> +use Authen::SASL qw(Perl);
>  use Git;
>  use MIME::Base64;

Shouldn't we load Authen::SASL only when it is necessary (on demand), rather than forcing everybody who use send-email (even if via sendmail, or with other authentication support)?

We load Net::SMTP::SSL only on demand.  
It would be:
   require Authen::SASL;
   Authen::SASL->import(qw(Perl));
-- 
Jakub Narębski
Zbigniew Jędrzejewski-Szmek· Sep 27, 2011, 21:36 UTC · re: Jakub Narebski · lore

[PATCH v2] send-email: auth plain/login fix

git send-email was not authenticating properly when communicating over TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is e.g. the standard server setup under debian with exim4 and probably everywhere where system accounts are used.

The solution comes from this forum thread: http://www.perlmonks.org/?node_id=904354

This patch is tested by sending it :)

Before: Net::SMTP>>> Net::SMTP(2.31) Net::SMTP>>> Net::Cmd(2.29) Net::SMTP>>> Exporter(5.64_01) Net::SMTP>>> IO::Socket::INET(1.31) Net::SMTP>>> IO::Socket(1.31) Net::SMTP>>> IO::Handle(1.28) ... Net::SMTP=GLOB(0x238f668)>>> STARTTLS Net::SMTP=GLOB(0x238f668)<<< 220 2.0.0 Ready to start TLS Net::SMTP::SSL=GLOB(0x238f668)>>> EHLO localhost.localdomain Net::SMTP::SSL=GLOB(0x238f668)<<< 250-yyy.yyy.yyy Hello xxx.xxx [xxx.xxx.xxx.xxx], pleased to meet you Net::SMTP::SSL=GLOB(0x238f668)<<< 250-ENHANCEDSTATUSCODES Net::SMTP::SSL=GLOB(0x238f668)<<< 250-PIPELINING Net::SMTP::SSL=GLOB(0x238f668)<<< 250-8BITMIME Net::SMTP::SSL=GLOB(0x238f668)<<< 250-SIZE 80000000 Net::SMTP::SSL=GLOB(0x238f668)<<< 250-DSN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH=LOGIN PLAIN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH LOGIN PLAIN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-DELIVERBY Net::SMTP::SSL=GLOB(0x238f668)<<< 250 HELP Password: Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified 5.5.2 AUTH mechanism must be specified

After: Net::SMTP=GLOB(0x1ac4a60)>>> STARTTLS Net::SMTP=GLOB(0x1ac4a60)<<< 220 2.0.0 Ready to start TLS Net::SMTP::SSL=GLOB(0x1ac4a60)>>> EHLO localhost.localdomain Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-yyy.yyy.yyy Hello xxx.xxx [xxx.xxx.xxx.xxx], pleased to meet you Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-ENHANCEDSTATUSCODES Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-PIPELINING Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-8BITMIME Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-SIZE 80000000 Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-DSN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-AUTH=LOGIN PLAIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-AUTH LOGIN PLAIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-DELIVERBY Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250 HELP Password: Net::SMTP::SSL=GLOB(0x1ac4a60)>>> AUTH LOGIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 334 VXNlcm5hbWU6 Net::SMTP::SSL=GLOB(0x1ac4a60)>>> emJ5c3plaw== Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 334 UGFzc3dvcmQ6 Net::SMTP::SSL=GLOB(0x1ac4a60)>>> dGVzdA== Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 535 5.7.0 authentication failed 5.7.0 authentication failed

The password is incorrect in this snippet, but the protocol works correctly.
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 v2: - added sign-off as requested, although the patch is as trivial
       as it gets
     - the import is performed only if it will be used
 git-send-email.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to git-send-email.perl +2 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 37dfbe7..5a22d18 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1098,6 +1098,8 @@ X-Mailer: git-send-email $gitversion
 		}
 
 		if (defined $smtp_authuser) {
+			require Authen::SASL;
+			Authen::SASL->import(qw(Perl));
 
 			if (!defined $smtp_authpass) {
 
-- 
1.7.6
Jeff King· Sep 27, 2011, 21:48 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH v2] send-email: auth plain/login fix

On Tue, Sep 27, 2011 at 11:36:59PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
>  v2: - added sign-off as requested, although the patch is as trivial
>        as it gets
>      - the import is performed only if it will be used

Nice, it's much better not to make the dependency required by people who are not using smtp auth, but...

Show 10 quoted lines
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 37dfbe7..5a22d18 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1098,6 +1098,8 @@ X-Mailer: git-send-email $gitversion
>  		}
>  
>  		if (defined $smtp_authuser) {
> +			require Authen::SASL;
> +			Authen::SASL->import(qw(Perl));

What about people who are using smtp auth, but don't need or have Authen::SASL? I.e., shouldn't this be:

  eval {
    require Authen::SASL;
    Authen::SASL->import(qw(Perl));
  }

and if we hit an error, just ignore it and continue without Authen::SASL loaded?

-Peff
Zbigniew Jędrzejewski-Szmek· Sep 28, 2011, 10:26 UTC · re: Jakub Narebski · lore

[PATCH v3] send-email: auth plain/login fix

git send-email was not authenticating properly when communicating over TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is e.g. the standard server setup under debian with exim4 and probably everywhere where system accounts are used.

The problem (only?) exists when libauthen-sasl-cyrus-perl (Authen::SASL::Cyrus) is installed. Importing Authen::SASL::Perl makes Authen::SASL use the perl implementation which works better.

The solution is based on this forum thread: http://www.perlmonks.org/?node_id=904354.

This patch is tested by sending it :)

Before: Net::SMTP>>> Net::SMTP(2.31) Net::SMTP>>> Net::Cmd(2.29) Net::SMTP>>> Exporter(5.64_01) Net::SMTP>>> IO::Socket::INET(1.31) Net::SMTP>>> IO::Socket(1.31) Net::SMTP>>> IO::Handle(1.28) ... Net::SMTP=GLOB(0x238f668)>>> STARTTLS Net::SMTP=GLOB(0x238f668)<<< 220 2.0.0 Ready to start TLS Net::SMTP::SSL=GLOB(0x238f668)>>> EHLO localhost.localdomain Net::SMTP::SSL=GLOB(0x238f668)<<< 250-yyy.yyy.yyy Hello xxx.xxx [xxx.xxx.xxx.xxx], pleased to meet you Net::SMTP::SSL=GLOB(0x238f668)<<< 250-ENHANCEDSTATUSCODES Net::SMTP::SSL=GLOB(0x238f668)<<< 250-PIPELINING Net::SMTP::SSL=GLOB(0x238f668)<<< 250-8BITMIME Net::SMTP::SSL=GLOB(0x238f668)<<< 250-SIZE 80000000 Net::SMTP::SSL=GLOB(0x238f668)<<< 250-DSN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH=LOGIN PLAIN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH LOGIN PLAIN Net::SMTP::SSL=GLOB(0x238f668)<<< 250-DELIVERBY Net::SMTP::SSL=GLOB(0x238f668)<<< 250 HELP Password: Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified 5.5.2 AUTH mechanism must be specified

After: Net::SMTP=GLOB(0x1ac4a60)>>> STARTTLS Net::SMTP=GLOB(0x1ac4a60)<<< 220 2.0.0 Ready to start TLS Net::SMTP::SSL=GLOB(0x1ac4a60)>>> EHLO localhost.localdomain Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-yyy.yyy.yyy Hello xxx.xxx [xxx.xxx.xxx.xxx], pleased to meet you Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-ENHANCEDSTATUSCODES Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-PIPELINING Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-8BITMIME Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-SIZE 80000000 Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-DSN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-AUTH=LOGIN PLAIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-AUTH LOGIN PLAIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250-DELIVERBY Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 250 HELP Password: Net::SMTP::SSL=GLOB(0x1ac4a60)>>> AUTH LOGIN Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 334 VXNlcm5hbWU6 Net::SMTP::SSL=GLOB(0x1ac4a60)>>> emJ5c3plaw== Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 334 UGFzc3dvcmQ6 Net::SMTP::SSL=GLOB(0x1ac4a60)>>> dGVzdA== Net::SMTP::SSL=GLOB(0x1ac4a60)<<< 535 5.7.0 authentication failed 5.7.0 authentication failed

The password is incorrect in this snippet, but the protocol works correctly.
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
v3:  - the import is performed only if it will be used, and failure is ignored
 git-send-email.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Show changes to git-send-email.perl +4 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 37dfbe7..dbc435a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1098,6 +1098,10 @@ X-Mailer: git-send-email $gitversion
 		}
 
 		if (defined $smtp_authuser) {
+			eval {
+				require Authen::SASL;
+				Authen::SASL->import(qw(Perl));
+			};
 
 			if (!defined $smtp_authpass) {
 
-- 
1.7.6
Junio C Hamano· Sep 28, 2011, 22:00 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH v3] send-email: auth plain/login fix

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> This patch is tested by sending it :)
I am tempted to shorten the log message to cull the protocol trace.
    send-email: auth plain/login fix
    
    git send-email does not authenticate properly when communicating over TLS
    with a server supporting only AUTH PLAIN and AUTH LOGIN. This is e.g. the
    standard server setup under debian with exim4 and probably everywhere
    where system accounts are used.
    
    The problem (only?) exists when libauthen-sasl-cyrus-perl (Authen::SASL::Cyrus)
    is installed. Importing Authen::SASL::Perl makes Authen::SASL use the perl
    implementation, which works better.
    
    The solution is based on this forum thread:
    
        http://www.perlmonks.org/?node_id=904354
    
    This patch is tested by sending it. Without this fix, the interaction with
    the server failed like this:
    
        ...
        Password:
        Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH
        Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified
        5.5.2 AUTH mechanism must be specified
    
    Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Around the line "the server failed like this:", it would be helpful if we can say how others can reproduce the protocol exchange log shown above. That would help those who may (or may not) be seeing a similar issue to diagnose if this commit may help them (or is the culprit of a breakage they find in the future).

Thanks.
Zbigniew Jędrzejewski-Szmek· Sep 29, 2011, 14:16 UTC · re: Junio C Hamano · lore

[PATCH v4] send-email: auth plain/login fix

git send-email was not authenticating properly when communicating over TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is e.g. the standard server setup under debian with exim4 and probably everywhere where system accounts are used.

The problem (only?) exists when libauthen-sasl-cyrus-perl (Authen::SASL::Cyrus) is installed. Importing Authen::SASL::Perl makes Authen::SASL use the perl implementation which works better.

The solution is based on this forum thread: http://www.perlmonks.org/?node_id=904354.

This patch is tested by sending it. Without this fix, the interaction with the server failed like this:

$ git send-email --smtp-encryption=tls --smtp-server=... --smtp-debug=1 change1.patch ... Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH LOGIN PLAIN Password: Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified 5.5.2 AUTH mechanism must be specified

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

--- v2: - the import is performed only if it will be used v3: - the import is performed only if it will be used, and failure is ignored v4: - improved commit message

 git-send-email.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Show changes to git-send-email.perl +4 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 37dfbe7..dbc435a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1098,6 +1098,10 @@ X-Mailer: git-send-email $gitversion
 		}
 
 		if (defined $smtp_authuser) {
+			eval {
+				require Authen::SASL;
+				Authen::SASL->import(qw(Perl));
+			};
 
 			if (!defined $smtp_authpass) {
 
-- 
1.7.6
Joe Perches· Sep 29, 2011, 15:01 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH v4] send-email: auth plain/login fix

On Thu, 2011-09-29 at 16:16 +0200, Zbigniew Jędrzejewski-Szmek wrote:
Show 9 quoted lines
> git send-email was not authenticating properly when communicating over
> TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is
> e.g. the standard server setup under debian with exim4 and probably
> everywhere where system accounts are used.
> 
> The problem (only?) exists when libauthen-sasl-cyrus-perl
> (Authen::SASL::Cyrus) is installed. Importing Authen::SASL::Perl
> makes Authen::SASL use the perl implementation which works
> better.
[]
> diff --git a/git-send-email.perl b/git-send-email.perl
[]
Show 8 quoted lines
> @@ -1098,6 +1098,10 @@ X-Mailer: git-send-email $gitversion
>  		}
>  
>  		if (defined $smtp_authuser) {
> +			eval {
> +				require Authen::SASL;
> +				Authen::SASL->import(qw(Perl));
> +			};
Thanks for keeping at this.
One comment:
This is a workaround for a nominal defect.

As such, I think the code should be commented to note why it exists.

How about adding a comment like:
 		if (defined $smtp_authuser) {
			# Workaround AUTH PLAIN/LOGIN interaction defect
			# with Authen::SASL::Cyrus
			eval {
				require Authen::SASL;
Zbigniew Jędrzejewski-Szmek· Sep 29, 2011, 17:02 UTC · re: Joe Perches · lore

[PATCH] send-email: auth plain/login fix

git send-email was not authenticating properly when communicating over TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is e.g. the standard server setup under debian with exim4 and probably everywhere where system accounts are used.

The problem (only?) exists when libauthen-sasl-cyrus-perl (Authen::SASL::Cyrus) is installed. Importing Authen::SASL::Perl makes Authen::SASL use the perl implementation which works better.

The solution is based on this forum thread: http://www.perlmonks.org/?node_id=904354.

This patch is tested by sending it. Without this fix, the interaction with the server failed like this:

$ git send-email --smtp-encryption=tls --smtp-server=... --smtp-debug=1 change1.patch ... Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH LOGIN PLAIN Password: Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified 5.5.2 AUTH mechanism must be specified

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
v2:  - the import is performed only if it will be used
v3:  - the import is performed only if it will be used, and failure is ignored
v4:  - improved commit message
v5:  - comment in code
 git-send-email.perl |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Show changes to git-send-email.perl +6 −0
diff --git a/git-send-email.perl b/git-send-email.perl
index 98ab33a..f2a6e46 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1095,6 +1095,12 @@ X-Mailer: git-send-email $gitversion
 		}
 
 		if (defined $smtp_authuser) {
+			# Workaround AUTH PLAIN/LOGIN interaction defect
+			# with Authen::SASL::Cyrus
+			eval {
+				require Authen::SASL;
+				Authen::SASL->import(qw(Perl));
+			};
 
 			if (!defined $smtp_authpass) {
 
-- 
1.7.6

← back to recent threads