On Sun, Jun 21, 2020 at 06:57:26AM -0700, Kevin J. McCarthy wrote:
On Sun, Jun 21, 2020 at 01:37:53PM +0200, Vincent Lefevre wrote:On 2020-06-20 14:49:56 -0700, Kevin J. McCarthy wrote: BTW, I don't think that testing $ssl_starttls here is useful, as I've just said in bug 246https://gitlab.com/muttmua/mutt/-/issues/246 Its value alone will not prevent a MITM attack, and this test may annoy users who do not need TLS because the connection is already encapsulated in an encrypted connection.Sorry, I don't understand this comment. The fix checks for $tunnel, and won't check for either in that case.The intent of the patch was to prevent a possible MITM in default configuration. Yes, there are other MITMs that aren't mitigated by $ssl_starttls, but this one can be done. The user is free to turn off $ssl_starttls in an account-hook if they know their unencrypted PREAUTH is desired.
This has been a bad week and I'm tired. Sorry, I understand your point now.
I think you are right. I'm protecting against *nothing* by inserting a $ssl_starttls prompt for "* PREAUTH" because the MITM can just as easily insert "* OK" and strip the STARTTLS capability.
So the prompt is just an annoyance because the user really needs to set $ssl_force_tls to stop all this nonsense.
The code should really be just:
else if (ascii_strncasecmp ("* PREAUTH", idata->buf, 9) == 0)
{
#if defined(USE_SSL)
/* Unless using $tunnel, an unencrypted PREAUTH response may be
* a MITM attack. Abort if $ssl_force_tls is set. */
if (!idata->conn->ssf && !Tunnel && option(OPTSSLFORCETLS))
{
mutt_error _("Encrypted connection unavailable");
mutt_sleep (1);
goto err_close_conn;
}
#endif
[...]
}
I'd appreciate other's comments. I'm plan on making another stable
release in a couple days and want to get this right.
-- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
signature.asc
Description: PGP signature
