# [PATCH] git-send-email: Add AUTH LOGIN support 19 messages from 2011-08-06 to 2011-09-29. Participants: Joe Perches, Junio C Hamano, Zbigniew Jędrzejewski-Szmek, Jakub Narebski, Jeff King. Thread: https://gitlist.dev/t/28030 ## Joe Perches, 2011-08-06 00:40 Subject: [PATCH] git-send-email: Add AUTH LOGIN support Message-ID: URL: https://gitlist.dev/e/c1be0c3bd05779897fdc92907fe65d84c88ac72b.1312591104.git.joe%40perches.com ``` 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 --- Documentation/git-send-email.txt | 3 +++ git-send-email.perl | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) 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=:: + 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, 2011-08-06 04:54 Subject: Re: [PATCH] git-send-email: Add AUTH LOGIN support Message-ID: <7vzkjn16n6.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzkjn16n6.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Joe Perches 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? It probably is not as simple as installing Authen::SASL::*::LOGIN, but still... ``` ## Joe Perches, 2011-08-06 05:21 Subject: Re: [PATCH] git-send-email: Add AUTH LOGIN support Message-ID: <1312608114.6419.18.camel@Joe-Laptop> URL: https://gitlist.dev/e/1312608114.6419.18.camel%40Joe-Laptop In-Reply-To: <7vzkjn16n6.fsf@alter.siamese.dyndns.org> ``` On Fri, 2011-08-05 at 21:54 -0700, Junio C Hamano wrote: > Joe Perches 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, 2011-08-19 01:16 Subject: Re: [PATCH] git-send-email: Add AUTH LOGIN support Message-ID: <1313716585.11178.2.camel@Joe-Laptop> URL: https://gitlist.dev/e/1313716585.11178.2.camel%40Joe-Laptop In-Reply-To: <1312608114.6419.18.camel@Joe-Laptop> ``` On Fri, 2011-08-05 at 22:21 -0700, Joe Perches wrote: > On Fri, 2011-08-05 at 21:54 -0700, Junio C Hamano wrote: > > Joe Perches 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, 2011-08-19 23:09 Subject: Re: [PATCH] git-send-email: Add AUTH LOGIN support Message-ID: <7v1uwh0zj4.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1uwh0zj4.fsf%40alter.siamese.dyndns.org In-Reply-To: <1313716585.11178.2.camel@Joe-Laptop> ``` Joe Perches writes: > 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, 2011-08-19 23:24 Subject: Re: [PATCH] git-send-email: Add AUTH LOGIN support Message-ID: <1313796280.11178.25.camel@Joe-Laptop> URL: https://gitlist.dev/e/1313796280.11178.25.camel%40Joe-Laptop In-Reply-To: <7v1uwh0zj4.fsf@alter.siamese.dyndns.org> ``` On Fri, 2011-08-19 at 16:09 -0700, Junio C Hamano wrote: > Joe Perches 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, 2011-08-20 01:01 Subject: Re: [PATCH] git-send-email: Add AUTH LOGIN support Message-ID: <7vsjowzyj8.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsjowzyj8.fsf%40alter.siamese.dyndns.org In-Reply-To: <1313796280.11178.25.camel@Joe-Laptop> ``` Joe Perches writes: > 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, 2011-09-24 15:49 Subject: [PATCH] send-email: auth plain/login fix Message-ID: <1316879367-1182-1-git-send-email-zbyszek@in.waw.pl> URL: https://gitlist.dev/e/1316879367-1182-1-git-send-email-zbyszek%40in.waw.pl In-Reply-To: <7vzkjn16n6.fsf@alter.siamese.dyndns.org> ``` 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(-) 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, 2011-09-26 16:38 Subject: Re: [PATCH] send-email: auth plain/login fix Message-ID: <7v8vpb5kea.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8vpb5kea.fsf%40alter.siamese.dyndns.org In-Reply-To: <1316879367-1182-1-git-send-email-zbyszek@in.waw.pl> ``` Zbigniew Jędrzejewski-Szmek 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, 2011-09-26 16:59 Subject: Re: [PATCH] send-email: auth plain/login fix Message-ID: <1317056343.14197.2.camel@Joe-Laptop> URL: https://gitlist.dev/e/1317056343.14197.2.camel%40Joe-Laptop In-Reply-To: <7v8vpb5kea.fsf@alter.siamese.dyndns.org> ``` On Mon, 2011-09-26 at 09:38 -0700, Junio C Hamano wrote: > Zbigniew Jędrzejewski-Szmek 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 > 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, 2011-09-26 17:24 Subject: Re: [PATCH] send-email: auth plain/login fix Message-ID: URL: https://gitlist.dev/e/m3fwjjp69m.fsf%40localhost.localdomain In-Reply-To: <1316879367-1182-1-git-send-email-zbyszek@in.waw.pl> ``` Zbigniew Jędrzejewski-Szmek 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. > > The solution comes from this forum thread: > http://www.perlmonks.org/?node_id=904354 Signoff? > --- > 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, 2011-09-27 21:36 Subject: [PATCH v2] send-email: auth plain/login fix Message-ID: <1317159419-23166-1-git-send-email-zbyszek@in.waw.pl> URL: https://gitlist.dev/e/1317159419-23166-1-git-send-email-zbyszek%40in.waw.pl In-Reply-To: ``` 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 --- 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(-) 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, 2011-09-27 21:48 Subject: Re: [PATCH v2] send-email: auth plain/login fix Message-ID: <20110927214817.GD5176@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110927214817.GD5176%40sigill.intra.peff.net In-Reply-To: <1317159419-23166-1-git-send-email-zbyszek@in.waw.pl> ``` 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... > 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, 2011-09-28 10:26 Subject: [PATCH v3] send-email: auth plain/login fix Message-ID: <1317205600-7210-1-git-send-email-zbyszek@in.waw.pl> URL: https://gitlist.dev/e/1317205600-7210-1-git-send-email-zbyszek%40in.waw.pl In-Reply-To: ``` 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 --- 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(-) 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, 2011-09-28 22:00 Subject: Re: [PATCH v3] send-email: auth plain/login fix Message-ID: <7v62kcz5rr.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v62kcz5rr.fsf%40alter.siamese.dyndns.org In-Reply-To: <1317205600-7210-1-git-send-email-zbyszek@in.waw.pl> ``` Zbigniew Jędrzejewski-Szmek 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 Signed-off-by: Junio C Hamano 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, 2011-09-29 14:16 Subject: [PATCH v4] send-email: auth plain/login fix Message-ID: <20110929141616.GU10763@in.waw.pl> URL: https://gitlist.dev/e/20110929141616.GU10763%40in.waw.pl In-Reply-To: <7v62kcz5rr.fsf@alter.siamese.dyndns.org> ``` 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 Signed-off-by: Junio C Hamano --- 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(-) 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, 2011-09-29 15:01 Subject: Re: [PATCH v4] send-email: auth plain/login fix Message-ID: <1317308474.1854.8.camel@Joe-Laptop> URL: https://gitlist.dev/e/1317308474.1854.8.camel%40Joe-Laptop In-Reply-To: <20110929141616.GU10763@in.waw.pl> ``` On Thu, 2011-09-29 at 16:16 +0200, Zbigniew Jędrzejewski-Szmek wrote: > 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 [] > @@ -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, 2011-09-29 17:02 Subject: [PATCH] send-email: auth plain/login fix Message-ID: <1317315765-5471-1-git-send-email-zbyszek@in.waw.pl> URL: https://gitlist.dev/e/1317315765-5471-1-git-send-email-zbyszek%40in.waw.pl In-Reply-To: <1317308474.1854.8.camel@Joe-Laptop> ``` 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 Signed-off-by: Junio C Hamano --- 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(-) 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 ``` ## Junio C Hamano, 2011-09-29 18:12 Subject: Re: [PATCH] send-email: auth plain/login fix Message-ID: <7vipobtdyn.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vipobtdyn.fsf%40alter.siamese.dyndns.org In-Reply-To: <1317315765-5471-1-git-send-email-zbyszek@in.waw.pl> ``` Thanks. ```