Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'

From: Brian Norris <computersforpeace@gmail.com>
Date: Feb 15, 2012, 23:49 UTC
Message-ID: <CAN8TOE_BnkOcMQRTY-GWrHozYD0+0giWn2LtjB8AVnP_DzA+Sg@mail.gmail.com>
In-Reply-To: <20120215220629.GA17672@sigill.intra.peff.net>

On Wed, Feb 15, 2012 at 2:06 PM, Jeff King <peff@peff.net> wrote:

Show 14 quoted lines

> On Wed, Feb 15, 2012 at 01:42:58PM -0800, Brian Norris wrote:
>
>> According to the manpage:
>>
>>  --smtp-encryption=<encryption>
>>         Specify the encryption to use, either ssl or tls. Any other value
>>       reverts to plain SMTP.  Default is the value of
>>         sendemail.smtpencryption.
>>
>> However, if I have already set sendemail.smtpencryption but try to override
>> it with something like 'no', the authentication code block still tries to ask
>> for a password (and fails).
>
> Sounds reasonable.

An error like
  Command unknown: 'AUTH' at /usr/local/libexec/git-core/git-send-email
is reasonable?

Show 13 quoted lines

>> This patch forces $smtp_encryption to 'none' when a proper encryption type is
>> not provided, then checks $smtp_encryption before proceeding to authentication.
>
> Defaulting everything except "ssl" or "tls" to "none" seems risky to me.
> If I am understanding your patch correctly, then doing this:
>
>  git send-email --smtp-encryption=SSL
>
> will silently treat that as "don't do encryption", which could have
> surprising security implications for the user. I chose all-caps as it is
> an obvious mistake to make. We probably should treat it the same as
> lowercase "ssl", but the same argument applies to other typos like
> "tsl".

Well, git-send-email already doesn't handle typos or capitalization correctly, AFAICT. So nothing new here.

> It seems like a much safer default would be to die() on an invalid
> encryption specifier.

Fine. But then we need to define a behavior that means 'no encryption.' Like 'none' instead of just saying 'anything but tls or ssl.'

Now that I look at this again, I think part of the issue I have is that there is no way to override *smtp-user* via command-line, in order to do unencrypted, unauthenticated email. So the *authentication* not the encryption is really my main problem...I'll take another look and try a new patch.

Thanks, Brian