From f7d397ea0913bb3a22bf44377a850fc8b5ab7f4d Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Mon, 10 Feb 2014 16:38:13 -0500 Subject: [PATCH] Eliminate SMTP AUTOMATIC authentication The server settings for IMAP and POP3 have no such AUTOMATIC setting. (Nor does Thunderbird have any such setting.) The AUTOMATIC option is no longer offered to users as a choice. A pre-existing setting will continue to be honored, but only to the extent that it doesn't result in insecure password transmission. Users in such a situation will get a "Failed to send some messages" notification containing the exception text that says to update their outgoing server authentication setting. One of the problems with "AUTOMATIC" is that users may not fully understand its security implications. For example, a MITM attack could mask a server's support for STARTTLS and CRAM-MD5, resulting in password disclosure in certain configurations. This commit also makes changes to the SMTP authentication process. No attempt is made to authenticate using methods that the server does not profess to support in its EHLO response. This is the same behavior as found in Thunderbird. --- res/values/strings.xml | 1 - src/com/fsck/k9/mail/AuthType.java | 22 ++++- .../fsck/k9/mail/transport/SmtpTransport.java | 95 ++++++++++++------- 3 files changed, 80 insertions(+), 38 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index 8e592c38a..9de519bd3 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -385,7 +385,6 @@ Please submit bug reports, contribute new features and ask questions at IMAP Exchange (WebDAV) - Automatic Normal password Encrypted password diff --git a/src/com/fsck/k9/mail/AuthType.java b/src/com/fsck/k9/mail/AuthType.java index 9c2425e3c..c77371661 100644 --- a/src/com/fsck/k9/mail/AuthType.java +++ b/src/com/fsck/k9/mail/AuthType.java @@ -6,13 +6,27 @@ import com.fsck.k9.R; public enum AuthType { /* - * The names of these auth. types are saved as strings when settings are - * exported, and are also saved as part of the Server URI saved in the - * account settings. + * The names of these authentication types are saved as strings when + * settings are exported and are also saved as part of the Server URI stored + * in the account settings. + * + * PLAIN and CRAM_MD5 originally referred to specific SASL authentication + * mechanisms. Their meaning has since been broadened to mean authentication + * with unencrypted and encrypted passwords, respectively. Nonetheless, + * their original names have been retained for backward compatibility with + * user settings. */ - AUTOMATIC(R.string.account_setup_auth_type_automatic), PLAIN(R.string.account_setup_auth_type_normal_password), CRAM_MD5(R.string.account_setup_auth_type_encrypted_password), + + /* + * The following are obsolete authentication settings that were used with + * SMTP. They are no longer presented to the user as options, but they may + * still exist in a user's settings from a previous version or may be found + * when importing settings. + */ + AUTOMATIC(0), + LOGIN(0); private final int mResourceId; diff --git a/src/com/fsck/k9/mail/transport/SmtpTransport.java b/src/com/fsck/k9/mail/transport/SmtpTransport.java index a52873595..4c5101ff0 100644 --- a/src/com/fsck/k9/mail/transport/SmtpTransport.java +++ b/src/com/fsck/k9/mail/transport/SmtpTransport.java @@ -49,7 +49,7 @@ public class SmtpTransport extends Transport { String host; int port; ConnectionSecurity connectionSecurity; - AuthType authType = AuthType.AUTOMATIC; + AuthType authType = AuthType.PLAIN; String username = null; String password = null; @@ -197,6 +197,7 @@ public class SmtpTransport extends Transport { @Override public void open() throws MessagingException { try { + boolean secureConnection = false; InetAddress[] addresses = InetAddress.getAllByName(mHost); for (int i = 0; i < addresses.length; i++) { try { @@ -211,6 +212,7 @@ public class SmtpTransport extends Transport { new SecureRandom()); mSocket = TrustedSocketFactory.createSocket(sslContext); mSocket.connect(socketAddress, SOCKET_CONNECT_TIMEOUT); + secureConnection = true; } else { mSocket = new Socket(); mSocket.connect(socketAddress, SOCKET_CONNECT_TIMEOUT); @@ -278,17 +280,12 @@ public class SmtpTransport extends Transport { * Exim. */ extensions = sendHello(localHost); + secureConnection = true; } else if (mConnectionSecurity == ConnectionSecurity.STARTTLS_REQUIRED) { throw new MessagingException("TLS not supported but required"); } } - boolean useAuthPlain = AuthType.PLAIN.equals(mAuthType) || AuthType.LOGIN.equals(mAuthType); - boolean useAuthCramMD5 = AuthType.CRAM_MD5.equals(mAuthType); - - // Automatically choose best authentication method if none was explicitly selected - boolean useAutomaticAuth = !(useAuthPlain || useAuthCramMD5); - boolean authLoginSupported = false; boolean authPlainSupported = false; boolean authCramMD5Supported = false; @@ -310,37 +307,69 @@ public class SmtpTransport extends Transport { if (mUsername != null && mUsername.length() > 0 && mPassword != null && mPassword.length() > 0) { - if (useAuthCramMD5 || (useAutomaticAuth && authCramMD5Supported)) { - if (!authCramMD5Supported && K9.DEBUG && K9.DEBUG_PROTOCOL_SMTP) { - Log.d(K9.LOG_TAG, "Using CRAM_MD5 as authentication method although the " + - "server didn't advertise support for it in EHLO response."); - } - saslAuthCramMD5(mUsername, mPassword); - } else if (useAuthPlain || (useAutomaticAuth && authPlainSupported)) { - if (!authPlainSupported && K9.DEBUG && K9.DEBUG_PROTOCOL_SMTP) { - Log.d(K9.LOG_TAG, "Using PLAIN as authentication method although the " + - "server didn't advertise support for it in EHLO response."); - } - try { + + switch (mAuthType) { + + /* + * LOGIN is an obsolete option which is unavailable to users, + * but it still may exist in a user's settings from a previous + * version, or it may have been imported. + */ + case LOGIN: + case PLAIN: + // try saslAuthPlain first, because it supports UTF-8 explicitly + if (authPlainSupported) { saslAuthPlain(mUsername, mPassword); - } catch (MessagingException ex) { - // PLAIN is a special case. Historically, PLAIN has represented both PLAIN and LOGIN; only the - // protocol being advertised by the server would be used, with PLAIN taking precedence. Instead - // of using only the requested protocol, we'll try PLAIN and then try LOGIN. - if (useAuthPlain && authLoginSupported) { - if (K9.DEBUG && K9.DEBUG_PROTOCOL_SMTP) { - Log.d(K9.LOG_TAG, "Using legacy PLAIN authentication behavior and trying LOGIN."); - } + } else if (authLoginSupported) { + saslAuthLogin(mUsername, mPassword); + } else { + throw new MessagingException("Authentication methods SASL PLAIN and LOGIN are unavailable."); + } + break; + + case CRAM_MD5: + if (authCramMD5Supported) { + saslAuthCramMD5(mUsername, mPassword); + } else { + throw new MessagingException("Authentication method CRAM-MD5 is unavailable."); + } + break; + + /* + * AUTOMATIC is an obsolete option which is unavailable to users, + * but it still may exist in a user's settings from a previous + * version, or it may have been imported. + */ + case AUTOMATIC: + if (secureConnection) { + // try saslAuthPlain first, because it supports UTF-8 explicitly + if (authPlainSupported) { + saslAuthPlain(mUsername, mPassword); + } else if (authLoginSupported) { saslAuthLogin(mUsername, mPassword); + } else if (authCramMD5Supported) { + saslAuthCramMD5(mUsername, mPassword); } else { - // If it was auto detected and failed, continue throwing the exception back up. - throw ex; + throw new MessagingException("No supported authentication methods available."); + } + } else { + if (authCramMD5Supported) { + saslAuthCramMD5(mUsername, mPassword); + } else { + /* + * We refuse to insecurely transmit the password + * using the obsolete AUTOMATIC setting because of + * the potential for a MITM attack. Affected users + * must choose a different setting. + */ + throw new MessagingException( + "Update your outgoing server authentication setting. AUTOMATIC auth. is unavailable."); } } - } else if (useAutomaticAuth && authLoginSupported) { - saslAuthLogin(mUsername, mPassword); - } else { - throw new MessagingException("No valid authentication mechanism found."); + break; + + default: + throw new MessagingException("Unhandled authentication method found in the server settings (bug)."); } } } catch (SSLException e) {