From: Brian Norris Date: Wed, 15 Feb 2012 23:49:59 GMT Subject: Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none' Message-ID: In-Reply-To: <20120215220629.GA17672@sigill.intra.peff.net> On Wed, Feb 15, 2012 at 2:06 PM, Jeff King wrote: > On Wed, Feb 15, 2012 at 01:42:58PM -0800, Brian Norris wrote: > >> According to the manpage: >> >>  --smtp-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? >> 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