Re: [PATCH v2] send-email: provide whitelist of SMTP AUTH mechanisms

> On Sun, Aug 2, 2015 at 12:42 PM, Jan Viktorin
> <viktorin@rehivetech.com> wrote:
> > When sending an e-mail, the client and server must
> > agree on an authentication mechanism. Some servers
> > (due to misconfiguration or a bug) deny valid
> > credentials for certain mechanisms. In this patch,
> > a new option --smtp-auth and configuration entry
> > smtpauth are introduced. If smtp_auth is defined,
> > it works as a whitelist of allowed mechanisms for
> > authentication selected from the ones supported by
> > the installed SASL perl library.
> 
> Nit: This would read a bit more nicely if wrapped to 70-72 columns.
> 
> > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> > ---
> > diff --git a/Documentation/git-send-email.txt
> > b/Documentation/git-send-email.txt index 7ae467b..c237c80 100644
> > --- a/Documentation/git-send-email.txt
> > +++ b/Documentation/git-send-email.txt
> > @@ -171,6 +171,14 @@ Sending
> > +--smtp-auth=<mechs>::
> > +       Specify allowed SMTP-AUTH mechanisms. This setting forces
> > using only
> > +       the listed mechanisms. Separate allowed mechanisms by a
> > whitespace.
> 
> Perhaps:
> 
>     Whitespace-separated list of allowed SMTP-AUTH mechanisms.
> 
> > +       Example: PLAIN LOGIN GSSAPI. If at least one of the
> > specified mechanisms
> > +       matchs those advertised by the SMTP server and it is
> > supported by the SASL
> 
> s/matchs/matches/
> 
> > +       library we use, it is used for authentication. If neither
> > of 'sendemail.smtpAuth'
> > +       or '--smtp-auth' is specified, all mechanisms supported on
> > client can be used.
> 
> s/neither of/neither/
> s/or/nor/
> 
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index ae9f869..ebc1e90 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -75,6 +75,9 @@ git send-email [options] <file | directory |
> > rev-list options > Pass an empty string to disable certificate
> >                                       verification.
> >      --smtp-domain           <str>  * The domain name sent to
> > HELO/EHLO handshake
> > +    --smtp-auth             <str>  * Space separated list of
> > allowed AUTH methods.
> 
> s/Space separated/Space-separated/
> 
> > +                                     This setting forces to use
> > one of the listed methods.
> > +                                     Supported: PLAIN LOGIN
> > CRAM-MD5 DIGEST-MD5.
> 
> Since you're no longer checking explicitly for these mechanisms, you
> probably want to drop the "Supported:" line.
> 
> >      --smtp-debug            <0|1>  * Disable, enable Net::SMTP
> > debug.
> >
> >    Automating:
> > @@ -1136,6 +1141,10 @@ sub smtp_auth_maybe {
> >                 Authen::SASL->import(qw(Perl));
> >         };
> >
> > +       if($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
> > +               die "invalid smtp auth: '${smtp_auth}'";
> > +       }
> 
> Style: space after 'if'
> 
> >         # TODO: Authentication may fail not because credentials were
> >         # invalid but due to other reasons, in which we should not
> >         # reject credentials.
> > @@ -1148,6 +1157,20 @@ sub smtp_auth_maybe {
> >                 'password' => $smtp_authpass
> >         }, sub {
> >                 my $cred = shift;
> > +
> > +               if($smtp_auth) {
> 
> Style: space after 'if'
> 
> > +                       my $sasl = Authen::SASL->new(
> > +                               mechanism => $smtp_auth,
> > +                               callback => {
> > +                                       user => $cred->{'username'},
> > +                                       pass => $cred->{'password'},
> > +                                       authname =>
> > $cred->{'username'},
> > +                               }
> > +                       );
> > +
> > +                       return !!$smtp->auth($sasl);
> > +               }
> > +
> >                 return !!$smtp->auth($cred->{'username'},
> > $cred->{'password'}); });
> >
> > --
> > 2.5.0