Re: [PATCH v2] send-email: provide whitelist of SMTP AUTH mechanisms
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Aug 2, 2015, 18:57 UTC
- Message-ID
- <CAPig+cQwFxVtO1C_RAumGP6_et21ggORB4jhpcUtBYNznNH1qA@mail.gmail.com>
- In-Reply-To
- <1438533769-17460-1-git-send-email-viktorin@rehivetech.com>
On Sun, Aug 2, 2015 at 12:42 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
Show 9 quoted lines
> 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.
Show 10 quoted lines
> 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/
Show 9 quoted lines
> 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.
Show 10 quoted lines
> --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'
Show 9 quoted lines
> # 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'
Show 17 quoted lines
> + 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